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..753b566c67deb74f9915a361f69b36ec9bb74b2e 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,17 @@ const char kEmbeddedSearchFieldTrialName[] = "EmbeddedSearch";
 // be ignored and Instant Extended will not be enabled by default.
 const char kDisablingSuffix[] = "DISABLED";
 
+// Reasons for falling back to the local NTP instead of using a valid search
+// provider's URL. Used in a UMA histogram so values should only be added at the
+// end and not reordered.
+// (Keep in sync with InstantExtended_LocalNTPFallbackReason.)
+enum LocalNTPFallbackReason {
+  LOCAL_NTP_FALLBACK_INSECURE_URL = 1,
+  LOCAL_NTP_FALLBACK_SUGGEST_OFF = 2,
+  LOCAL_NTP_FALLBACK_BLOCKED_URL = 3,
+  LOCAL_NTP_FALLBACK_MAX
+};
+
 // Used to set the Instant support state of the Navigation entry.
 const char kInstantSupportStateKey[] = "instant_support_state";
 
@@ -114,6 +126,12 @@ base::string16 InstantSupportStateToString(InstantSupportState state) {
   return base::ASCIIToUTF16(kInstantSupportUnknown);
 }
 
+void RecordLocalNTPFallbackReason(LocalNTPFallbackReason reason) {
+  UMA_HISTOGRAM_ENUMERATION("InstantExtended.LocalNTPFallbackReason",
+                            reason,
+                            LOCAL_NTP_FALLBACK_MAX);
+}
+
 TemplateURL* GetDefaultSearchProviderTemplateURL(Profile* profile) {
   TemplateURLService* template_url_service =
       TemplateURLServiceFactory::GetForProfile(profile);
@@ -454,20 +472,29 @@ GURL GetNewTabPageURL(Profile* profile) {
   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);

[Jered, 2014/01/30 17:13:12]

How about adding another bucket, BAD_URL, to measure these other two cases? I'm
curious how much profile corruption is really a problem.

[samarth, 2014/01/31 16:05:20]

Done.

 
   GURL url(TemplateURLRefToGURL(template_url->new_tab_url_ref(),
                                 kDisableStartMargin, false, false));
-  if (!url.is_valid() || !url.SchemeIsSecure())
+  if (!url.is_valid())
     return GURL(chrome::kChromeSearchLocalNtpUrl);
 
-  if (!IsURLAllowedForSupervisedUser(url, profile))
+  if (!url.SchemeIsSecure()) {

[Jered, 2014/01/30 17:13:12]

If you add a bucket NONE, then we can normalize as a fraction of total calls to
this function, and you can write
LocalNTPFallbackReason fallback = NONE;
if (cond1)
  fallback = R1;
else if (cond2)
  fallback = R2;
...
record(fallback);
if (fallback != NONE)
  return url;
return GURL(localntp);

[samarth, 2014/01/31 16:05:20]

I ended up redoing this code to make it simpler and clearer.  It also changes it
so that we only log when the rewrite is happening and not whenever
GetNewTabPageURL is called.

+    RecordLocalNTPFallbackReason(LOCAL_NTP_FALLBACK_INSECURE_URL);
     return GURL(chrome::kChromeSearchLocalNtpUrl);
+  }
+
+  if (!IsSuggestPrefEnabled(profile)) {
+    RecordLocalNTPFallbackReason(LOCAL_NTP_FALLBACK_SUGGEST_OFF);
+    return GURL(chrome::kChromeSearchLocalNtpUrl);
+  }
+
+  if (!IsURLAllowedForSupervisedUser(url, profile)) {
+    RecordLocalNTPFallbackReason(LOCAL_NTP_FALLBACK_BLOCKED_URL);
+    return GURL(chrome::kChromeSearchLocalNtpUrl);
+  }
 
   return url;
 }