NTP: add histogram to track New Tab page URL status.

chrome/browser/search/search.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/search/search.h"
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
+#include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
@@ skipping 60 matching lines @@
 // Dev & Canary, for now the code accepts both names.
 // TODO(dcblack): Remove the InstantExtended name once M31 hits the Beta
 // channel.
 const char kInstantExtendedFieldTrialName[] = "InstantExtended";
 const char kEmbeddedSearchFieldTrialName[] = "EmbeddedSearch";
 
 // If the field trial's group name ends with this string its configuration will
 // be ignored and Instant Extended will not be enabled by default.
 const char kDisablingSuffix[] = "DISABLED";
 
+// Status of the New Tab URL for the default Search provider. NOTE: Used in a
+// UMA histogram so values should only be added at the end and not reordered.
+enum NewTabPageURLState {
+  // Valid URL that should be used.
+  NEW_TAB_URL_VALID = 0,
+
+  // Corrupt state (e.g. no profile or template url).
+  NEW_TAB_URL_BAD = 1,
+
+  // URL should not be used because in incognito window.
+  NEW_TAB_URL_INCOGNITO = 2,
+
+  // No New Tab URL set for provider.
+  NEW_TAB_URL_NOT_SET = 3,
+
+  // URL is not secure.
+  NEW_TAB_URL_INSECURE = 4,
+
+  // URL should not be used because Suggest is disabled.
+  NEW_TAB_URL_SUGGEST_OFF = 5,
+
+  // URL should not be used because it is blocked for a supervised user.
+  NEW_TAB_URL_BLOCKED = 6,
+
+  NEW_TAB_URL_MAX
+};
+
 // Used to set the Instant support state of the Navigation entry.
 const char kInstantSupportStateKey[] = "instant_support_state";
 
 const char kInstantSupportEnabled[] = "Instant support enabled";
 const char kInstantSupportDisabled[] = "Instant support disabled";
 const char kInstantSupportUnknown[] = "Instant support unknown";
 
 InstantSupportState StringToInstantSupportState(const base::string16& value) {
   if (value == base::ASCIIToUTF16(kInstantSupportEnabled))
     return INSTANT_SUPPORT_YES;
@@ skipping 153 matching lines @@
   ManagedModeURLFilter* url_filter =
       managed_user_service->GetURLFilterForUIThread();
   if (url_filter->GetFilteringBehaviorForURL(url) ==
           ManagedModeURLFilter::BLOCK) {
     return false;
   }
 #endif
   return true;
 }
 
