Move preference MACs to the protected preference stores.

chrome/browser/prefs/tracked/tracked_preferences_migration_unittest.cc

Index: chrome/browser/prefs/tracked/tracked_preferences_migration_unittest.cc
diff --git a/chrome/browser/prefs/tracked/tracked_preferences_migration_unittest.cc b/chrome/browser/prefs/tracked/tracked_preferences_migration_unittest.cc
index 1c93a39fc0920e8cdfb6c93dc9538add9f755c5c..38dd2a6536b9f3333d7158b6df1169ae27a411bd 100644
--- a/chrome/browser/prefs/tracked/tracked_preferences_migration_unittest.cc
+++ b/chrome/browser/prefs/tracked/tracked_preferences_migration_unittest.cc
@@ -11,8 +11,14 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/prefs/testing_pref_service.h"
 #include "base/values.h"
+#include "chrome/browser/prefs/dictionary_pref_store.h"
 #include "chrome/browser/prefs/interceptable_pref_filter.h"
+#include "chrome/browser/prefs/pref_hash_store_impl.h"
+#include "chrome/browser/prefs/profile_pref_store_manager.h"
+#include "chrome/browser/prefs/tracked/pref_service_hash_store_contents.h"
+#include "chrome/browser/prefs/tracked/pref_store_hash_store_contents.h"
 #include "chrome/browser/prefs/tracked/tracked_preferences_migration.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -49,9 +55,9 @@ class SimpleInterceptablePrefFilter : public InterceptablePrefFilter {
   // InterceptablePrefFilter implementation.
   virtual void FinalizeFilterOnLoad(
       const PostFilterOnLoadCallback& post_filter_on_load_callback,
-      scoped_ptr<base::DictionaryValue> pref_store_contents,
+      base::DictionaryValue* pref_store_contents,
       bool prefs_altered) OVERRIDE {
-    post_filter_on_load_callback.Run(pref_store_contents.Pass(), prefs_altered);
+    post_filter_on_load_callback.Run(prefs_altered);
   }
 };
 
@@ -69,6 +75,45 @@ class TrackedPreferencesMigrationTest : public testing::Test {
     MOCK_UNPROTECTED_PREF_STORE,
     MOCK_PROTECTED_PREF_STORE,
   };
+  class MockTrackedPreferencesMigrationDelegate
+      : public TrackedPreferencesMigrationDelegate {
+   public:
+    MockTrackedPreferencesMigrationDelegate(
+        TrackedPreferencesMigrationTest* outer,
+        InterceptablePrefFilter* pref_filter,
+        PrefHashStore* pref_hash_store,
+        MockPrefStoreID pref_store_id)
+        : outer_(outer),
+          pref_filter_(pref_filter),
+          pref_hash_store_(pref_hash_store),
+          pref_store_id_(pref_store_id) {}
+
+    // TrackedPreferencesMigrationDelegate implementation
+    virtual bool IsValid() OVERRIDE { return true; }
+    virtual void CleanPreference(const std::string& key) OVERRIDE {
+      outer_->RemovePathFromStore(pref_store_id_, key);
+    }
+    virtual void NotifyOnSuccessfulWrite(
+        const base::Closure& on_successful_write) OVERRIDE {
+      outer_->RegisterSuccessfulWriteClosure(pref_store_id_,
+                                             on_successful_write);
+    }
+    virtual void InterceptLoadedPreferences(
+        const Intercept& intercept) OVERRIDE {
+      pref_filter_->InterceptNextFilterOnLoad(intercept);
+    }
+    virtual PrefHashStore* GetPrefHashStore() OVERRIDE {
+      return pref_hash_store_;
+    }
+
+   private:
+    TrackedPreferencesMigrationTest* outer_;
+    InterceptablePrefFilter* pref_filter_;
+    PrefHashStore* pref_hash_store_;
+    MockPrefStoreID pref_store_id_;
+
+    DISALLOW_COPY_AND_ASSIGN(MockTrackedPreferencesMigrationDelegate);
+  };
 
   TrackedPreferencesMigrationTest()
       : unprotected_prefs_(new base::DictionaryValue),
