Add TrackedPreferenceValidationDelegate

chrome/browser/prefs/profile_pref_store_manager.cc

Index: chrome/browser/prefs/profile_pref_store_manager.cc
diff --git a/chrome/browser/prefs/profile_pref_store_manager.cc b/chrome/browser/prefs/profile_pref_store_manager.cc
index 6558c4afb65254a13ca360d3952e58c10579e2d4..8bd2fe5553a389e20482e9a9d12d115f44524e21 100644
--- a/chrome/browser/prefs/profile_pref_store_manager.cc
+++ b/chrome/browser/prefs/profile_pref_store_manager.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/prefs/pref_hash_store_impl.h"
 #include "chrome/browser/prefs/tracked/pref_service_hash_store_contents.h"
 #include "chrome/browser/prefs/tracked/segregated_pref_store.h"
+#include "chrome/browser/prefs/tracked/tracked_preference_validation_observer.h"
 #include "chrome/browser/prefs/tracked/tracked_preferences_migration.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/pref_names.h"
@@ -182,6 +183,7 @@ void InitializeHashStoreObserver::OnInitializationCompleted(bool succeeded) {
   if (succeeded && pre_update_version < PrefHashStoreImpl::VERSION_LATEST) {
     PrefHashFilter(pref_hash_store_impl_.PassAs<PrefHashStore>(),
                    tracking_configuration_,
+                   scoped_ptr<TrackedPreferenceValidationObserver>(),
                    reporting_ids_count_).Initialize(*pref_store_);
     UMA_HISTOGRAM_ENUMERATION(
         "Settings.TrackedPreferencesAlternateStoreVersionUpdatedFrom",
@@ -192,6 +194,87 @@ void InitializeHashStoreObserver::OnInitializationCompleted(bool succeeded) {
   delete this;
 }
 
+// A TrackedPreferenceValidationObserver that delegates to another shared
+// observer instance.
+class DelegatingValidationObserver

[Mattias Nissler (ping if slow), 2014/05/15 14:32:25]

Ah, so this is where the ownership awkwardness happens get handled. If the
PrefHashFilter wouldn't own the observer, you could just pass the same pointer
to both instances, no?

[grt (UTC plus 2), 2014/05/15 18:21:35]

Yes. This introduces other issues that Erik has already pointed out.

+    : public TrackedPreferenceValidationObserver {
+ public:
+  // A reference-counted holder of a TrackedPreferenceValidationObserver.
+  class RefCountedObserver : public base::RefCounted<RefCountedObserver> {
+   public:
+    explicit RefCountedObserver(
+        scoped_ptr<TrackedPreferenceValidationObserver> observer);
+    TrackedPreferenceValidationObserver* get();
+
+   private:
+    friend class base::RefCounted<RefCountedObserver>;
+    ~RefCountedObserver();
+
+    scoped_ptr<TrackedPreferenceValidationObserver> observer_;
+  };
+
+  explicit DelegatingValidationObserver(
+      const scoped_refptr<RefCountedObserver>& observer);
+  virtual ~DelegatingValidationObserver();
+
+  // TrackedPreferenceValidationObserver methods.
+  virtual void OnAtomicPreferenceValidation(
+      const std::string& pref_path,
+      const base::Value* value,
+      PrefHashStoreTransaction::ValueState value_state,
+      TrackedPreferenceHelper::ResetAction reset_action) OVERRIDE;
+  virtual void OnSplitPreferenceValidation(
+      const std::string& pref_path,
+      const base::DictionaryValue* dict_value,
+      const std::vector<std::string>& invalid_keys,
+      PrefHashStoreTransaction::ValueState value_state,
+      TrackedPreferenceHelper::ResetAction reset_action) OVERRIDE;
+
+ private:
+  scoped_refptr<RefCountedObserver> observer_;
+  DISALLOW_COPY_AND_ASSIGN(DelegatingValidationObserver);
+};
+
+DelegatingValidationObserver::RefCountedObserver::RefCountedObserver(
+    scoped_ptr<TrackedPreferenceValidationObserver> observer)
+    : observer_(observer.Pass()) {
+}
+
+DelegatingValidationObserver::RefCountedObserver::~RefCountedObserver() {
+}
+
+TrackedPreferenceValidationObserver*
+DelegatingValidationObserver::RefCountedObserver::get() {
+  return observer_.get();
+}
+
+DelegatingValidationObserver::DelegatingValidationObserver(
+    const scoped_refptr<RefCountedObserver>& observer)
+    : observer_(observer) {
+}
+
+DelegatingValidationObserver::~DelegatingValidationObserver() {
+}
+
+void DelegatingValidationObserver::OnAtomicPreferenceValidation(
+    const std::string& pref_path,
+    const base::Value* value,
+    PrefHashStoreTransaction::ValueState value_state,
+    TrackedPreferenceHelper::ResetAction reset_action) {
+  observer_->get()->OnAtomicPreferenceValidation(
+      pref_path, value, value_state, reset_action);
+}
+
+void DelegatingValidationObserver::OnSplitPreferenceValidation(
+    const std::string& pref_path,
+    const base::DictionaryValue* dict_value,
+    const std::vector<std::string>& invalid_keys,
+    PrefHashStoreTransaction::ValueState value_state,
+    TrackedPreferenceHelper::ResetAction reset_action) {
+  observer_->get()->OnSplitPreferenceValidation(
+      pref_path, dict_value, invalid_keys, value_state, reset_action);
+}
+
 }  // namespace
 
 // TODO(erikwright): Enable this on Chrome OS and Android once MACs are moved
@@ -259,7 +342,8 @@ void ProfilePrefStoreManager::ResetPrefHashStore() {
 }
 
 PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore(
-    const scoped_refptr<base::SequencedTaskRunner>& io_task_runner) {
+    const scoped_refptr<base::SequencedTaskRunner>& io_task_runner,
+    scoped_ptr<TrackedPreferenceValidationObserver> verification_observer) {
   scoped_ptr<PrefFilter> pref_filter;
   if (!kPlatformSupportsPreferenceTracking) {
     return new JsonPrefStore(GetPrefFilePathFromProfilePath(profile_path_),
@@ -286,14 +370,28 @@ PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore(
     }
   }
 
-  scoped_ptr<PrefHashFilter> unprotected_pref_hash_filter(
-      new PrefHashFilter(GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
-                         unprotected_configuration,
-                         reporting_ids_count_));
-  scoped_ptr<PrefHashFilter> protected_pref_hash_filter(
-      new PrefHashFilter(GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
-                         protected_configuration,
-                         reporting_ids_count_));
+  scoped_refptr<DelegatingValidationObserver::RefCountedObserver> observer;
+  if (verification_observer) {
+    observer = new DelegatingValidationObserver::RefCountedObserver(
+        verification_observer.Pass());
+  }
+
+  scoped_ptr<PrefHashFilter> unprotected_pref_hash_filter(new PrefHashFilter(
+      GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
+      unprotected_configuration,
+      observer ?
+          scoped_ptr<TrackedPreferenceValidationObserver>(
+              new DelegatingValidationObserver(observer)) :
+          scoped_ptr<TrackedPreferenceValidationObserver>(),
+      reporting_ids_count_));
+  scoped_ptr<PrefHashFilter> protected_pref_hash_filter(new PrefHashFilter(
+      GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
+      protected_configuration,
+      observer ?
+          scoped_ptr<TrackedPreferenceValidationObserver>(
+              new DelegatingValidationObserver(observer)) :
+          scoped_ptr<TrackedPreferenceValidationObserver>(),
+      reporting_ids_count_));
 
   PrefHashFilter* raw_unprotected_pref_hash_filter =
       unprotected_pref_hash_filter.get();
@@ -377,6 +475,7 @@ bool ProfilePrefStoreManager::InitializePrefsFromMasterPrefs(
         new DictionaryPrefStore(&master_prefs));
     PrefHashFilter(GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
                    tracking_configuration_,
+                   scoped_ptr<TrackedPreferenceValidationObserver>(),
                    reporting_ids_count_).Initialize(*pref_store);
   }
 
@@ -392,6 +491,7 @@ ProfilePrefStoreManager::CreateDeprecatedCombinedProfilePrefStore(
     pref_filter.reset(
         new PrefHashFilter(GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
                            tracking_configuration_,
+                           scoped_ptr<TrackedPreferenceValidationObserver>(),
                            reporting_ids_count_));
   }
   return new JsonPrefStore(GetPrefFilePathFromProfilePath(profile_path_),