+NewTabPageURLState GetNewTabPageURLState(Profile* profile, GURL* new_tab_url) {

[Jered, 2014/01/31 21:56:23]

See comment below. How about adding
struct NewTabURL {
  NewTabURL(const GURL& url, int state);
};
then changing this to
NewTabURL GetNewTabPageURL(Profile* profile) {
  if (!profile)
    return NewTabURL(NEW_TAB_URL_BAD, GURL());
  if (profile->IsOffTheRecord())
    return NewTabURL(NEW_TAB_URL_INCOGNITO, GURL());
  ...
}
Caller just says
NewTabURL new_tab_url(GetNewTabPageURL(profile));
UMA_HISTOGRAM_ENUMERATION("NewTab.URLState", new_tab_url.reason,
NEW_TAB_URL_MAX);
*url = new_tab_url.url;
return url->is_valid(); // or something

[samarth, 2014/01/31 23:52:58]

OK, I did something like this. WDYT?

+  if (!profile)
+    return NEW_TAB_URL_BAD;
+
+  if (profile->IsOffTheRecord())
+    return NEW_TAB_URL_INCOGNITO;
+
+  TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
+  if (!template_url)
+    return NEW_TAB_URL_BAD;
+
+  *new_tab_url = TemplateURLRefToGURL(template_url->new_tab_url_ref(),
+                                      kDisableStartMargin, false, false);
+  if (!new_tab_url->is_valid())
+    return NEW_TAB_URL_NOT_SET;
+
+  if (!new_tab_url->SchemeIsSecure())
+    return NEW_TAB_URL_INSECURE;
+
+  if (!IsSuggestPrefEnabled(profile))
+    return NEW_TAB_URL_SUGGEST_OFF;
+
+  if (!IsURLAllowedForSupervisedUser(*new_tab_url, profile))
+    return NEW_TAB_URL_BLOCKED;
+
+  return NEW_TAB_URL_VALID;
+}
+
 }  // namespace
 
 // Negative start-margin values prevent the "es_sm" parameter from being used.
 const int kDisableStartMargin = -1;
 
 bool IsInstantExtendedAPIEnabled() {
 #if defined(OS_IOS) || defined(OS_ANDROID)
   return false;
 #else
   return true;
@@ skipping 171 matching lines @@
   if (!template_url)
     return result;
   for (size_t i = 0; i < template_url->URLCount(); ++i) {
     TemplateURLRef ref(template_url, i);
     result.push_back(TemplateURLRefToGURL(ref, kDisableStartMargin, false,
                                           false));
   }
   return result;
 }
 
 GURL GetNewTabPageURL(Profile* profile) {

[Jered, 2014/01/31 21:56:23]

It'd be nice to keep the property that this function always returns the correct
URL. That way we don't have to worry about other callers not duplicating the
extra logic in HandleRewrite.

[samarth, 2014/01/31 23:52:58]

Done.

-  if (!profile || profile->IsOffTheRecord())
-    return GURL();
-
-  if (!IsSuggestPrefEnabled(profile))
-    return GURL(chrome::kChromeSearchLocalNtpUrl);
-
-  TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
-  if (!template_url)
-    return GURL(chrome::kChromeSearchLocalNtpUrl);
-
-  GURL url(TemplateURLRefToGURL(template_url->new_tab_url_ref(),
-                                kDisableStartMargin, false, false));
-  if (!url.is_valid() || !url.SchemeIsSecure())
-    return GURL(chrome::kChromeSearchLocalNtpUrl);
-
-  if (!IsURLAllowedForSupervisedUser(url, profile))
-    return GURL(chrome::kChromeSearchLocalNtpUrl);
-
-  return url;
+  GURL new_tab_url;
+  return GetNewTabPageURLState(profile, &new_tab_url) == NEW_TAB_URL_VALID ?
+      new_tab_url : GURL();
 }
 
 GURL GetSearchResultPrefetchBaseURL(Profile* profile) {
   return ShouldPrefetchSearchResults() ?
       GetInstantURL(profile, kDisableStartMargin, true) : GURL();
 }
 
 bool ShouldPrefetchSearchResults() {
   FieldTrialFlags flags;
   return GetFieldTrialInfo(&flags) && GetBoolValueForFlagWithDefault(
@@ skipping 104 matching lines @@
 bool HandleNewTabURLRewrite(GURL* url,
                             content::BrowserContext* browser_context) {
   if (!IsInstantExtendedAPIEnabled())
     return false;
 
   if (!url->SchemeIs(chrome::kChromeUIScheme) ||
       url->host() != chrome::kChromeUINewTabHost)
     return false;
 
   Profile* profile = Profile::FromBrowserContext(browser_context);
-  GURL new_tab_url(GetNewTabPageURL(profile));
-  if (!new_tab_url.is_valid())
-    return false;
+  GURL new_tab_url;
+  NewTabPageURLState url_state = GetNewTabPageURLState(profile, &new_tab_url);
+  UMA_HISTOGRAM_ENUMERATION("NewTab.URLState", url_state, NEW_TAB_URL_MAX);
 
-  *url = new_tab_url;
-  return true;
+  if (url_state == NEW_TAB_URL_VALID) {
+    *url = new_tab_url;
+    return true;
+  }
+
+  if (url_state != NEW_TAB_URL_INCOGNITO) {
+    // For non-incognito profiles, use the local NTP when the default search
+    // provider has no valid new tab url.
+    *url = GURL(chrome::kChromeSearchLocalNtpUrl);
+    return true;
+  }
+
+  return false;
 }
 
 bool HandleNewTabURLReverseRewrite(GURL* url,
                                    content::BrowserContext* browser_context) {
   if (!IsInstantExtendedAPIEnabled())
     return false;
 
+  // Do nothing in incognito.
   Profile* profile = Profile::FromBrowserContext(browser_context);
-  GURL new_tab_url(GetNewTabPageURL(profile));
-  if (!new_tab_url.is_valid() ||
-      !search::MatchesOriginAndPath(new_tab_url, *url))
+  if (profile && profile->IsOffTheRecord())
     return false;
 
-  *url = GURL(chrome::kChromeUINewTabURL);
-  return true;
+  if (search::MatchesOriginAndPath(
+      GURL(chrome::kChromeSearchLocalNtpUrl), *url)) {
+    *url = GURL(chrome::kChromeUINewTabURL);
+    return true;
+  }
+
+  GURL new_tab_url(GetNewTabPageURL(profile));
+  if (new_tab_url.is_valid() &&
+      search::MatchesOriginAndPath(new_tab_url, *url)) {
+    *url = GURL(chrome::kChromeUINewTabURL);
+    return true;
+  }
+
+  return false;
 }
 
 void SetInstantSupportStateInNavigationEntry(InstantSupportState state,
                                              content::NavigationEntry* entry) {
   if (!entry)
     return;
 
   entry->SetExtraData(kInstantSupportStateKey,
                       InstantSupportStateToString(state));
 }
@@ skipping 75 matching lines @@
 
 // Given a FieldTrialFlags object, returns the boolean value of the provided
 // flag.
 bool GetBoolValueForFlagWithDefault(const std::string& flag,
                                     bool default_value,
                                     const FieldTrialFlags& flags) {
   return !!GetUInt64ValueForFlagWithDefault(flag, default_value ? 1 : 0, flags);
 }
 
 }  // namespace chrome