Multi-strategy based tracking.

chrome/browser/prefs/pref_hash_filter.cc

Index: chrome/browser/prefs/pref_hash_filter.cc
diff --git a/chrome/browser/prefs/pref_hash_filter.cc b/chrome/browser/prefs/pref_hash_filter.cc
index d50d66c0e550ad3b6cb8f73994592d1b09a321e6..746a8b79dac4ee30860ceb231464dcb1633949d9 100644
--- a/chrome/browser/prefs/pref_hash_filter.cc
+++ b/chrome/browser/prefs/pref_hash_filter.cc
@@ -7,90 +7,170 @@
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/values.h"
-#include "chrome/browser/prefs/pref_hash_store.h"
 #include "chrome/common/pref_names.h"
 
 namespace {
 
-// 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. Replace
-// removed preferences with "".
-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,
-#else
-  "",
-#endif
-  prefs::kExtensionKnownDisabled,
-  prefs::kProfileResetPromptMemento,
-};
-
 void ReportValidationResult(PrefHashStore::ValueState value_state,
-                            size_t value_index) {
+                            size_t value_index,
+                            size_t num_values) {
   switch (value_state) {
     case PrefHashStore::UNCHANGED:
       UMA_HISTOGRAM_ENUMERATION("Settings.TrackedPreferenceUnchanged",
-                                value_index, arraysize(kTrackedPrefs));
+                                value_index, num_values);
       return;
     case PrefHashStore::CLEARED:
       UMA_HISTOGRAM_ENUMERATION("Settings.TrackedPreferenceCleared",
-                                value_index, arraysize(kTrackedPrefs));
+                                value_index, num_values);
       return;
     case PrefHashStore::MIGRATED:
       UMA_HISTOGRAM_ENUMERATION("Settings.TrackedPreferenceMigrated",
-                                value_index, arraysize(kTrackedPrefs));
+                                value_index, num_values);
       return;
     case PrefHashStore::CHANGED:
       UMA_HISTOGRAM_ENUMERATION("Settings.TrackedPreferenceChanged",
-                                value_index, arraysize(kTrackedPrefs));
+                                value_index, num_values);
       return;
     case PrefHashStore::UNKNOWN_VALUE:
       UMA_HISTOGRAM_ENUMERATION("Settings.TrackedPreferenceInitialized",
-                                value_index, arraysize(kTrackedPrefs));
+                                value_index, num_values);
       return;
   }
   NOTREACHED() << "Unexpected PrefHashStore::ValueState: " << value_state;
 }
 
+void ReportSplitPreferenceChangedCount(const std::string& path,

[Alexei Svitkine (slow), 2013/12/17 22:19:56]

Add a comment. Mention that |path| should be one of the kTrackedPrefs paths.

[gab, 2013/12/18 17:43:14]

Added a comment; kTrackedPref is in the scope of the constructor only, I don't
think this comment should mention it, it doesn't depend on it.

+                                       size_t count) {
+  // Note: The factory creates and owns the histogram.

[Alexei Svitkine (slow), 2013/12/17 22:19:56]

Nit: Mention that this histogram corresponds to a specific macro invocation.

[gab, 2013/12/18 17:43:14]

Done.

+  base::HistogramBase* histogram =
+      base::LinearHistogram::FactoryGet(
+          "Settings.TrackedSplitPreferenceChanged." + path,
+          1,
+          100,  // Allow counts up to 100.
+          101,
+          base::HistogramBase::kUmaTargetedHistogramFlag);
+  histogram->Add(count);
+}
+
 }  // namespace
 
 PrefHashFilter::PrefHashFilter(scoped_ptr<PrefHashStore> pref_hash_store)
