Multi-strategy based tracking.

chrome/browser/prefs/tracked_preference.h

Index: chrome/browser/prefs/tracked_preference.h
diff --git a/chrome/browser/prefs/tracked_preference.h b/chrome/browser/prefs/tracked_preference.h
new file mode 100644
index 0000000000000000000000000000000000000000..d47a5baf7eee519f871f1e56375a8bfda6379c86
--- /dev/null
+++ b/chrome/browser/prefs/tracked_preference.h
@@ -0,0 +1,83 @@
+// 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.
+
+#ifndef CHROME_BROWSER_PREFS_TRACKED_PREFERENCE_H_
+#define CHROME_BROWSER_PREFS_TRACKED_PREFERENCE_H_
+
+#include "base/macros.h"
+#include "chrome/browser/prefs/pref_hash_store.h"
+
+namespace base {
+class DictionaryValue;
+class Value;
+}
+
+// A TrackedPreference is an abstract class which handles decision making and
+// reporting for its subclasses. Its subclasses are responsible for implementing
+// StoreHash and EnforceAndReport.
+class TrackedPreference {
+ 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 level to be safe.
+    ENFORCE_ALL
+  };
+
+  TrackedPreference(size_t reporting_id,
+                    size_t reporting_ids_count,
+                    EnforcementLevel enforcement_level);
+  virtual ~TrackedPreference() {}
+
+  // Stores a new hash in |pref_hash_store| for this preference's |value|.
+  virtual void StoreHash(const base::Value* value,
+                         PrefHashStore* pref_hash_store) const = 0;
+
+  // Enforces that the value of this tracked preference in |pref_store_contents|
+  // is valid according to |pref_hash_store|; otherwise discarding the value in
+  // |pref_store_contents|. Reports on its findings via UMA.
+  virtual void EnforceAndReport(
+      PrefHashStore* pref_hash_store,
+      base::DictionaryValue* pref_store_contents) const = 0;
+
+ protected:
+  enum ResetAction {
+    DONT_RESET,
+    WANTED_RESET,
+    DO_RESET,
+  };
+
+  // Returns an action stating whether a reset is desired (DO_RESET) based on

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

Consider extracting all of this into a helper class rather than using
implementation inheritance.

TrackedPreferenceHelper? TrackedPreferenceCommon?

Each of the TrackedPreference implementations could then hold an instance of
this.

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

I had considered this; it's nice if you have one helper which every
TrackedPreference shares.

But in the current implementation, not every TrackedPreference uses the same
enforcement level.

Thus, unless we put the burden on the caller to also generate a
TrackedPreferenceHelper for each TrackedPreference the helper would have to be
an implementation detail of TrackedPreference; at which point implementation
inheritance is just as good.

I agree it's not ideal, but I think it works just fine in this scenario.

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

I don't think the caller needs to know about it. And they don't need to be
shared instances. This is not a virtual class I'm proposing, just basically a
utility class that would hold the few data members and the non-virtual functions
that are currently in the abstract base class. The tracked preferences would
initialize it in their initializer lists.

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

Done, as discussed I don't think it's much greater in this case (it's actually
more lines of code and more components to reason about), but if the general
paradigm is better for Chrome, I'm fine with using it here.

+  // observing |value_state| or not (DONT_RESET). |allow_changes_|,
+  // |allow_seeding_|, and |allow_migration_| make the decision softer in favor
+  // of WANTED_RESET over DO_RESET in various scenarios.

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

It's not clear to me reading this comment what the difference between
WANTED_RESET and DO_RESET is. Please document them here or at the enum above.

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

Done.

+  ResetAction GetAction(PrefHashStore::ValueState value_state) const;
+
+  // Reports |value_state| via UMA under |reporting_id_|.
+  void ReportValidationResult(PrefHashStore::ValueState value_state) const;
+
+  // Reports |reset_action| via UMA under |reporting_id_|.
+  void ReportAction(ResetAction reset_action) const;
+
+ private:
+  size_t reporting_id_;
+  size_t reporting_ids_count_;
+
+  // Allow setting changes.
+  bool allow_changes_;
+  // Allow seeding unknown values for atomic preferences.
+  bool allow_seeding_;
+  // Allow migration of values validated by the old MAC algorithm.
+  bool allow_migration_;
+
+  DISALLOW_COPY_AND_ASSIGN(TrackedPreference);
+};
+
+#endif  // CHROME_BROWSER_PREFS_TRACKED_PREFERENCE_H_