Add histograms to track synced pref changes.

chrome/browser/prefs/pref_metrics_service.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/prefs/pref_metrics_service.h"
 
 #include <map>
 
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
+#include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_prepopulate_data.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/common/pref_names.h"
 #include "components/browser_context_keyed_service/browser_context_dependency_manager.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 
 using TemplateURLPrepopulateData::kMaxPrepopulatedEngineID;
 
 namespace {
 
-// Use a map to convert domains to their histogram identifiers. Ids are defined
-// in tools/metrics/histograms/histograms.xml and (usually) also in
-// chrome/browser/search_engines/prepopulated_engines.json.
-typedef std::map<std::string, int> DomainIdMap;
-
 // Converts a host name into a domain name for easier matching.
-std::string GetDomain(const std::string& host) {
+std::string GetDomainFromHost(const std::string& host) {
   return net::registry_controlled_domains::GetDomainAndRegistry(
       host,
       net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES);
 }
 
-void AddDomain(const std::string& domain,
-               int domain_id,
-               DomainIdMap* domain_id_map) {
-  domain_id_map->insert(std::make_pair(domain, domain_id));
-}
-
-// Builds a map that associates domain name strings with histogram enum values,
-// for prepopulated DSEs and select non-prepopulated ones.
-void BuildDomainIdMap(Profile* profile, DomainIdMap* domain_id_map) {
-  // Add prepopulated search engine hosts to the map.
-  ScopedVector<TemplateURL> prepopulated_urls;
-  size_t default_search_index;  // unused
-  TemplateURLPrepopulateData::GetPrepopulatedEngines(profile,
-      &prepopulated_urls.get(), &default_search_index);
-  for (size_t i = 0; i < prepopulated_urls.size(); ++i) {
-    AddDomain(GetDomain(prepopulated_urls[i]->url_ref().GetHost()),
-              prepopulated_urls[i]->prepopulate_id(),
-              domain_id_map);
-  }
-  // Add some common search engine domains that are not prepopulated. Assign
-  // these domains id numbers 102-114 which extend the prepopulated engines
-  // histogram enum.
-  AddDomain("searchnu.com", 102, domain_id_map);
-  AddDomain("babylon.com", 103, domain_id_map);
-  AddDomain("delta-search.com", 104, domain_id_map);
-  AddDomain("iminent.com", 105, domain_id_map);
-  AddDomain("hao123.com", 106, domain_id_map);
-  AddDomain("sweetim.com", 107, domain_id_map);
-  AddDomain("snap.do", 108, domain_id_map);
-  AddDomain("snapdo.com", 109, domain_id_map);
-  AddDomain("softonic.com", 110, domain_id_map);
-  AddDomain("searchfunmoods.com", 111, domain_id_map);
-  AddDomain("incredibar.com", 112, domain_id_map);
-  AddDomain("sweetpacks.com", 113, domain_id_map);
-  AddDomain("imesh.net", 114, domain_id_map);
-  // IMPORTANT: If you add more domains here, be sure to update the
-  // kMaxPrepopulatedEngineID and available ids in prepopulated_engines.json.
-
-  // The following hosts may not be prepopulated, depending on the country
-  // settings. Add them here, using their existing ids. See histograms.xml.
-  AddDomain("conduit.com", 36, domain_id_map);
-  AddDomain("avg.com", 50, domain_id_map);
-  AddDomain("mail.ru", 83, domain_id_map);
-}
-
-// Maps a host name to a histogram enum value. The enum value '0' represents
-// 'Unknown', i.e. an unrecognized host.
-int MapHostToId(const DomainIdMap& domain_id_map, const std::string& host) {
-  DomainIdMap::const_iterator it = domain_id_map.find(GetDomain(host));
-  if (it != domain_id_map.end())
-    return it->second;
-  return 0;
-}
-
 }  // namespace
 
 PrefMetricsService::PrefMetricsService(Profile* profile)
     : profile_(profile) {
+  BuildDomainIdMap();
   RecordLaunchPrefs();
+
+  PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_);
+  synced_pref_change_registrar_.reset(new SyncedPrefChangeRegistrar(prefs));
+
+  RegisterSyncedPrefObservers();
 }
 
 PrefMetricsService::~PrefMetricsService() {
 }
 
 void PrefMetricsService::RecordLaunchPrefs() {
   PrefService* prefs = profile_->GetPrefs();
   const bool show_home_button = prefs->GetBoolean(prefs::kShowHomeButton);
   const bool home_page_is_ntp = prefs->GetBoolean(prefs::kHomePageIsNewTabPage);
 
   UMA_HISTOGRAM_BOOLEAN("Settings.ShowHomeButton", show_home_button);
   UMA_HISTOGRAM_BOOLEAN("Settings.HomePageIsNewTabPage", home_page_is_ntp);
 
   int restore_on_startup = profile_->GetPrefs()->GetInteger(
       prefs::kRestoreOnStartup);
   UMA_HISTOGRAM_ENUMERATION("Settings.StartupPageLoadSettings",
       restore_on_startup, SessionStartupPref::kPrefValueMax);
   if (restore_on_startup == SessionStartupPref::kPrefValueURLs) {
     const int url_list_size = profile_->GetPrefs()->GetList(
         prefs::kURLsToRestoreOnStartup)->GetSize();
     UMA_HISTOGRAM_CUSTOM_COUNTS(
         "Settings.StartupPageLoadURLs", url_list_size, 1, 50, 20);
   }
 
-  DomainIdMap domain_id_map;
-  BuildDomainIdMap(profile_, &domain_id_map);
-
   // Record the default search engine id.
   TemplateURLService* template_url_service =
       TemplateURLServiceFactory::GetForProfile(profile_);
   if (template_url_service) {
     TemplateURL* template_url =
         template_url_service->GetDefaultSearchProvider();
     if (template_url) {
-      const int domain_id =
-          MapHostToId(domain_id_map, template_url->url_ref().GetHost());
+      const int domain_id = MapHostToId(template_url->url_ref().GetHost());
       UMA_HISTOGRAM_ENUMERATION("Settings.DefaultSearchProvider",
                                 domain_id, kMaxPrepopulatedEngineID);
     }
   }
   // If the home page isn't the NTP, record the home page domain id.
   if (!home_page_is_ntp) {
     GURL homepage_url(prefs->GetString(prefs::kHomePage));
     if (homepage_url.is_valid()) {
-      const int domain_id = MapHostToId(domain_id_map, homepage_url.host());
+      const int domain_id = MapHostToId(homepage_url.host());
       UMA_HISTOGRAM_ENUMERATION("Settings.HomePageDomain",
                                 domain_id, kMaxPrepopulatedEngineID);
     }
   }
   // If startup pages are set, record all startup page domain ids.
   if (restore_on_startup == SessionStartupPref::kPrefValueURLs) {
     const ListValue* url_list = prefs->GetList(prefs::kURLsToRestoreOnStartup);
     for (size_t i = 0; i < url_list->GetSize(); i++) {
       std::string url_text;
       if (url_list->GetString(i, &url_text)) {
         GURL start_url(url_text);
         if (start_url.is_valid()) {
-          const int domain_id = MapHostToId(domain_id_map, start_url.host());
+          const int domain_id = MapHostToId(start_url.host());
           UMA_HISTOGRAM_ENUMERATION("Settings.StartupPageDomains",
                                     domain_id, kMaxPrepopulatedEngineID);
         }
       }
     }
   }
 }
 
