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 5dea96839cb2e8dd87ddd7790f32db74276d191f..18ad24fbda447167cbd957f9af9321457f8dae60 100644
--- a/chrome/browser/prefs/pref_hash_filter.h
+++ b/chrome/browser/prefs/pref_hash_filter.h
@@ -13,6 +13,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/pref_filter.h"
 #include "chrome/browser/prefs/pref_hash_store.h"
+#include "chrome/browser/prefs/tracked_preference.h"
 
 namespace base {
 class DictionaryValue;
@@ -24,25 +25,22 @@ class Value;
 // are changed.
 class PrefHashFilter : public PrefFilter {
  public:
-  // Enforcement levels are defined in order of intensity; the next level always
-  // implies the previous one and more.
-  enum EnforcementLevel {
-    NO_ENFORCEMENT,
-    ENFORCE,
-    ENFORCE_NO_SEEDING,
-    ENFORCE_NO_SEEDING_NO_MIGRATION,
-    // ENFORCE_ALL must always remain last; it is meant to be used when the
-    // desired level is underdetermined and the caller wants to enforce the
-    // strongest setting to be safe.
-    ENFORCE_ALL
+  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 {
+  struct TrackedPreferenceMetaData {

[erikwright (departed), 2014/01/15 22:27:42]

nit: I think Metadata is more common than MetaData.

[gab, 2014/01/16 01:28:02]

Done.

     size_t reporting_id;
     const char* name;
     // Whether to actually reset this specific preference when everything else
     // indicates that it should be.
     bool allow_enforcement;

[robertshield, 2014/01/16 03:59:30]

nit: make this an enum value too?

[gab, 2014/01/16 18:58:06]

It will soon be (see other CL I sent you; do you feel strongly it should be in
this CL?).

[robertshield, 2014/01/17 03:59:44]

If it's in the other CL that's fine by me.

+    PrefTrackingStrategy strategy;
   };
 
   // Constructs a PrefHashFilter tracking the specified |tracked_preferences|
@@ -52,10 +50,10 @@ class PrefHashFilter : public PrefFilter {
   // filter will enforce factory defaults upon detecting an untrusted preference
   // value.
   PrefHashFilter(scoped_ptr<PrefHashStore> pref_hash_store,
-                 const TrackedPreference tracked_preferences[],
+                 const TrackedPreferenceMetaData tracked_preferences[],
                  size_t tracked_preferences_size,
                  size_t reporting_ids_count,
-                 EnforcementLevel enforcement_level);
+                 TrackedPreference::EnforcementLevel enforcement_level);
 
   virtual ~PrefHashFilter();
 
@@ -66,20 +64,12 @@ class PrefHashFilter : public PrefFilter {
                             const base::Value* value) OVERRIDE;
 
  private:
-  typedef std::map<std::string, const TrackedPreference> TrackedPreferencesMap;
+  typedef std::map<std::string, scoped_ptr<const TrackedPreference> >

[erikwright (departed), 2014/01/16 16:48:32]

I believe it is unsafe to place scoped ponters in a map. I note that there are
no other instances in the codebase. I also note the existence of
scoped_ptr_hash_map.h in base/containers

[gab, 2014/01/16 18:58:06]

Great finding! I was just discussing with Cait yesterday whether there was a
better way to do this :)!

Thanks!

+      TrackedPreferencesMap;
 
   scoped_ptr<PrefHashStore> pref_hash_store_;
   TrackedPreferencesMap tracked_paths_;
 
-  size_t reporting_ids_count_;
-
-  // Prevent setting changes.
-  bool enforce_;
-  // Do not seed unknown values.
-  bool no_seeding_;
-  // Do not migrate values validated by the old MAC algorithm.
-  bool no_migration_;
-
   DISALLOW_COPY_AND_ASSIGN(PrefHashFilter);
 };