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

chrome/browser/search/search.cc

Index: chrome/browser/search/search.cc
diff --git a/chrome/browser/search/search.cc b/chrome/browser/search/search.cc
index d4e8b1a604de280ee73b5b4f9694cd45f76b35c1..cb9ee26c599a8c2f45e7dcb2452fd9d81d45f5ab 100644
--- a/chrome/browser/search/search.cc
+++ b/chrome/browser/search/search.cc
@@ -6,6 +6,7 @@
 
 #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"
@@ -86,6 +87,33 @@ const char kEmbeddedSearchFieldTrialName[] = "EmbeddedSearch";
 // 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";
 
@@ -259,6 +287,34 @@ bool IsURLAllowedForSupervisedUser(const GURL& url, Profile* profile) {
   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.
@@ -451,25 +507,9 @@ std::vector<GURL> GetSearchURLs(Profile* profile) {
 }
 
 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) {
@@ -594,12 +634,23 @@ bool HandleNewTabURLRewrite(GURL* url,
     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,
@@ -607,14 +658,25 @@ bool HandleNewTabURLReverseRewrite(GURL* url,
   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,