+void PrefMetricsService::RegisterSyncedPrefObservers() {
+  ValueHistogramCallback booleanHandler = base::Bind(
+      &PrefMetricsService::LogBooleanPrefChange, base::Unretained(this));
+  ValueHistogramCallback urlHandler = base::Bind(
+      &PrefMetricsService::LogUrlPrefChange, base::Unretained(this));
+
+  AddPrefObserver(prefs::kShowHomeButton, "ShowHomeButton", booleanHandler);
+  AddPrefObserver(prefs::kHomePageIsNewTabPage, "HomePageIsNewTabPage",
+                  booleanHandler);
+  AddPrefObserver(prefs::kHomePage, "HomePageDomain", urlHandler);
+
+  const int startupPrefValueMax = SessionStartupPref::kPrefValueMax;
+  AddPrefObserver(prefs::kRestoreOnStartup, "StartupPageLoadSettings",
+                  base::Bind(&PrefMetricsService::LogIntegerPrefChange,
+                             base::Unretained(this),
+                             startupPrefValueMax));

[battre, 2013/08/06 15:54:39]

nit: inline startupPrefValueMax?

[Ken Rockot(use gerrit already), 2013/08/06 20:18:32]

Done.

+
+  AddPrefObserver(prefs::kURLsToRestoreOnStartup, "StartupPageDomains",
+                  base::Bind(&PrefMetricsService::LogListPrefChange,
+                             base::Unretained(this),
+                             urlHandler));
+}
+
+void PrefMetricsService::AddPrefObserver(
+    const std::string& path,
+    const std::string& histogram_base,

[battre, 2013/08/06 15:54:39]

nit: histogram_name_prefix?

[Ken Rockot(use gerrit already), 2013/08/06 20:18:32]

Done.

+    const ValueHistogramCallback& callback) {
+  synced_pref_change_registrar_->Add(path.c_str(),
+      base::Bind(&PrefMetricsService::OnPrefChanged,
+                 base::Unretained(this),
+                 histogram_base, callback));
+}
+
+void PrefMetricsService::BuildDomainIdMap() {
+  // Add prepopulated search engine hosts to the map.
+  ScopedVector<TemplateURL> prepopulated_urls;
+  size_t default_search_index;  // unused
+  TemplateURLPrepopulateData::GetPrepopulatedEngines(profile_,
+      &prepopulated_urls.get(), &default_search_index);
+  for (size_t i = 0; i < prepopulated_urls.size(); ++i) {
+    AddToDomainIdMap(
+        GetDomainFromHost(prepopulated_urls[i]->url_ref().GetHost()),
+        prepopulated_urls[i]->prepopulate_id());
+  }
+  // Add some common search engine domains that are not prepopulated. Assign
+  // these domains id numbers 102-114 which extend the prepopulated engines
+  // histogram enum.
+  AddToDomainIdMap("searchnu.com", 102);
+  AddToDomainIdMap("babylon.com", 103);
+  AddToDomainIdMap("delta-search.com", 104);
+  AddToDomainIdMap("iminent.com", 105);
+  AddToDomainIdMap("hao123.com", 106);
+  AddToDomainIdMap("sweetim.com", 107);
+  AddToDomainIdMap("snap.do", 108);
+  AddToDomainIdMap("snapdo.com", 109);
+  AddToDomainIdMap("softonic.com", 110);
+  AddToDomainIdMap("searchfunmoods.com", 111);
+  AddToDomainIdMap("incredibar.com", 112);
+  AddToDomainIdMap("sweetpacks.com", 113);
+  AddToDomainIdMap("imesh.net", 114);
+  // IMPORTANT: If you add more domains here, be sure to update the
+  // kMaxPrepopulatedEngineID and available ids in prepopulated_engines.json.
+
+  // The following hosts may not be prepopulated, depending on the country
+  // settings. Add them here, using their existing ids. See histograms.xml.
+  AddToDomainIdMap("conduit.com", 36);
+  AddToDomainIdMap("avg.com", 50);
+  AddToDomainIdMap("mail.ru", 83);
+}
+
+void PrefMetricsService::AddToDomainIdMap(const std::string& domain,
+    int domain_id) {

[battre, 2013/08/06 15:54:39]

nit: align parameters.

+  domain_id_map_.insert(std::make_pair(domain, domain_id));
+}
+
+int PrefMetricsService::MapHostToId(const std::string& host) const {
+  DomainIdMap::const_iterator it = domain_id_map_.find(
+      GetDomainFromHost(host));
+  if (it != domain_id_map_.end())
+    return it->second;
+  return 0;
+}
+
+void PrefMetricsService::OnPrefChanged(const std::string& histogram_base,
+                                       const ValueHistogramCallback& callback,
+                                       const std::string& path,
+                                       bool from_sync) {
+  PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_);
+  const PrefService::Preference* pref = prefs->FindPreference(path.c_str());
+  DCHECK(pref);
+  std::string source_name(
+      from_sync ? ".PulledFromSync" : ".PushedToSync");
+  std::string histogram_name("Settings." + histogram_base + source_name);
+  callback.Run(histogram_name, pref->GetValue());

