diff options
author | vitalybuka@chromium.org <vitalybuka@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-09 04:06:23 +0000 |
---|---|---|
committer | vitalybuka@chromium.org <vitalybuka@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-09-09 04:06:23 +0000 |
commit | 8456becde4132cdf2ef9b12217cc23b97aebb895 (patch) | |
tree | f31a6f083450db5a82313b607588e4065f3bf54b | |
parent | 507f5b062c149dc48e279f86a37c1fb0d9292b5f (diff) | |
download | chromium_src-8456becde4132cdf2ef9b12217cc23b97aebb895.zip chromium_src-8456becde4132cdf2ef9b12217cc23b97aebb895.tar.gz chromium_src-8456becde4132cdf2ef9b12217cc23b97aebb895.tar.bz2 |
Revert 221966 "Revert 221944 "Add UMA to report Preferences File..."
Re-apply and disable for Chrome Frame.
> Revert 221944 "Add UMA to report Preferences File Corruption"
>
> Breaks chrome_frame_tests on Win Aura.
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20Chromium&testType=chrome_frame_tests&builder=Win%20Aura%20Tests%20(1)
>
> > Add UMA to report Preferences File Corruption
> > Adds PrefMetricsService methods to track changes to user preferences
> > that happen outside of Chrome and report discrepancies via UMA.
> >
> > BUG=266569
> >
> > Review URL: https://chromiumcodereview.appspot.com/22676002
>
> TBR=bbudge@chromium.org
>
> Review URL: https://codereview.chromium.org/23597020
TBR=bbudge@chromium.org
Review URL: https://codereview.chromium.org/23788010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221972 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/browser_resources.grd | 3 | ||||
-rw-r--r-- | chrome/browser/prefs/browser_prefs.cc | 2 | ||||
-rw-r--r-- | chrome/browser/prefs/pref_metrics_service.cc | 242 | ||||
-rw-r--r-- | chrome/browser/prefs/pref_metrics_service.h | 56 | ||||
-rw-r--r-- | chrome/browser/prefs/pref_metrics_service_unittest.cc | 274 | ||||
-rw-r--r-- | chrome/chrome_tests_unit.gypi | 1 | ||||
-rw-r--r-- | chrome/common/pref_names.cc | 5 | ||||
-rw-r--r-- | chrome/common/pref_names.h | 2 | ||||
-rw-r--r-- | tools/metrics/histograms/histograms.xml | 45 |
9 files changed, 612 insertions, 18 deletions
diff --git a/chrome/browser/browser_resources.grd b/chrome/browser/browser_resources.grd index 5c441ba..448b7c4 100644 --- a/chrome/browser/browser_resources.grd +++ b/chrome/browser/browser_resources.grd @@ -378,6 +378,9 @@ <include name="IDR_NETWORK_JS" file="resources\chromeos\network.js" type="BINDATA" /> <include name="IDR_NETWORK_CSS" file="resources\chromeos\network.css" type="BINDATA" /> </if> + <if expr="pp_ifdef('_google_chrome')"> + <include name="IDR_PREF_HASH_SEED_BIN" file="resources\settings_internal\pref_hash_seed.bin" type="BINDATA" /> + </if> </includes> </release> </grit> diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc index 5d4c679..990249f 100644 --- a/chrome/browser/prefs/browser_prefs.cc +++ b/chrome/browser/prefs/browser_prefs.cc @@ -52,6 +52,7 @@ #include "chrome/browser/pepper_flash_settings_manager.h" #include "chrome/browser/plugins/plugin_finder.h" #include "chrome/browser/prefs/incognito_mode_prefs.h" +#include "chrome/browser/prefs/pref_metrics_service.h" #include "chrome/browser/prefs/pref_service_syncable.h" #include "chrome/browser/prefs/session_startup_pref.h" #include "chrome/browser/printing/cloud_print/cloud_print_url.h" @@ -217,6 +218,7 @@ void RegisterLocalState(PrefRegistrySimple* registry) { MetricsLog::RegisterPrefs(registry); MetricsService::RegisterPrefs(registry); metrics::CachingPermutedEntropyProvider::RegisterPrefs(registry); + PrefMetricsService::RegisterPrefs(registry); PrefProxyConfigTrackerImpl::RegisterPrefs(registry); ProfileInfoCache::RegisterPrefs(registry); profiles::RegisterPrefs(registry); diff --git a/chrome/browser/prefs/pref_metrics_service.cc b/chrome/browser/prefs/pref_metrics_service.cc index 3814032..9081706 100644 --- a/chrome/browser/prefs/pref_metrics_service.cc +++ b/chrome/browser/prefs/pref_metrics_service.cc @@ -5,48 +5,119 @@ #include "chrome/browser/prefs/pref_metrics_service.h" #include "base/bind.h" +#include "base/command_line.h" +#include "base/json/json_string_value_serializer.h" #include "base/metrics/histogram.h" +#include "base/prefs/pref_registry_simple.h" #include "base/prefs/pref_service.h" +#include "base/strings/string_number_conversions.h" +#include "chrome/browser/browser_process.h" +#include "chrome/browser/browser_shutdown.h" +// Accessing the Device ID API here is a layering violation. +// TODO(bbudge) Move the API so it's usable here. +// http://crbug.com/276485 +#include "chrome/browser/extensions/api/music_manager_private/device_id.h" +#include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/prefs/pref_service_syncable.h" +#include "chrome/browser/prefs/scoped_user_pref_update.h" #include "chrome/browser/prefs/session_startup_pref.h" #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_engines/template_url_prepopulate_data.h" +#include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" #include "components/browser_context_keyed_service/browser_context_dependency_manager.h" +#include "crypto/hmac.h" +#include "grit/browser_resources.h" #include "net/base/registry_controlled_domains/registry_controlled_domain.h" +#include "ui/base/resource/resource_bundle.h" namespace { -// Converts a host name into a domain name for easier matching. -std::string GetDomainFromHost(const std::string& host) { - return net::registry_controlled_domains::GetDomainAndRegistry( - host, - net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES); -} - const int kSessionStartupPrefValueMax = SessionStartupPref::kPrefValueMax; +// These preferences must be kept in sync with the TrackedPreference enum in +// tools/metrics/histograms/histograms.xml. To add a new preference, append it +// to the array and add a corresponding value to the histogram enum. +const char* kTrackedPrefs[] = { + prefs::kShowHomeButton, + prefs::kHomePageIsNewTabPage, + prefs::kHomePage, + prefs::kRestoreOnStartup, + prefs::kURLsToRestoreOnStartup, + prefs::kExtensionsPref, + prefs::kGoogleServicesLastUsername, + prefs::kSearchProviderOverrides, + prefs::kDefaultSearchProviderSearchURL, + prefs::kDefaultSearchProviderKeyword, + prefs::kDefaultSearchProviderName, +#if !defined(OS_ANDROID) + prefs::kPinnedTabs, +#endif +}; + +static const size_t kSHA256DigestSize = 32; + } // namespace PrefMetricsService::PrefMetricsService(Profile* profile) - : profile_(profile) { + : profile_(profile), + prefs_(profile_->GetPrefs()), + local_state_(g_browser_process->local_state()), + profile_name_(profile_->GetPath().AsUTF8Unsafe()), + tracked_pref_paths_(kTrackedPrefs), + tracked_pref_path_count_(arraysize(kTrackedPrefs)), + checked_tracked_prefs_(false), + weak_factory_(this) { + pref_hash_seed_ = ResourceBundle::GetSharedInstance().GetRawDataResource( + IDR_PREF_HASH_SEED_BIN).as_string(); + RecordLaunchPrefs(); PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_); synced_pref_change_registrar_.reset(new SyncedPrefChangeRegistrar(prefs)); RegisterSyncedPrefObservers(); + + // The following code might cause callbacks into this instance before we exit + // the constructor. This instance should be initialized at this point. + + // Android has no GetDeviceId. +#if !defined(OS_ANDROID) + // We need the machine id to compute pref value hashes. Fetch that, and then + // call CheckTrackedPreferences in the callback. + extensions::api::DeviceId::GetDeviceId( + "PrefMetricsService", // non-empty string to obfuscate the device id. + Bind(&PrefMetricsService::GetDeviceIdCallback, + weak_factory_.GetWeakPtr())); +#endif // !defined(OS_ANDROID) +} + +// For unit testing only. +PrefMetricsService::PrefMetricsService(Profile* profile, + PrefService* local_state, + const std::string& device_id, + const char** tracked_pref_paths, + int tracked_pref_path_count) + : profile_(profile), + prefs_(profile->GetPrefs()), + local_state_(local_state), + pref_hash_seed_(kSHA256DigestSize, 0), + device_id_(device_id), + tracked_pref_paths_(tracked_pref_paths), + tracked_pref_path_count_(tracked_pref_path_count), + checked_tracked_prefs_(false), + weak_factory_(this) { + CheckTrackedPreferences(); } PrefMetricsService::~PrefMetricsService() { } void PrefMetricsService::RecordLaunchPrefs() { - PrefService* prefs = profile_->GetPrefs(); - bool show_home_button = prefs->GetBoolean(prefs::kShowHomeButton); - bool home_page_is_ntp = prefs->GetBoolean(prefs::kHomePageIsNewTabPage); + bool show_home_button = prefs_->GetBoolean(prefs::kShowHomeButton); + bool home_page_is_ntp = prefs_->GetBoolean(prefs::kHomePageIsNewTabPage); UMA_HISTOGRAM_BOOLEAN("Settings.ShowHomeButton", show_home_button); if (show_home_button) { UMA_HISTOGRAM_BOOLEAN("Settings.GivenShowHomeButton_HomePageIsNewTabPage", @@ -60,7 +131,7 @@ void PrefMetricsService::RecordLaunchPrefs() { // (pages that share a TLD+1 with a known engine but aren't actually search // pages, e.g. plus.google.com). if (!home_page_is_ntp) { - GURL homepage_url(prefs->GetString(prefs::kHomePage)); + GURL homepage_url(prefs_->GetString(prefs::kHomePage)); if (homepage_url.is_valid()) { UMA_HISTOGRAM_ENUMERATION( "Settings.HomePageEngineType", @@ -69,11 +140,11 @@ void PrefMetricsService::RecordLaunchPrefs() { } } - int restore_on_startup = prefs->GetInteger(prefs::kRestoreOnStartup); + int restore_on_startup = prefs_->GetInteger(prefs::kRestoreOnStartup); UMA_HISTOGRAM_ENUMERATION("Settings.StartupPageLoadSettings", restore_on_startup, kSessionStartupPrefValueMax); if (restore_on_startup == SessionStartupPref::kPrefValueURLs) { - const ListValue* url_list = prefs->GetList(prefs::kURLsToRestoreOnStartup); + const ListValue* url_list = prefs_->GetList(prefs::kURLsToRestoreOnStartup); UMA_HISTOGRAM_CUSTOM_COUNTS("Settings.StartupPageLoadURLs", url_list->GetSize(), 1, 50, 20); // Similarly, check startup pages for known search engine TLD+1s. @@ -92,6 +163,13 @@ void PrefMetricsService::RecordLaunchPrefs() { } } +// static +void PrefMetricsService::RegisterPrefs(PrefRegistrySimple* registry) { + // Register the top level dictionary to map profile names to dictionaries of + // tracked preferences. + registry->RegisterDictionaryPref(prefs::kProfilePreferenceHashes); +} + void PrefMetricsService::RegisterSyncedPrefObservers() { LogHistogramValueCallback booleanHandler = base::Bind( &PrefMetricsService::LogBooleanPrefChange, base::Unretained(this)); @@ -169,6 +247,142 @@ void PrefMetricsService::LogListPrefChange( } } +void PrefMetricsService::GetDeviceIdCallback(const std::string& device_id) { + device_id_ = device_id; + // On Aura, this seems to be called twice. + if (!checked_tracked_prefs_) + CheckTrackedPreferences(); +} + +// To detect changes to Preferences that happen outside of Chrome, we hash +// selected pref values and save them in local state. CheckTrackedPreferences +// compares the saved values to the values observed in the profile's prefs. A +// dictionary of dictionaries in local state holds the hashed values, grouped by +// profile. To make the system more resistant to spoofing, pref values are +// hashed with the pref path and the device id. +void PrefMetricsService::CheckTrackedPreferences() { + DCHECK(!checked_tracked_prefs_); + + const base::DictionaryValue* pref_hash_dicts = + local_state_->GetDictionary(prefs::kProfilePreferenceHashes); + // Get the hashed prefs dictionary if it exists. If it doesn't, it will be + // created if we set preference values below. + const base::DictionaryValue* hashed_prefs = NULL; + pref_hash_dicts->GetDictionary(profile_name_, &hashed_prefs); + for (int i = 0; i < tracked_pref_path_count_; ++i) { + // Skip prefs that haven't been registered. + if (!prefs_->FindPreference(tracked_pref_paths_[i])) + continue; + + bool changed = false; + const base::Value* value = prefs_->GetUserPrefValue(tracked_pref_paths_[i]); + if (value) { + std::string value_hash = + GetHashedPrefValue(tracked_pref_paths_[i], value); + std::string last_hash; + if (hashed_prefs && + hashed_prefs->GetString(tracked_pref_paths_[i], &last_hash)) { + if (value_hash != last_hash) { + changed = true; + // Record that the preference changed from its last value. + UMA_HISTOGRAM_ENUMERATION("Settings.TrackedPreferenceChanged", + i, tracked_pref_path_count_); + UpdateTrackedPreference(tracked_pref_paths_[i]); + } + } else { + changed = true; + // Record that we haven't tracked this preference yet, or the hash in + // local state was removed. + UMA_HISTOGRAM_ENUMERATION("Settings.TrackedPreferenceInitialized", + i, tracked_pref_path_count_); + UpdateTrackedPreference(tracked_pref_paths_[i]); + } + } else { + // There is no preference set. Remove any hashed value from local state + // and if one was present, record that a pref was cleared. + if (RemoveTrackedPreference(tracked_pref_paths_[i])) { + changed = true; + UMA_HISTOGRAM_ENUMERATION("Settings.TrackedPreferenceCleared", + i, tracked_pref_path_count_); + } + } + if (!changed) { + UMA_HISTOGRAM_ENUMERATION("Settings.TrackedPreferenceUnchanged", + i, tracked_pref_path_count_); + } + } + + checked_tracked_prefs_ = true; + + // Now that we've checked the incoming preferences, register for change + // notifications, unless this is test code. + // TODO(bbudge) Fix failing browser_tests and so we can remove this test. + // Several tests fail when they shutdown before they can write local state. + if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType) && + !CommandLine::ForCurrentProcess()->HasSwitch(switches::kChromeFrame)) { + InitializePrefObservers(); + } +} + +void PrefMetricsService::UpdateTrackedPreference(const char* path) { + const base::Value* value = prefs_->GetUserPrefValue(path); + if (value) { + DictionaryPrefUpdate update(local_state_, prefs::kProfilePreferenceHashes); + update->SetString(GetHashedPrefPath(path), + GetHashedPrefValue(path, value)); + } else { + RemoveTrackedPreference(path); + } +} + +bool PrefMetricsService::RemoveTrackedPreference(const char* path) { + DictionaryPrefUpdate update(local_state_, prefs::kProfilePreferenceHashes); + return update->Remove(GetHashedPrefPath(path), NULL); +} + +std::string PrefMetricsService::GetHashedPrefPath(const char* path) { + std::string hash_pref_path(profile_name_); + hash_pref_path.append("."); + hash_pref_path.append(path); + return hash_pref_path; +} + +std::string PrefMetricsService::GetHashedPrefValue( + const char* path, + const base::Value* value) { + DCHECK(value); + + std::string string_to_hash(device_id_); + string_to_hash.append(path); + JSONStringValueSerializer serializer(&string_to_hash); + serializer.Serialize(*value); + + crypto::HMAC hmac(crypto::HMAC::SHA256); + unsigned char digest[kSHA256DigestSize]; + if (!hmac.Init(pref_hash_seed_) || + !hmac.Sign(string_to_hash, digest, kSHA256DigestSize)) { + NOTREACHED(); + return std::string(); + } + + return base::HexEncode(digest, kSHA256DigestSize); +} + +void PrefMetricsService::InitializePrefObservers() { + pref_registrar_.Init(prefs_); + for (int i = 0; i < tracked_pref_path_count_; ++i) { + // Skip prefs that haven't been registered. + if (!prefs_->FindPreference(tracked_pref_paths_[i])) + continue; + + pref_registrar_.Add( + tracked_pref_paths_[i], + base::Bind(&PrefMetricsService::UpdateTrackedPreference, + weak_factory_.GetWeakPtr(), + tracked_pref_paths_[i])); + } +} + // static PrefMetricsService::Factory* PrefMetricsService::Factory::GetInstance() { return Singleton<PrefMetricsService::Factory>::get(); diff --git a/chrome/browser/prefs/pref_metrics_service.h b/chrome/browser/prefs/pref_metrics_service.h index fc48935..b2657c1 100644 --- a/chrome/browser/prefs/pref_metrics_service.h +++ b/chrome/browser/prefs/pref_metrics_service.h @@ -10,11 +10,15 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/singleton.h" +#include "base/memory/weak_ptr.h" +#include "base/prefs/pref_change_registrar.h" #include "chrome/browser/prefs/synced_pref_change_registrar.h" #include "chrome/browser/profiles/profile.h" #include "components/browser_context_keyed_service/browser_context_keyed_service.h" #include "components/browser_context_keyed_service/browser_context_keyed_service_factory.h" +class PrefRegistrySimple; + // PrefMetricsService is responsible for recording prefs-related UMA stats. class PrefMetricsService : public BrowserContextKeyedService { public: @@ -40,16 +44,23 @@ class PrefMetricsService : public BrowserContextKeyedService { content::BrowserContext* context) const OVERRIDE; }; + // Registers preferences in local state. + static void RegisterPrefs(PrefRegistrySimple* registry); + private: - // 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; + friend class PrefMetricsServiceTest; // Function to log a Value to a histogram typedef base::Callback<void(const std::string&, const Value*)> LogHistogramValueCallback; + // For unit testing only. + PrefMetricsService(Profile* profile, + PrefService* local_settings, + const std::string& device_id, + const char** tracked_pref_paths, + int tracked_pref_path_count); + // Record prefs state on browser context creation. void RecordLaunchPrefs(); @@ -82,8 +93,45 @@ class PrefMetricsService : public BrowserContextKeyedService { const std::string& histogram_name, const Value* value); + // Callback to receive a unique device_id. + void GetDeviceIdCallback(const std::string& device_id); + + // Checks the tracked preferences against their last known values and reports + // any discrepancies. This must be called after |device_id| has been set. + void CheckTrackedPreferences(); + + // Updates the hash of the tracked preference in local state. This must be + // called after |device_id| has been set. + void UpdateTrackedPreference(const char* path); + + // Removes the tracked preference from local state. Returns 'true' iff. the + // value was present. + bool RemoveTrackedPreference(const char* path); + + // Gets the path to the preference value hash in local state. + std::string GetHashedPrefPath(const char* path); + + // Computes an MD5 hash for the given preference value. + std::string GetHashedPrefValue(const char* path, const base::Value* value); + + void InitializePrefObservers(); + Profile* profile_; + PrefService* prefs_; + PrefService* local_state_; + std::string profile_name_; + std::string pref_hash_seed_; + std::string device_id_; + const char** tracked_pref_paths_; + const int tracked_pref_path_count_; + bool checked_tracked_prefs_; + + PrefChangeRegistrar pref_registrar_; scoped_ptr<SyncedPrefChangeRegistrar> synced_pref_change_registrar_; + + base::WeakPtrFactory<PrefMetricsService> weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(PrefMetricsService); }; #endif // CHROME_BROWSER_PREFS_PREF_METRICS_SERVICE_H_ diff --git a/chrome/browser/prefs/pref_metrics_service_unittest.cc b/chrome/browser/prefs/pref_metrics_service_unittest.cc new file mode 100644 index 0000000..9e05c49 --- /dev/null +++ b/chrome/browser/prefs/pref_metrics_service_unittest.cc @@ -0,0 +1,274 @@ +// 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 "base/memory/scoped_ptr.h" +#include "base/metrics/histogram.h" +#include "base/metrics/statistics_recorder.h" +#include "base/prefs/testing_pref_service.h" +#include "base/values.h" +#include "chrome/browser/prefs/pref_metrics_service.h" +#include "chrome/browser/prefs/scoped_user_pref_update.h" +#include "chrome/test/base/testing_pref_service_syncable.h" +#include "chrome/test/base/testing_profile.h" +#include "components/user_prefs/pref_registry_syncable.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// TestingProfile may register some real preferences; to avoid interference, +// define fake preferences for testing. +const char* kTrackedPrefs[] = { + "pref_metrics_service_test.pref1", + "pref_metrics_service_test.pref2", +}; + +const int kTrackedPrefCount = arraysize(kTrackedPrefs); + +} // namespace + +class PrefMetricsServiceTest : public testing::Test { + protected: + virtual void SetUp() { + pref1_changed_ = 0; + pref2_changed_ = 0; + pref1_cleared_ = 0; + pref2_cleared_ = 0; + pref1_initialized_ = 0; + pref2_initialized_ = 0; + pref1_unchanged_ = 0; + pref2_unchanged_ = 0; + + base::StatisticsRecorder::Initialize(); + + prefs_ = profile_.GetTestingPrefService(); + + // Register our test-only tracked prefs as string values. + for (int i = 0; i < kTrackedPrefCount; ++i) { + prefs_->registry()->RegisterStringPref( + kTrackedPrefs[i], + "test_default_value", + user_prefs::PrefRegistrySyncable::SYNCABLE_PREF); + } + + // Initialize pref in local state that holds hashed values. + PrefMetricsService::RegisterPrefs(local_state_.registry()); + + // Update global counts in case another test left stray samples. + UpdateHistogramSamples(); + } + + scoped_ptr<PrefMetricsService> CreatePrefMetricsService() { + return scoped_ptr<PrefMetricsService>( + new PrefMetricsService(&profile_, + &local_state_, + "test_device_id", + kTrackedPrefs, + kTrackedPrefCount)); + } + + std::string GetHashedPrefValue(PrefMetricsService* service, + const char* path, base::Value* value) { + return service->GetHashedPrefValue(path, value); + } + + void GetSamples(const char* histogram_name, int* bucket1, int* bucket2) { + base::HistogramBase* histogram = + base::StatisticsRecorder::FindHistogram(histogram_name); + if (!histogram) { + *bucket1 = 0; + *bucket2 = 0; + } else { + scoped_ptr<base::HistogramSamples> samples(histogram->SnapshotSamples()); + *bucket1 = samples->GetCount(0); + *bucket2 = samples->GetCount(1); + } + } + + void UpdateHistogramSamples() { + int changed1, changed2; + GetSamples("Settings.TrackedPreferenceChanged", &changed1, &changed2); + pref1_changed_ = changed1 - pref1_changed_total; + pref2_changed_ = changed2 - pref2_changed_total; + pref1_changed_total = changed1; + pref2_changed_total = changed2; + + int cleared1, cleared2; + GetSamples("Settings.TrackedPreferenceCleared", &cleared1, &cleared2); + pref1_cleared_ = cleared1 - pref1_cleared_total; + pref2_cleared_ = cleared2 - pref2_cleared_total; + pref1_cleared_total = cleared1; + pref2_cleared_total = cleared2; + + int inited1, inited2; + GetSamples("Settings.TrackedPreferenceInitialized", &inited1, &inited2); + pref1_initialized_ = inited1 - pref1_initialized_total; + pref2_initialized_ = inited2 - pref2_initialized_total; + pref1_initialized_total = inited1; + pref2_initialized_total = inited2; + + int unchanged1, unchanged2; + GetSamples("Settings.TrackedPreferenceUnchanged", &unchanged1, &unchanged2); + pref1_unchanged_ = unchanged1 - pref1_unchanged_total; + pref2_unchanged_ = unchanged2 - pref2_unchanged_total; + pref1_unchanged_total = unchanged1; + pref2_unchanged_total = unchanged2; + } + + TestingProfile profile_; + TestingPrefServiceSyncable* prefs_; + TestingPrefServiceSimple local_state_; + + // Since histogram samples are recorded by a global StatisticsRecorder, we + // need to maintain total counts so we can compute deltas for individual + // tests. + static int pref1_changed_total; + static int pref2_changed_total; + static int pref1_cleared_total; + static int pref2_cleared_total; + static int pref1_initialized_total; + static int pref2_initialized_total; + static int pref1_unchanged_total; + static int pref2_unchanged_total; + + // Counts of samples recorded since UpdateHistogramSamples was last called. + int pref1_changed_; + int pref2_changed_; + int pref1_cleared_; + int pref2_cleared_; + int pref1_initialized_; + int pref2_initialized_; + int pref1_unchanged_; + int pref2_unchanged_; +}; + +int PrefMetricsServiceTest::pref1_changed_total; +int PrefMetricsServiceTest::pref2_changed_total; +int PrefMetricsServiceTest::pref1_cleared_total; +int PrefMetricsServiceTest::pref2_cleared_total; +int PrefMetricsServiceTest::pref1_initialized_total; +int PrefMetricsServiceTest::pref2_initialized_total; +int PrefMetricsServiceTest::pref1_unchanged_total; +int PrefMetricsServiceTest::pref2_unchanged_total; + +TEST_F(PrefMetricsServiceTest, StartupNoUserPref) { + // Local state is empty and no user prefs are set. We should record that we + // checked preferences once but since there are no user preference values, no + // other histogram data should be collected. + scoped_ptr<PrefMetricsService> test = CreatePrefMetricsService(); + UpdateHistogramSamples(); + EXPECT_EQ(0, pref1_changed_); + EXPECT_EQ(0, pref2_changed_); + EXPECT_EQ(0, pref1_cleared_); + EXPECT_EQ(0, pref2_cleared_); + EXPECT_EQ(0, pref1_initialized_); + EXPECT_EQ(0, pref2_initialized_); + EXPECT_EQ(1, pref1_unchanged_); + EXPECT_EQ(1, pref2_unchanged_); +} + +TEST_F(PrefMetricsServiceTest, StartupUserPref) { + // Local state is empty. Set a value for one tracked pref. We should record + // that we checked preferences once and initialized a hash for the pref. + prefs_->SetString(kTrackedPrefs[0], "foo"); + { + scoped_ptr<PrefMetricsService> test = CreatePrefMetricsService(); + UpdateHistogramSamples(); + EXPECT_EQ(0, pref1_changed_); + EXPECT_EQ(0, pref2_changed_); + EXPECT_EQ(0, pref1_cleared_); + EXPECT_EQ(0, pref2_cleared_); + EXPECT_EQ(1, pref1_initialized_); + EXPECT_EQ(0, pref2_initialized_); + EXPECT_EQ(0, pref1_unchanged_); + EXPECT_EQ(1, pref2_unchanged_); + + // Change the pref. This should be observed by the PrefMetricsService, which + // will update the hash in local_state_ to stay in sync. + prefs_->SetString(kTrackedPrefs[0], "bar"); + } + // The next startup should record no changes. + { + scoped_ptr<PrefMetricsService> test = CreatePrefMetricsService(); + UpdateHistogramSamples(); + EXPECT_EQ(0, pref1_changed_); + EXPECT_EQ(0, pref2_changed_); + EXPECT_EQ(0, pref1_cleared_); + EXPECT_EQ(0, pref2_cleared_); + EXPECT_EQ(0, pref1_initialized_); + EXPECT_EQ(0, pref2_initialized_); + EXPECT_EQ(1, pref1_unchanged_); + EXPECT_EQ(1, pref2_unchanged_); + } +} + +TEST_F(PrefMetricsServiceTest, ChangedUserPref) { + // Local state is empty. Set a value for the tracked pref. We should record + // that we checked preferences once and initialized a hash for the pref. + prefs_->SetString(kTrackedPrefs[0], "foo"); + { + scoped_ptr<PrefMetricsService> test = CreatePrefMetricsService(); + UpdateHistogramSamples(); + EXPECT_EQ(0, pref1_changed_); + EXPECT_EQ(0, pref2_changed_); + EXPECT_EQ(0, pref1_cleared_); + EXPECT_EQ(0, pref2_cleared_); + EXPECT_EQ(1, pref1_initialized_); + EXPECT_EQ(0, pref2_initialized_); + EXPECT_EQ(0, pref1_unchanged_); + EXPECT_EQ(1, pref2_unchanged_); + // Hashed prefs should now be stored in local state. + } + // Change the value of the tracked pref while there is no PrefMetricsService + // to update the hash. We should observe a pref value change. + prefs_->SetString(kTrackedPrefs[0], "bar"); + { + scoped_ptr<PrefMetricsService> test = CreatePrefMetricsService(); + UpdateHistogramSamples(); + EXPECT_EQ(1, pref1_changed_); + EXPECT_EQ(0, pref2_changed_); + EXPECT_EQ(0, pref1_cleared_); + EXPECT_EQ(0, pref2_cleared_); + EXPECT_EQ(0, pref1_initialized_); + EXPECT_EQ(0, pref2_initialized_); + EXPECT_EQ(0, pref1_unchanged_); + EXPECT_EQ(1, pref2_unchanged_); + } + // Clear the value of the tracked pref while there is no PrefMetricsService + // to update the hash. We should observe a pref value removal. + prefs_->ClearPref(kTrackedPrefs[0]); + { + scoped_ptr<PrefMetricsService> test = CreatePrefMetricsService(); + UpdateHistogramSamples(); + EXPECT_EQ(0, pref1_changed_); + EXPECT_EQ(0, pref2_changed_); + EXPECT_EQ(1, pref1_cleared_); + EXPECT_EQ(0, pref2_cleared_); + EXPECT_EQ(0, pref1_initialized_); + EXPECT_EQ(0, pref2_initialized_); + EXPECT_EQ(0, pref1_unchanged_); + EXPECT_EQ(1, pref2_unchanged_); + } +} + +// Tests that serialization of dictionary values is stable. If the order of +// the entries or any whitespace changes, it would cause a spike in pref change +// UMA events as every hash would change. +TEST_F(PrefMetricsServiceTest, PrefHashStability) { + scoped_ptr<PrefMetricsService> test = CreatePrefMetricsService(); + + DictionaryValue dict; + dict.Set("a", new StringValue("foo")); + dict.Set("d", new StringValue("bad")); + dict.Set("b", new StringValue("bar")); + dict.Set("c", new StringValue("baz")); + EXPECT_EQ("C503FB7C65EEFD5C07185F616A0AA67923C069909933F362022B1F187E73E9A2", + GetHashedPrefValue(test.get(), "pref.path1", &dict)); + + ListValue list; + list.Set(0, new base::FundamentalValue(true)); + list.Set(1, new base::FundamentalValue(100)); + list.Set(2, new base::FundamentalValue(1.0)); + EXPECT_EQ("3163EC3C96263143AF83EA5C9860DFB960EE2263413C7D7D8A9973FCC00E7692", + GetHashedPrefValue(test.get(), "pref.path2", &list)); +} diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 78d760b..be0656f 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1098,6 +1098,7 @@ 'browser/prefs/chrome_pref_service_unittest.cc', 'browser/prefs/command_line_pref_store_unittest.cc', 'browser/prefs/incognito_mode_prefs_unittest.cc', + 'browser/prefs/pref_metrics_service_unittest.cc', 'browser/prefs/pref_model_associator_unittest.cc', 'browser/prefs/proxy_config_dictionary_unittest.cc', 'browser/prefs/proxy_policy_unittest.cc', diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index d719e4b..c111cb5 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -2597,4 +2597,9 @@ const char kEnableDRM[] = "settings.privacy.drm_enabled"; const char kWatchdogExtensionActive[] = "profile.extensions.activity_log.watchdog_extension_active"; +// A dictionary pref which maps profile names to dictionary values which hold +// hashes of profile prefs that we track to detect changes that happen outside +// of Chrome. +const char kProfilePreferenceHashes[] = "profile.preference_hashes"; + } // namespace prefs diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 2be5bda..ccc0e2e 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -949,6 +949,8 @@ extern const char kEnableDRM[]; extern const char kWatchdogExtensionActive[]; +extern const char kProfilePreferenceHashes[]; + } // namespace prefs #endif // CHROME_COMMON_PREF_NAMES_H_ diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index f2ddfb1..2d1fec4 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -14387,6 +14387,36 @@ other types of suffix sets. </summary> </histogram> +<histogram name="Settings.TrackedPreferenceChanged" enum="TrackedPreference"> + <summary> + The id of a tracked preference whose value has been changed since the last + time Chrome set it. + </summary> +</histogram> + +<histogram name="Settings.TrackedPreferenceCleared" enum="TrackedPreference"> + <summary> + The id of a tracked preference whose value has been cleared since the last + time Chrome set it. + </summary> +</histogram> + +<histogram name="Settings.TrackedPreferenceInitialized" + enum="TrackedPreference"> + <summary> + The id of a tracked preference whose last value isn't known. We may be just + starting to track the preference, or local state may have been changed + outside of Chrome. + </summary> +</histogram> + +<histogram name="Settings.TrackedPreferenceUnchanged" enum="TrackedPreference"> + <summary> + The id of a tracked preference whose value has not changed since the last + time Chrome set it. + </summary> +</histogram> + <histogram name="SimpleCache.App.CheckCRCResult" enum="CheckCRCResult"> <summary> Whether or not the CRC was checked at the moment when the last reference to @@ -25196,6 +25226,21 @@ other types of suffix sets. </int> </enum> +<enum name="TrackedPreference" type="int"> + <int value="0" label="prefs::kShowHomeButton"/> + <int value="1" label="prefs::kHomePageIsNewTabPage"/> + <int value="2" label="prefs::kHomePage"/> + <int value="3" label="prefs::kRestoreOnStartup"/> + <int value="4" label="prefs::kURLsToRestoreOnStartup"/> + <int value="5" label="prefs::kExtensionsPref"/> + <int value="6" label="prefs::kGoogleServicesLastUsername"/> + <int value="7" label="prefs::kSearchProviderOverrides"/> + <int value="8" label="prefs::kDefaultSearchProviderSearchURL"/> + <int value="9" label="prefs::kDefaultSearchProviderKeyword"/> + <int value="10" label="prefs::kDefaultSearchProviderName"/> + <int value="11" label="prefs::kPinnedTabs"/> +</enum> + <enum name="TranslateError" type="int"> <int value="0" label="No error"/> <int value="1" label="Network error"/> |