@@ -79,6 +124,11 @@ class TrackedPreferencesMigrationTest : public testing::Test {
         protected_store_migration_complete_(false) {}
 
   virtual void SetUp() OVERRIDE {
+    ProfilePrefStoreManager::RegisterPrefs(local_state_.registry());
+    const char kSeed[] = "01234567012345670123456701234567";
+    const char kDeviceId[] = "device-id";
+    const char kHashStoreId[] = "hash-store-id";
+
     std::set<std::string> unprotected_pref_names;
     std::set<std::string> protected_pref_names;
     unprotected_pref_names.insert(kUnprotectedPref);
@@ -86,25 +136,39 @@ class TrackedPreferencesMigrationTest : public testing::Test {
     protected_pref_names.insert(kProtectedPref);
     protected_pref_names.insert(kPreviouslyUnprotectedPref);
 
+    unprotected_pref_hash_store_.reset(
+        new PrefHashStoreImpl(kSeed, kDeviceId, false));
+    protected_pref_hash_store_.reset(
+        new PrefHashStoreImpl(kSeed, kDeviceId, true));
+    scoped_ptr<PrefHashStoreImpl> legacy_pref_hash_store(
+        new PrefHashStoreImpl(kSeed, kDeviceId, false));
+    unprotected_pref_hash_store_->SetHashStoreContents(
+        scoped_ptr<HashStoreContents>(new PrefStoreHashStoreContents(
+            new DictionaryPrefStore(unprotected_prefs_.get()))));
+    protected_pref_hash_store_->SetHashStoreContents(
+        scoped_ptr<HashStoreContents>(new PrefStoreHashStoreContents(
+            new DictionaryPrefStore(protected_prefs_.get()))));
+    legacy_pref_hash_store->SetHashStoreContents(scoped_ptr<HashStoreContents>(
+        new PrefServiceHashStoreContents(kHashStoreId, &local_state_)));
+
     SetupTrackedPreferencesMigration(
         unprotected_pref_names,
         protected_pref_names,
-        base::Bind(&TrackedPreferencesMigrationTest::RemovePathFromStore,
-                   base::Unretained(this), MOCK_UNPROTECTED_PREF_STORE),
-        base::Bind(&TrackedPreferencesMigrationTest::RemovePathFromStore,
-                   base::Unretained(this), MOCK_PROTECTED_PREF_STORE),
-        base::Bind(
-            &TrackedPreferencesMigrationTest::RegisterSuccessfulWriteClosure,
-            base::Unretained(this), MOCK_UNPROTECTED_PREF_STORE),
-        base::Bind(
-            &TrackedPreferencesMigrationTest::RegisterSuccessfulWriteClosure,
-            base::Unretained(this), MOCK_PROTECTED_PREF_STORE),
-        &mock_unprotected_pref_filter_,
-        &mock_protected_pref_filter_);
+        scoped_ptr<TrackedPreferencesMigrationDelegate>(
+            new MockTrackedPreferencesMigrationDelegate(
+                this,
+                &mock_unprotected_pref_filter_,
+                unprotected_pref_hash_store_.get(),
+                MOCK_UNPROTECTED_PREF_STORE)),
+        scoped_ptr<TrackedPreferencesMigrationDelegate>(
+            new MockTrackedPreferencesMigrationDelegate(
+                this,
+                &mock_protected_pref_filter_,
+                protected_pref_hash_store_.get(),
+                MOCK_PROTECTED_PREF_STORE)),
+        legacy_pref_hash_store.PassAs<PrefHashStore>());
 
     // Verify initial expectations are met.
-    EXPECT_TRUE(HasPrefs(MOCK_UNPROTECTED_PREF_STORE));
-    EXPECT_TRUE(HasPrefs(MOCK_PROTECTED_PREF_STORE));
     EXPECT_FALSE(
         WasOnSuccessfulWriteCallbackRegistered(MOCK_UNPROTECTED_PREF_STORE));
     EXPECT_FALSE(
@@ -183,29 +247,18 @@ class TrackedPreferencesMigrationTest : public testing::Test {
             base::Bind(&TrackedPreferencesMigrationTest::GetPrefsBack,
                        base::Unretained(this),
                        MOCK_UNPROTECTED_PREF_STORE),
-            unprotected_prefs_.Pass());
+            unprotected_prefs_.get());
         break;
       case MOCK_PROTECTED_PREF_STORE:
         mock_protected_pref_filter_.FilterOnLoad(
             base::Bind(&TrackedPreferencesMigrationTest::GetPrefsBack,
                        base::Unretained(this),
                        MOCK_PROTECTED_PREF_STORE),
-            protected_prefs_.Pass());
+            protected_prefs_.get());
         break;
     }
   }
 