-    : pref_hash_store_(pref_hash_store.Pass()),
-      tracked_paths_(kTrackedPrefs, kTrackedPrefs + arraysize(kTrackedPrefs)) {}
+    : pref_hash_store_(pref_hash_store.Pass()) {
+  // 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. Each
+  // tracked preference must be given a unique reporting ID.
+  static const TrackedPreference kTrackedPrefs[] = {
+    { 0, prefs::kShowHomeButton, TRACKING_STRATEGY_ATOMIC },
+    { 1, prefs::kHomePageIsNewTabPage, TRACKING_STRATEGY_ATOMIC },
+    { 2, prefs::kHomePage, TRACKING_STRATEGY_ATOMIC },
+    { 3, prefs::kRestoreOnStartup, TRACKING_STRATEGY_ATOMIC },
+    { 4, prefs::kURLsToRestoreOnStartup, TRACKING_STRATEGY_ATOMIC },
+    { 5, prefs::kExtensionsPref, TRACKING_STRATEGY_SPLIT },
+    { 6, prefs::kGoogleServicesLastUsername, TRACKING_STRATEGY_ATOMIC },
+    { 7, prefs::kSearchProviderOverrides, TRACKING_STRATEGY_ATOMIC },
+    { 8, prefs::kDefaultSearchProviderSearchURL, TRACKING_STRATEGY_ATOMIC },
+    { 9, prefs::kDefaultSearchProviderKeyword, TRACKING_STRATEGY_ATOMIC },
+    { 10, prefs::kDefaultSearchProviderName, TRACKING_STRATEGY_ATOMIC },
+  #if !defined(OS_ANDROID)
+    { 11, prefs::kPinnedTabs, TRACKING_STRATEGY_ATOMIC },
+  #endif
+    { 12, prefs::kExtensionKnownDisabled, TRACKING_STRATEGY_ATOMIC },
+    { 13, prefs::kProfileResetPromptMemento, TRACKING_STRATEGY_ATOMIC },
+  };
+
+  InitTrackedPreferences(kTrackedPrefs, arraysize(kTrackedPrefs));
+}
+
+PrefHashFilter::PrefHashFilter(
+    scoped_ptr<PrefHashStore> pref_hash_store,
+    const PrefHashFilter::TrackedPreference tracked_preferences[],
+    size_t tracked_preferences_size)
+    : pref_hash_store_(pref_hash_store.Pass()) {
+  InitTrackedPreferences(tracked_preferences, tracked_preferences_size);
+}
 
 PrefHashFilter::~PrefHashFilter() {}
 
 // Updates the stored hash to correspond to the updated preference value.
 void PrefHashFilter::FilterUpdate(const std::string& path,
                                   const base::Value* value) {
-  if (tracked_paths_.find(path) != tracked_paths_.end())
-    pref_hash_store_->StoreHash(path, value);
+  TrackedPreferencesMap::const_iterator it = tracked_paths_.find(path);
+  if (it != tracked_paths_.end())
+    StoreHashUsingStrategy(path, value, it->second.strategy);
 }
 
 // Validates loaded preference values according to stored hashes, reports
 // validation results via UMA, and updates hashes in case of mismatch.
 void PrefHashFilter::FilterOnLoad(base::DictionaryValue* pref_store_contents) {
-  for (size_t i = 0; i < arraysize(kTrackedPrefs); ++i) {
-    if (kTrackedPrefs[i][0] == '\0')
-      continue;
+  for (TrackedPreferencesMap::const_iterator it = tracked_paths_.begin();
+       it != tracked_paths_.end(); ++it) {
+    const std::string& pref_path = it->first;
+    const TrackedPreferenceMetaData& metadata = it->second;
     const base::Value* value = NULL;
-    pref_store_contents->Get(kTrackedPrefs[i], &value);
+    pref_store_contents->Get(pref_path, &value);
     PrefHashStore::ValueState value_state =
-        pref_hash_store_->CheckValue(kTrackedPrefs[i], value);
-    ReportValidationResult(value_state, i);
+        CheckValueUsingStrategy(pref_path, value, metadata.strategy);
+    ReportValidationResult(value_state, metadata.reporting_id,
+                           tracked_paths_.size());
     if (value_state != PrefHashStore::UNCHANGED)
-      pref_hash_store_->StoreHash(kTrackedPrefs[i], value);
+      StoreHashUsingStrategy(pref_path, value, metadata.strategy);
+  }
+}
+
+void PrefHashFilter::InitTrackedPreferences(
+    const PrefHashFilter::TrackedPreference tracked_preferences[],
+    size_t tracked_preferences_size) {
+  for (size_t i = 0; i < tracked_preferences_size; ++i) {
+    const TrackedPreferenceMetaData metadata = {
+      tracked_preferences[i].reporting_id,
+      tracked_preferences[i].strategy,
+    };
+    bool is_new = tracked_paths_.insert(std::make_pair(
+        std::string(tracked_preferences[i].name), metadata)).second;
+    DCHECK(is_new);
+  }
+}
+
+void PrefHashFilter::StoreHashUsingStrategy(const std::string& path,
+                                            const base::Value* value,
+                                            PrefTrackingStrategy strategy) {
+  switch (strategy) {
+    case TRACKING_STRATEGY_ATOMIC:
+      pref_hash_store_->StoreHash(path, value);
+      break;
+    case TRACKING_STRATEGY_SPLIT:
+      const base::DictionaryValue* dict_value = NULL;
+      if (value && !value->GetAsDictionary(&dict_value)) {
+        NOTREACHED();
+        break;
+      }
+      pref_hash_store_->StoreSplitHash(path, dict_value);
+      break;
+  }
+}
+
+PrefHashStore::ValueState PrefHashFilter::CheckValueUsingStrategy(
+    const std::string& path,
+    const base::Value* value,
+    PrefTrackingStrategy strategy) {
+  switch (strategy) {
+    case TRACKING_STRATEGY_ATOMIC:
+      return pref_hash_store_->CheckValue(path, value);
+    case TRACKING_STRATEGY_SPLIT:

[Alexei Svitkine (slow), 2013/12/17 22:19:56]

Nit: Either add {} around this case body, since its not trivial, or refactor the
code to not use a switch (e.g. if/else if). Same for the function above.

[gab, 2013/12/18 17:43:14]

Done.

+      const base::DictionaryValue* dict_value = NULL;
+      if (value && !value->GetAsDictionary(&dict_value)) {
+        NOTREACHED();
+        return PrefHashStore::UNKNOWN_VALUE;
+      }
+
+      std::vector<std::string> invalid_keys;
+      PrefHashStore::ValueState state =
+          pref_hash_store_->CheckSplitValue(path, dict_value, &invalid_keys);
+      if (state == PrefHashStore::CHANGED)
+        ReportSplitPreferenceChangedCount(path, invalid_keys.size());
+      return state;
   }
+  NOTREACHED();
+  return PrefHashStore::UNKNOWN_VALUE;
 }