Add more Rappor metrics

chrome/browser/prefs/pref_metrics_service.cc

Index: chrome/browser/prefs/pref_metrics_service.cc
diff --git a/chrome/browser/prefs/pref_metrics_service.cc b/chrome/browser/prefs/pref_metrics_service.cc
index 888655497304d37a38ec618f75140ac56cb5c130..7f71e6ba354d26aa256a18a222c90b2f6773d6ce 100644
--- a/chrome/browser/prefs/pref_metrics_service.cc
+++ b/chrome/browser/prefs/pref_metrics_service.cc
@@ -17,12 +17,13 @@
 #include "chrome/browser/prefs/synced_pref_change_registrar.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url_prepopulate_data.h"
 #include "chrome/browser/ui/tabs/pinned_tab_codec.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
-#include "components/rappor/rappor_service.h"
+#include "components/rappor/rappor_utils.h"
 #include "crypto/hmac.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 
@@ -80,16 +81,20 @@ void PrefMetricsService::RecordLaunchPrefs() {
           "Settings.HomePageEngineType",
           TemplateURLPrepopulateData::GetEngineType(homepage_url),
           SEARCH_ENGINE_MAX);
-      if (g_browser_process->rappor_service()) {
-        g_browser_process->rappor_service()->RecordSample(
-            "Settings.HomePage2",
-            rappor::ETLD_PLUS_ONE_RAPPOR_TYPE,
-            net::registry_controlled_domains::GetDomainAndRegistry(homepage_url,
-                net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES));
-      }
+      rappor::SampleDomainAndRegistryFromGURL(
+          g_browser_process->rappor_service(),
+          "Settings.HomePage2",
+          homepage_url);
     }
   }
 
+  GURL ntp_url = chrome::GetNewTabPageURL(profile_);
+  if (ntp_url.is_valid()) {
+    rappor::SampleDomainAndRegistryFromGURL(g_browser_process->rappor_service(),
+                                            "Settings.NewTabPageURL",
+                                            ntp_url);

[battre, 2014/06/06 09:49:43]

Please check whether this does what you expect if an extension overrides the
NTP. You can try for example
https://chrome.google.com/webstore/detail/empty-new-tab-page/dpjamkmjmigaoobj...

[Steven Holte, 2014/06/06 21:00:50]

It does not.  It records whatever would have been displayed.  Any idea what I
could check to see if the NTP has been overriden?

+  }
+
   int restore_on_startup = prefs_->GetInteger(prefs::kRestoreOnStartup);
   UMA_HISTOGRAM_ENUMERATION("Settings.StartupPageLoadSettings",
                             restore_on_startup, kSessionStartupPrefValueMax);
@@ -108,6 +113,10 @@ void PrefMetricsService::RecordLaunchPrefs() {
               "Settings.StartupPageEngineTypes",
               TemplateURLPrepopulateData::GetEngineType(start_url),
               SEARCH_ENGINE_MAX);
+          rappor::SampleDomainAndRegistryFromGURL(
+              g_browser_process->rappor_service(),
+              "Settings.StartupPage",
+              start_url);

[battre, 2014/06/06 09:49:43]

Just be aware that you get >1 sample per user/profile. Does your pipeline
support this?

[Steven Holte, 2014/06/06 21:38:04]

I discussed this with Vasyl and Tyler, and it seems like the way we handle
multiple samples right now poses some problems for analysis. For now, I've
changed this to just record the first startup page, since that's the most
significant.

         }
       }
     }
@@ -165,7 +174,7 @@ void PrefMetricsService::OnPrefChanged(
       from_sync ? ".PulledFromSync" : ".PushedToSync");
   std::string histogram_name("Settings." + histogram_name_prefix + source_name);
   callback.Run(histogram_name, pref->GetValue());
-};
+}
 
 void PrefMetricsService::LogBooleanPrefChange(const std::string& histogram_name,
                                               const base::Value* value) {