Introduce a new framework for back-and-forth tracked/protected preferences migration.

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 e35df37edb45769367d2673f109a1097ab88b4e1..47907ae238aaf9fbf998490cbd441d753f826e2f 100644
--- a/chrome/browser/prefs/pref_hash_filter.cc
+++ b/chrome/browser/prefs/pref_hash_filter.cc
@@ -6,9 +6,9 @@
 
 #include <algorithm>
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
-#include "base/prefs/persistent_pref_store.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/pref_store.h"
 #include "base/strings/string_number_conversions.h"
@@ -92,51 +92,6 @@ void PrefHashFilter::ClearResetTime(PrefService* user_prefs) {
   user_prefs->ClearPref(prefs::kPreferenceResetTime);
 }
 
-void PrefHashFilter::MigrateValues(PersistentPrefStore* source,
-                                   PersistentPrefStore* destination) {
-  bool commit_source = false;
-  bool commit_destination = false;
-
-  scoped_ptr<PrefHashStoreTransaction> transaction =
-      pref_hash_store_->BeginTransaction();
-  for (TrackedPreferencesMap::const_iterator it = tracked_paths_.begin();
-       it != tracked_paths_.end();
-       ++it) {
-    const base::Value* source_value = NULL;
-    if (source->GetValue(it->first, &source_value)) {
-      if (!destination->GetValue(it->first, NULL)) {
-        base::DictionaryValue temp_dictionary;
-        // Copy the value from |source| into a suitable place for a
-        // TrackedPreference to act on it.
-        temp_dictionary.Set(it->first, source_value->DeepCopy());
-        // Check whether the value is correct according to our MAC. May remove
-        // the value from |temp_dictionary|.
-        it->second->EnforceAndReport(&temp_dictionary, transaction.get());
-        // Now take the value as it appears in |temp_dictionary| and put it in
-        // |destination|.
-        scoped_ptr<base::Value> checked_value;
-        if (temp_dictionary.Remove(it->first, &checked_value)) {
-          destination->SetValue(it->first, checked_value.release());
-          commit_destination = true;
-        }
-      }
-      source->RemoveValue(it->first);
-      commit_source = true;
-    }
-  }
-
-  // Order these such that a crash at any point is still recoverable. We assume
-  // that they are configured such that the writes will occur on worker threads
-  // in the order that we asked for them.
-  if (commit_destination)
-    destination->CommitPendingWrite();
-  transaction.reset();
-  // If we crash here, we will just delete the values from |source| in a future
-  // invocation of MigrateValues.
-  if (commit_source)
-    source->CommitPendingWrite();
-}
-
 void PrefHashFilter::Initialize(const PrefStore& pref_store) {
   scoped_ptr<PrefHashStoreTransaction> hash_store_transaction(
       pref_hash_store_->BeginTransaction());
@@ -150,38 +105,26 @@ void PrefHashFilter::Initialize(const PrefStore& pref_store) {
   }
 }
 
-// Validates loaded preference values according to stored hashes, reports
-// validation results via UMA, and updates hashes in case of mismatch.
-bool PrefHashFilter::FilterOnLoad(base::DictionaryValue* pref_store_contents) {
-  DCHECK(pref_store_contents);
-  base::TimeTicks checkpoint = base::TimeTicks::Now();
-
-  bool did_reset = false;
-  {
-    scoped_ptr<PrefHashStoreTransaction> hash_store_transaction(
-        pref_hash_store_->BeginTransaction());
-    for (TrackedPreferencesMap::const_iterator it = tracked_paths_.begin();
-         it != tracked_paths_.end(); ++it) {
-      if (it->second->EnforceAndReport(pref_store_contents,
-                                       hash_store_transaction.get())) {
-        did_reset = true;
-      }
-    }
-  }
-
-  if (did_reset) {
-    pref_store_contents->Set(prefs::kPreferenceResetTime,
-                             new base::StringValue(base::Int64ToString(
-                                 base::Time::Now().ToInternalValue())));
+void PrefHashFilter::FilterOnLoad(
+    const PostFilterOnLoadCallback& post_filter_on_load_callback,
+    scoped_ptr<base::DictionaryValue> pref_store_contents) {
+  if (filter_on_load_interceptor_.is_null()) {
+    FinalizeFilterOnLoad(post_filter_on_load_callback,
+                         pref_store_contents.Pass(),
+                         false);
+  } else {
+    // Note, in practice (in the implementation as it was in May 2014) it would
+    // be okay to pass an unretained |this| pointer below, but in order to avoid
+    // having to augment the API everywhere to explicitly enforce the ownership
+    // model as it happens to currently be: make the relationship simpler by
+    // weakly binding the FinalizeFilterOnLoadCallback below to |this|.
+    const FinalizeFilterOnLoadCallback finalize_filter_on_load(
+        base::Bind(&PrefHashFilter::FinalizeFilterOnLoad,
+                   AsWeakPtr(),
+                   post_filter_on_load_callback));
+    filter_on_load_interceptor_.Run(finalize_filter_on_load,
+                                    pref_store_contents.Pass());
   }
-
-  // TODO(gab): Remove this histogram by Feb 21 2014; after sufficient timing
-  // data has been gathered from the wild to be confident this doesn't
-  // significantly affect startup.
-  UMA_HISTOGRAM_TIMES("Settings.FilterOnLoadTime",
-                      base::TimeTicks::Now() - checkpoint);
-
-  return did_reset;
 }
 
 // Marks |path| has having changed if it is part of |tracked_paths_|. A new hash
@@ -234,3 +177,45 @@ void PrefHashFilter::FilterSerializeData(
   // necessary anyways).
   pref_hash_store_->CommitPendingWrite();
 }
+
+void PrefHashFilter::InterceptNextFilterOnLoad(
+    const FilterOnLoadInterceptor& filter_on_load_interceptor) {
+  DCHECK(filter_on_load_interceptor_.is_null());
+  filter_on_load_interceptor_ = filter_on_load_interceptor;
+}
+
+void PrefHashFilter::FinalizeFilterOnLoad(
+    const PostFilterOnLoadCallback& post_filter_on_load_callback,
+    scoped_ptr<base::DictionaryValue> pref_store_contents,
+    bool prefs_altered) {
+  DCHECK(pref_store_contents);
+  base::TimeTicks checkpoint = base::TimeTicks::Now();
+
+  bool did_reset = false;
+  {
+    scoped_ptr<PrefHashStoreTransaction> hash_store_transaction(
+        pref_hash_store_->BeginTransaction());
+    for (TrackedPreferencesMap::const_iterator it = tracked_paths_.begin();
+         it != tracked_paths_.end(); ++it) {
+      if (it->second->EnforceAndReport(pref_store_contents.get(),
+                                       hash_store_transaction.get())) {
+        did_reset = true;
+        prefs_altered = true;
+      }
+    }
+  }
+
+  if (did_reset) {
+    pref_store_contents->Set(prefs::kPreferenceResetTime,
+                             new base::StringValue(base::Int64ToString(
+                                 base::Time::Now().ToInternalValue())));
+  }
+
+  // TODO(gab): Remove this histogram by Feb 21 2014; after sufficient timing
+  // data has been gathered from the wild to be confident this doesn't
+  // significantly affect startup.
+  UMA_HISTOGRAM_TIMES("Settings.FilterOnLoadTime",
+                      base::TimeTicks::Now() - checkpoint);
+
+  post_filter_on_load_callback.Run(pref_store_contents.Pass(), prefs_altered);
+}