Multi-strategy based tracking.

chrome/browser/prefs/pref_hash_filter.h

Index: chrome/browser/prefs/pref_hash_filter.h
diff --git a/chrome/browser/prefs/pref_hash_filter.h b/chrome/browser/prefs/pref_hash_filter.h
index baa7811a05aca3fd5ed75039ac5f2f8f626e780a..bf604a4fedd14cb4481fab79cb4a41d672e5b983 100644
--- a/chrome/browser/prefs/pref_hash_filter.h
+++ b/chrome/browser/prefs/pref_hash_filter.h
@@ -12,8 +12,7 @@
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/pref_filter.h"
-
-class PrefHashStore;
+#include "chrome/browser/prefs/pref_hash_store.h"
 
 namespace base {
 class DictionaryValue;
@@ -25,7 +24,30 @@ class Value;
 // are changed.
 class PrefHashFilter : public PrefFilter {
  public:
+  enum PrefTrackingStrategy {
+    // Atomic preferences are tracked as a whole.
+    TRACKING_STRATEGY_ATOMIC,
+    // Split preferences are dictionaries for which each top-level entry is
+    // tracked independently. Note: preferences using this strategy must be kept
+    // in sync with TrackedSplitPreferences in histograms.xml.
+    TRACKING_STRATEGY_SPLIT,
+  };
+
+  struct TrackedPreference {
+    size_t reporting_id;
+    const char* name;
+    PrefTrackingStrategy strategy;
+  };
+
+  // Constructs a PrefHashFilter with the default set of tracked preferences.
   explicit PrefHashFilter(scoped_ptr<PrefHashStore> pref_hash_store);
+
+  // Constructs a PrefHashFilter with the specified |tracked_preferences|. Meant
+  // to be used by tests only.
+  PrefHashFilter(scoped_ptr<PrefHashStore> pref_hash_store,

[erikwright (departed), 2013/12/19 15:21:28]

Maybe this should be the standard constructor and the list of tracked
preferences should live permanently in chrome_pref_service_factory.cc .

In fact, if you look at my CL for unloaded profiles you will see that I stuck a
method for initializing a pref hash store into pref filter just because that was
the place with access to the list of preferences.

It would be easier to correctly place if the list of tracked preferences was
elsewhere.

[gab, 2013/12/20 18:37:06]

Good point, moved.

+                 const TrackedPreference tracked_preferences[],
+                 size_t tracked_preferences_size);
+
   virtual ~PrefHashFilter();
 
   // PrefFilter implementation.
@@ -35,8 +57,33 @@ class PrefHashFilter : public PrefFilter {
                             const base::Value* value) OVERRIDE;
 
  private:
+  struct TrackedPreferenceMetaData {

[erikwright (departed), 2013/12/19 15:21:28]

Is there a good reason to define this struct vs. just storing TrackedPreference
instances?

Sure, there is one unnecessary field (the char* pointer) but it could simplify
the code a bit, especially if you ever add extra properties (because you won't
need to double declare them or copy them over).

[gab, 2013/12/20 18:37:06]

Yea, I was thinking about that too when I added reporting_id to
TrackedPreference as well.

Now that you mention it too I guess it makes sense, plus it's only a pointer,
not a full string extra to store.

It just feels weird to have a map for which the key is the name if the value
also contains the name.

+    size_t reporting_id;
+    PrefTrackingStrategy strategy;
+  };
+
+  typedef std::map<std::string, const TrackedPreferenceMetaData>
+      TrackedPreferencesMap;
+
+  /// Initializes |tracked_paths_| from |tracked_preferences_|.
+  void InitTrackedPreferences(const TrackedPreference tracked_preferences[],
+                              size_t tracked_preferences_size);
+
+  // Updates the hash(es) in |pref_hash_store_| for |value| at |path| based on
+  // the tracking |strategy|.
+  void StoreHashUsingStrategy(const std::string& path,
+                              const base::Value* value,
+                              PrefTrackingStrategy strategy);
+
+  // Checks the hash(es) in |pref_hash_store_| for |value| at |path| based on
+  // the tracking |strategy|.
+  PrefHashStore::ValueState CheckValueUsingStrategy(
+      const std::string& path,
+      const base::Value* value,
+      PrefTrackingStrategy strategy);
+
   scoped_ptr<PrefHashStore> pref_hash_store_;
-  std::set<std::string> tracked_paths_;
+  TrackedPreferencesMap tracked_paths_;
 
   DISALLOW_COPY_AND_ASSIGN(PrefHashFilter);
 };