[battre, 2013/08/06 15:54:39]

This may be a bug. pref->GetValue() does not necessary return the value we got
from sync. It may return the value that is configured by a policy or extension.

[Ken Rockot(use gerrit already), 2013/08/06 16:27:46]

I think it's correct as-is, but can you elaborate? A value changed by policy
should never invoke this callback, because
PrefModelAssociator::ProcessPrefChange is cut off before notifying observers in
the case where the pref !IsUserModifiable(). I'm not sure about values
configured by extension - is PrefModelAssociator aware of the distinction?

[battre, 2013/08/07 07:22:39]

This is what I had in mind.

A preference is controlled by policy. Now Sync updates the preference and
triggers the notification. We check the current value to report to UMA and won't
get the Sync value but the policy value.

+};
+
+void PrefMetricsService::LogBooleanPrefChange(const std::string& histogram_name,
+                                              const Value* value) {
+  bool boolean_value = false;
+  if (!value->GetAsBoolean(&boolean_value))
+    return;
+  UMA_HISTOGRAM_BOOLEAN(histogram_name.c_str(), boolean_value);
+}
+
+void PrefMetricsService::LogIntegerPrefChange(int boundary_value,
+                                              const std::string& histogram_name,
+                                              const Value* value) {
+  int integer_value = 0;
+  if (!value->GetAsInteger(&integer_value))
+    return;
+  UMA_HISTOGRAM_ENUMERATION(histogram_name.c_str(), integer_value,
+                            boundary_value);
+}
+
+void PrefMetricsService::LogUrlPrefChange(const std::string& histogram_name,
+                                          const Value* value) {
+  std::string string_value;
+  if (!value->GetAsString(&string_value))
+    return;
+  GURL url(string_value);
+  if (url.is_valid()) {
+    int domain_id = MapHostToId(url.host());
+    UMA_HISTOGRAM_ENUMERATION(histogram_name.c_str(), domain_id,
+                              kMaxPrepopulatedEngineID);
+  }
+}
+
+void PrefMetricsService::LogListPrefChange(
+    const ValueHistogramCallback& item_callback,
+    const std::string& histogram_name,
+    const Value* value) {
+  const ListValue* items = NULL;
+  if (!value->GetAsList(&items))
+    return;
+  for (size_t i = 0; i < items->GetSize(); ++i) {
+    const Value *item_value = NULL;
+    if (items->Get(i, &item_value)) {
+      item_callback.Run(histogram_name, item_value);
+    }

[battre, 2013/08/06 15:54:39]

nit: no {}

[Ken Rockot(use gerrit already), 2013/08/06 20:18:32]

Done.

+  }
+}
+
 // static
 PrefMetricsService::Factory* PrefMetricsService::Factory::GetInstance() {
   return Singleton<PrefMetricsService::Factory>::get();
 }
 
 // static
 PrefMetricsService* PrefMetricsService::Factory::GetForProfile(
     Profile* profile) {
   return static_cast<PrefMetricsService*>(
       GetInstance()->GetServiceForBrowserContext(profile, true));
 }
 
 PrefMetricsService::Factory::Factory()
     : BrowserContextKeyedServiceFactory(
         "PrefMetricsService",
         BrowserContextDependencyManager::GetInstance()) {
   DependsOn(TemplateURLServiceFactory::GetInstance());
+  DependsOn(ProfileSyncServiceFactory::GetInstance());
 }
 
 PrefMetricsService::Factory::~Factory() {
 }
 
 BrowserContextKeyedService*
 PrefMetricsService::Factory::BuildServiceInstanceFor(
     content::BrowserContext* profile) const {
   return new PrefMetricsService(static_cast<Profile*>(profile));
 }
 
 bool PrefMetricsService::Factory::ServiceIsCreatedWithBrowserContext() const {
   return true;
 }
 
 bool PrefMetricsService::Factory::ServiceIsNULLWhileTesting() const {
   return false;
 }
 
 content::BrowserContext* PrefMetricsService::Factory::GetBrowserContextToUse(
     content::BrowserContext* context) const {
   return chrome::GetBrowserContextRedirectedInIncognito(context);
 }