-  bool HasPrefs(MockPrefStoreID store_id) {
-    switch (store_id) {
-      case MOCK_UNPROTECTED_PREF_STORE:
-        return unprotected_prefs_;
-      case MOCK_PROTECTED_PREF_STORE:
-        return protected_prefs_;
-    }
-    NOTREACHED();
-    return false;
-  }
-
   bool StoreModifiedByMigration(MockPrefStoreID store_id) {
     switch (store_id) {
       case MOCK_UNPROTECTED_PREF_STORE:
@@ -256,18 +309,13 @@ class TrackedPreferencesMigrationTest : public testing::Test {
   // Helper given as an InterceptablePrefFilter::FinalizeFilterOnLoadCallback
   // to the migrator to be invoked when it's done.
   void GetPrefsBack(MockPrefStoreID store_id,
-                    scoped_ptr<base::DictionaryValue> prefs,
                     bool prefs_altered) {
     switch (store_id) {
       case MOCK_UNPROTECTED_PREF_STORE:
-        EXPECT_FALSE(unprotected_prefs_);
-        unprotected_prefs_ = prefs.Pass();
         migration_modified_unprotected_store_ = prefs_altered;
         unprotected_store_migration_complete_ = true;
         break;
       case MOCK_PROTECTED_PREF_STORE:
-        EXPECT_FALSE(protected_prefs_);
-        protected_prefs_ = prefs.Pass();
         migration_modified_protected_store_ = prefs_altered;
         protected_store_migration_complete_ = true;
         break;
@@ -278,11 +326,9 @@ class TrackedPreferencesMigrationTest : public testing::Test {
   void RemovePathFromStore(MockPrefStoreID store_id, const std::string& key) {
     switch (store_id) {
       case MOCK_UNPROTECTED_PREF_STORE:
-        ASSERT_TRUE(unprotected_prefs_);
         unprotected_prefs_->RemovePath(key, NULL);
         break;
       case MOCK_PROTECTED_PREF_STORE:
-        ASSERT_TRUE(protected_prefs_);
         protected_prefs_->RemovePath(key, NULL);
         break;
     }
@@ -290,6 +336,9 @@ class TrackedPreferencesMigrationTest : public testing::Test {
 
   scoped_ptr<base::DictionaryValue> unprotected_prefs_;
   scoped_ptr<base::DictionaryValue> protected_prefs_;
+  scoped_ptr<PrefHashStoreImpl> unprotected_pref_hash_store_;
+  scoped_ptr<PrefHashStoreImpl> protected_pref_hash_store_;
+  TestingPrefServiceSimple local_state_;
 
   SimpleInterceptablePrefFilter mock_unprotected_pref_filter_;
   SimpleInterceptablePrefFilter mock_protected_pref_filter_;
@@ -306,7 +355,9 @@ class TrackedPreferencesMigrationTest : public testing::Test {
 
 }  // namespace
 
-TEST_F(TrackedPreferencesMigrationTest, NoMigrationRequired) {
+TEST_F(TrackedPreferencesMigrationTest, NoMigration) {
+  // Note that, in this test, there are no MACs in Local State, so they are not
+  // migrated.
   PresetStoreValue(MOCK_UNPROTECTED_PREF_STORE, kUnprotectedPref,
                    kUnprotectedPrefValue);
   PresetStoreValue(MOCK_PROTECTED_PREF_STORE, kProtectedPref,
@@ -315,8 +366,6 @@ TEST_F(TrackedPreferencesMigrationTest, NoMigrationRequired) {
   // Hand unprotected prefs to the migrator which should wait for the protected
   // prefs.
   HandPrefsToMigrator(MOCK_UNPROTECTED_PREF_STORE);
-  EXPECT_FALSE(HasPrefs(MOCK_UNPROTECTED_PREF_STORE));

[gab, 2014/06/11 18:23:55]

These HasPrefs() tests were important to confirm the appropriate flow of calls
to TrackedPreferencesMigrator::MigrateIfReady(). In your model you'll have to
find another way to confirm that the migrator received the data it needs for
either store (even if it doesn't receive ownership of that data).

-  EXPECT_TRUE(HasPrefs(MOCK_PROTECTED_PREF_STORE));
   EXPECT_FALSE(MigrationCompleted());
 
   // Hand protected prefs to the migrator which should proceed with the
@@ -325,8 +374,6 @@ TEST_F(TrackedPreferencesMigrationTest, NoMigrationRequired) {
   EXPECT_TRUE(MigrationCompleted());
 
   // Prefs should have been handed back over.
-  EXPECT_TRUE(HasPrefs(MOCK_UNPROTECTED_PREF_STORE));
-  EXPECT_TRUE(HasPrefs(MOCK_PROTECTED_PREF_STORE));
   EXPECT_FALSE(
       WasOnSuccessfulWriteCallbackRegistered(MOCK_UNPROTECTED_PREF_STORE));
   EXPECT_FALSE(
@@ -358,15 +405,11 @@ TEST_F(TrackedPreferencesMigrationTest, FullMigration) {
                    kPreviouslyProtectedPrefValue);
 
   HandPrefsToMigrator(MOCK_UNPROTECTED_PREF_STORE);
-  EXPECT_FALSE(HasPrefs(MOCK_UNPROTECTED_PREF_STORE));
-  EXPECT_TRUE(HasPrefs(MOCK_PROTECTED_PREF_STORE));
   EXPECT_FALSE(MigrationCompleted());
 
   HandPrefsToMigrator(MOCK_PROTECTED_PREF_STORE);
   EXPECT_TRUE(MigrationCompleted());
 
-  EXPECT_TRUE(HasPrefs(MOCK_UNPROTECTED_PREF_STORE));
-  EXPECT_TRUE(HasPrefs(MOCK_PROTECTED_PREF_STORE));
   EXPECT_TRUE(
       WasOnSuccessfulWriteCallbackRegistered(MOCK_UNPROTECTED_PREF_STORE));
   EXPECT_TRUE(
@@ -444,7 +487,8 @@ TEST_F(TrackedPreferencesMigrationTest, FullMigration) {
 }
 
 TEST_F(TrackedPreferencesMigrationTest, CleanupOnly) {
-  // Already migrated; only cleanup needed.
+  // Already migrated; cleanup still needed. Note that no MACs are stored in
+  // Local State and, thus, none are migrated.
   PresetStoreValue(
       MOCK_UNPROTECTED_PREF_STORE, kUnprotectedPref, kUnprotectedPrefValue);
   PresetStoreValue(MOCK_UNPROTECTED_PREF_STORE,
@@ -463,15 +507,11 @@ TEST_F(TrackedPreferencesMigrationTest, CleanupOnly) {
                    kPreviouslyUnprotectedPrefValue);
 
   HandPrefsToMigrator(MOCK_UNPROTECTED_PREF_STORE);
-  EXPECT_FALSE(HasPrefs(MOCK_UNPROTECTED_PREF_STORE));
-  EXPECT_TRUE(HasPrefs(MOCK_PROTECTED_PREF_STORE));
   EXPECT_FALSE(MigrationCompleted());
 
   HandPrefsToMigrator(MOCK_PROTECTED_PREF_STORE);
   EXPECT_TRUE(MigrationCompleted());
 
-  EXPECT_TRUE(HasPrefs(MOCK_UNPROTECTED_PREF_STORE));
-  EXPECT_TRUE(HasPrefs(MOCK_PROTECTED_PREF_STORE));
   EXPECT_FALSE(
       WasOnSuccessfulWriteCallbackRegistered(MOCK_UNPROTECTED_PREF_STORE));
   EXPECT_FALSE(