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..316794fc26b064ecf82817d74c1e52e899a4b61a 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,15 @@ 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.

[Alexei Svitkine (slow), 2014/01/30 15:19:24]

Mention that this is used in an UMA macro, so new values should be added at the
end and existing values shouldn't be re-ordered or removed.

[samarth, 2014/01/30 16:45:29]

Done.

+enum LocalNTPFallbackReason {
+  LOCAL_NTP_FALLBACK_INSECURE_URL = 1,
+  LOCAL_NTP_FALLBACK_SUGGEST_OFF = 2,
+  LOCAL_NTP_FALLBACK_BLOCKED_URL = 3,
+  LOCAL_NTP_FALLBACK_MAX = 4

[Alexei Svitkine (slow), 2014/01/30 15:19:24]

Maybe better to remove the = 4 here, so that it automatically is set to the last
value + 1 (so that new values being added don't need to update it).

[samarth, 2014/01/30 16:45:29]

Done.

+};
+
 // Used to set the Instant support state of the Navigation entry.
 const char kInstantSupportStateKey[] = "instant_support_state";
 
@@ -454,20 +464,35 @@ 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);
 
   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()) {
+    UMA_HISTOGRAM_ENUMERATION("InstantExtended.LocalNTPFallbackReason",

[Alexei Svitkine (slow), 2014/01/30 15:19:24]

Can you make a small helper function in the anon namespace that wraps this macro
and takes just the enum value? That way, you don't need to repeat the string and
it's more efficient from code size perspective (the macro expands to a blob of
code, so having it in a single place avoids the duplication.)

[samarth, 2014/01/30 16:45:29]

Done.

+                              LOCAL_NTP_FALLBACK_INSECURE_URL,
+                              LOCAL_NTP_FALLBACK_MAX);
     return GURL(chrome::kChromeSearchLocalNtpUrl);
+  }
+
+  if (!IsSuggestPrefEnabled(profile)) {
+    UMA_HISTOGRAM_ENUMERATION("InstantExtended.LocalNTPFallbackReason",
+                              LOCAL_NTP_FALLBACK_SUGGEST_OFF,
+                              LOCAL_NTP_FALLBACK_MAX);
+    return GURL(chrome::kChromeSearchLocalNtpUrl);
+  }
+
+  if (!IsURLAllowedForSupervisedUser(url, profile)) {
+    UMA_HISTOGRAM_ENUMERATION("InstantExtended.LocalNTPFallbackReason",
+                              LOCAL_NTP_FALLBACK_BLOCKED_URL,
+                              LOCAL_NTP_FALLBACK_MAX);
+    return GURL(chrome::kChromeSearchLocalNtpUrl);
+  }
 
   return url;
 }