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..e875d15e164393deeb3647e9b59a80fce1fefa7d 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,19 @@ 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::SampleETLDp1(g_browser_process->rappor_service(),
+                           "Settings.HomePage2",
+                           homepage_url);
     }
   }
 
+  GURL ntp_url = chrome::GetNewTabPageURL(profile_);
+  if (ntp_url.is_valid()) {
+    rappor::SampleETLDp1(g_browser_process->rappor_service(),
+                         "Settings.NewTabPageURL",
+                         ntp_url);
+  }
+
   int restore_on_startup = prefs_->GetInteger(prefs::kRestoreOnStartup);
   UMA_HISTOGRAM_ENUMERATION("Settings.StartupPageLoadSettings",
                             restore_on_startup, kSessionStartupPrefValueMax);
@@ -108,6 +112,9 @@ void PrefMetricsService::RecordLaunchPrefs() {
               "Settings.StartupPageEngineTypes",
               TemplateURLPrepopulateData::GetEngineType(start_url),
               SEARCH_ENGINE_MAX);
+          rappor::SampleETLDp1(g_browser_process->rappor_service(),

[Alexei Svitkine (slow), 2014/06/05 14:23:35]

Is this doing really what you intend?

It looks like this will be logged multiple times, as it's iterating over
multiple URLs.

Additionally, it looks to be done only in the "restore on start up based on
prefs" case, which I'm not sure the meaning of. I guess an owner of this code
should review it, since I don't have enough understanding of it.

[Steven Holte, 2014/06/05 20:17:07]

My assumption is that this is when chrome://settings/ "On startup" is set to
"Open a specific page or set of pages", and we'd collect a sample for each page,
though we'll only generate one report, with more bits set.

On 2014/06/05 14:23:35, Alexei Svitkine wrote:

[Alexei Svitkine (slow), 2014/06/05 21:05:23]

Ah, that makes sense. Please verify your assumption with an owner, though.

+                               "Settings.StartupPage",

[Alexei Svitkine (slow), 2014/06/05 14:23:35]

Not in this CL, but now that we're getting a non-trivial number of metrics, we
should add a rappor.xml file to list them, describe them and specify owners.

+                               start_url);
         }
       }
     }