Separate storage for protected preferences into Protected Preferences file.

Integrates the SegregatedPrefStore with chrome_pref_service_factory.cc .

BUG=349158

Review URL: https://codereview.chromium.org/218583003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@261448 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 461 insertions, 57 deletions

diff --git a/chrome/browser/prefs/profile_pref_store_manager.cc b/chrome/browser/prefs/profile_pref_store_manager.cc
index fc350a0..40e63fa 100644
--- a/chrome/browser/prefs/profile_pref_store_manager.cc
+++ b/chrome/browser/prefs/profile_pref_store_manager.cc
@@ -13,12 +13,100 @@
 #include "base/prefs/pref_registry_simple.h"
 #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/common/chrome_constants.h"
 #include "chrome/common/pref_names.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 
 namespace {
 
+// An adaptor that allows a PrefHashStoreImpl to access a preference store
+// directly as a dictionary. Uses an equivalent layout to
+// PrefStoreHashStoreContents.
+class DictionaryHashStoreContents : public HashStoreContents {
+ public:
+  // Instantiates a HashStoreContents that is a copy of |to_copy|. The copy is
+  // mutable but does not affect the original, nor is it persisted to disk in
+  // any other way.
+  explicit DictionaryHashStoreContents(const HashStoreContents& to_copy)
+      : hash_store_id_(to_copy.hash_store_id()),
+        super_mac_(to_copy.GetSuperMac()) {
+    if (to_copy.IsInitialized())
+      dictionary_.reset(to_copy.GetContents()->DeepCopy());
+    int version = 0;
+    if (to_copy.GetVersion(&version))
+      version_.reset(new int(version));
+  }
+
+  // HashStoreContents implementation
+  virtual std::string hash_store_id() const OVERRIDE { return hash_store_id_; }
+
+  virtual void Reset() OVERRIDE {
+    dictionary_.reset();
+    super_mac_.clear();
+    version_.reset();
+  }
+
+  virtual bool IsInitialized() const OVERRIDE {
+    return dictionary_;
+  }
+
+  virtual const base::DictionaryValue* GetContents() const OVERRIDE{
+    return dictionary_.get();
+  }
+
+  virtual scoped_ptr<MutableDictionary> GetMutableContents() OVERRIDE {
+    return scoped_ptr<MutableDictionary>(
+        new SimpleMutableDictionary(this));
+  }
+
+  virtual std::string GetSuperMac() const OVERRIDE { return super_mac_; }
+
+  virtual void SetSuperMac(const std::string& super_mac) OVERRIDE {
+    super_mac_ = super_mac;
+  }
+
+  virtual bool GetVersion(int* version) const OVERRIDE {
+    if (!version_)
+      return false;
+    *version = *version_;
+    return true;
+  }
+
+  virtual void SetVersion(int version) OVERRIDE {
+    version_.reset(new int(version));
+  }
+
+ private:
+  class SimpleMutableDictionary
+      : public HashStoreContents::MutableDictionary {
+   public:
+    explicit SimpleMutableDictionary(DictionaryHashStoreContents* outer)
+        : outer_(outer) {}
+
+    virtual ~SimpleMutableDictionary() {}
+
+    // MutableDictionary implementation
+    virtual base::DictionaryValue* operator->() OVERRIDE {
+      if (!outer_->dictionary_)
+        outer_->dictionary_.reset(new base::DictionaryValue);
+      return outer_->dictionary_.get();
+    }
+
+   private:
+    DictionaryHashStoreContents* outer_;
+
+    DISALLOW_COPY_AND_ASSIGN(SimpleMutableDictionary);
+  };
+
+  const std::string hash_store_id_;
+  std::string super_mac_;
+  scoped_ptr<int> version_;
+  scoped_ptr<base::DictionaryValue> dictionary_;
+
+  DISALLOW_COPY_AND_ASSIGN(DictionaryHashStoreContents);
+};
+
 // An in-memory PrefStore backed by an immutable DictionaryValue.
 class DictionaryPrefStore : public PrefStore {
  public:
@@ -187,15 +275,61 @@ void ProfilePrefStoreManager::ResetPrefHashStore() {
 PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore(
     const scoped_refptr<base::SequencedTaskRunner>& io_task_runner) {
   scoped_ptr<PrefFilter> pref_filter;
-  if (kPlatformSupportsPreferenceTracking) {
-    pref_filter.reset(
-        new PrefHashFilter(GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
-                           tracking_configuration_,
-                           reporting_ids_count_));
+  if (!kPlatformSupportsPreferenceTracking) {
+    return new JsonPrefStore(GetPrefFilePathFromProfilePath(profile_path_),
+                             io_task_runner,
+                             scoped_ptr<PrefFilter>());
   }
-  return new JsonPrefStore(GetPrefFilePathFromProfilePath(profile_path_),
-                           io_task_runner,
-                           pref_filter.Pass());
+
+  std::vector<PrefHashFilter::TrackedPreferenceMetadata>
+      unprotected_configuration;
+  std::vector<PrefHashFilter::TrackedPreferenceMetadata>
+      protected_configuration;
+  std::set<std::string> protected_pref_names;
+  for (std::vector<PrefHashFilter::TrackedPreferenceMetadata>::const_iterator
+           it = tracking_configuration_.begin();
+       it != tracking_configuration_.end();
+       ++it) {
+    if (it->enforcement_level > PrefHashFilter::NO_ENFORCEMENT) {
+      protected_configuration.push_back(*it);
+      protected_pref_names.insert(it->name);
+    } else {
+      unprotected_configuration.push_back(*it);
+    }
+  }
+
+  scoped_ptr<PrefFilter> unprotected_pref_hash_filter(
+      new PrefHashFilter(GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
+                         unprotected_configuration,
+                         reporting_ids_count_));
+  scoped_ptr<PrefFilter> protected_pref_hash_filter(
+      new PrefHashFilter(GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
+                         protected_configuration,
+                         reporting_ids_count_));
+
+  scoped_refptr<PersistentPrefStore> unprotected_pref_store(
+      new JsonPrefStore(GetPrefFilePathFromProfilePath(profile_path_),
+                        io_task_runner,
+                        unprotected_pref_hash_filter.Pass()));
+  scoped_refptr<PersistentPrefStore> protected_pref_store(new JsonPrefStore(
+      profile_path_.Append(chrome::kProtectedPreferencesFilename),
+      io_task_runner,
+      protected_pref_hash_filter.Pass()));
+
+  // The on_initialized callback is used to migrate newly protected values from
+  // the main Preferences store to the Protected Preferences store. It is also
+  // responsible for the initial migration to a two-store model.
+  return new SegregatedPrefStore(
+      unprotected_pref_store,
+      protected_pref_store,
+      protected_pref_names,
+      base::Bind(&PrefHashFilter::MigrateValues,
+                 base::Owned(new PrefHashFilter(
+                     CopyPrefHashStore(),
+                     protected_configuration,
+                     reporting_ids_count_)),
+                 unprotected_pref_store,
+                 protected_pref_store));
 }
 
 void ProfilePrefStoreManager::UpdateProfileHashStoreIfRequired(
@@ -231,6 +365,8 @@ bool ProfilePrefStoreManager::InitializePrefsFromMasterPrefs(
   if (!base::CreateDirectory(profile_path_))
     return false;
 
+  // This will write out to a single combined file which will be immediately
+  // migrated to two files on load.
   JSONFileValueSerializer serializer(
       GetPrefFilePathFromProfilePath(profile_path_));
 
@@ -253,6 +389,21 @@ bool ProfilePrefStoreManager::InitializePrefsFromMasterPrefs(
   return success;
 }
 
+PersistentPrefStore*
+ProfilePrefStoreManager::CreateDeprecatedCombinedProfilePrefStore(
+    const scoped_refptr<base::SequencedTaskRunner>& io_task_runner) {
+  scoped_ptr<PrefFilter> pref_filter;
+  if (kPlatformSupportsPreferenceTracking) {
+    pref_filter.reset(
+        new PrefHashFilter(GetPrefHashStoreImpl().PassAs<PrefHashStore>(),
+                           tracking_configuration_,
+                           reporting_ids_count_));
+  }
+  return new JsonPrefStore(GetPrefFilePathFromProfilePath(profile_path_),
+                           io_task_runner,
+                           pref_filter.Pass());
+}
+
 scoped_ptr<PrefHashStoreImpl> ProfilePrefStoreManager::GetPrefHashStoreImpl() {
   DCHECK(kPlatformSupportsPreferenceTracking);
 
@@ -262,3 +413,15 @@ scoped_ptr<PrefHashStoreImpl> ProfilePrefStoreManager::GetPrefHashStoreImpl() {
       scoped_ptr<HashStoreContents>(new PrefServiceHashStoreContents(
           profile_path_.AsUTF8Unsafe(), local_state_))));
 }
+
+scoped_ptr<PrefHashStore> ProfilePrefStoreManager::CopyPrefHashStore() {
+  DCHECK(kPlatformSupportsPreferenceTracking);
+
+  PrefServiceHashStoreContents real_contents(profile_path_.AsUTF8Unsafe(),
+                                             local_state_);
+  return scoped_ptr<PrefHashStore>(new PrefHashStoreImpl(
+      seed_,
+      device_id_,
+      scoped_ptr<HashStoreContents>(
+          new DictionaryHashStoreContents(real_contents))));
+}
diff --git a/chrome/browser/prefs/profile_pref_store_manager.h b/chrome/browser/prefs/profile_pref_store_manager.h
index f75d3d0..9463d0b 100644
--- a/chrome/browser/prefs/profile_pref_store_manager.h
+++ b/chrome/browser/prefs/profile_pref_store_manager.h
@@ -98,6 +98,11 @@ class ProfilePrefStoreManager {
   bool InitializePrefsFromMasterPrefs(
       const base::DictionaryValue& master_prefs);
 
+  // Creates a single-file PrefStore as was used in M34 and earlier. Used only
+  // for testing migration.
+  PersistentPrefStore* CreateDeprecatedCombinedProfilePrefStore(
+      const scoped_refptr<base::SequencedTaskRunner>& io_task_runner);
+
  private:
   class InitializeHashStoreObserver;
 
@@ -105,6 +110,10 @@ class ProfilePrefStoreManager {
   // if |kPlatformSupportsPreferenceTracking|.
   scoped_ptr<PrefHashStoreImpl> GetPrefHashStoreImpl();
 
+  // Returns a PrefHashStore that is a copy of the current state of the real
+  // hash store.
+  scoped_ptr<PrefHashStore> CopyPrefHashStore();
+
   const base::FilePath profile_path_;
   const std::vector<PrefHashFilter::TrackedPreferenceMetadata>
       tracking_configuration_;
diff --git a/chrome/browser/prefs/profile_pref_store_manager_unittest.cc b/chrome/browser/prefs/profile_pref_store_manager_unittest.cc
index c7bb745..064cab0 100644
--- a/chrome/browser/prefs/profile_pref_store_manager_unittest.cc
+++ b/chrome/browser/prefs/profile_pref_store_manager_unittest.cc
@@ -8,6 +8,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ -15,6 +16,7 @@
 #include "base/prefs/json_pref_store.h"
 #include "base/prefs/persistent_pref_store.h"
 #include "base/prefs/pref_service.h"
+#include "base/prefs/pref_service_factory.h"
 #include "base/prefs/pref_store.h"
 #include "base/prefs/testing_pref_service.h"
 #include "base/run_loop.h"
@@ -60,9 +62,9 @@ class RegistryVerifier : public PrefStore::Observer {
   scoped_refptr<PrefRegistry> pref_registry_;
 };
 
+const char kUnprotectedAtomic[] = "unprotected_atomic";
 const char kTrackedAtomic[] = "tracked_atomic";
 const char kProtectedAtomic[] = "protected_atomic";
-const char kProtectedSplit[] = "protected_split";
 
 const char kFoobar[] = "FOOBAR";
 const char kBarfoo[] = "BARFOO";
@@ -70,12 +72,13 @@ const char kHelloWorld[] = "HELLOWORLD";
 const char kGoodbyeWorld[] = "GOODBYEWORLD";
 
 const PrefHashFilter::TrackedPreferenceMetadata kConfiguration[] = {
-    {0, kTrackedAtomic, PrefHashFilter::NO_ENFORCEMENT,
+    {0u, kTrackedAtomic, PrefHashFilter::NO_ENFORCEMENT,
      PrefHashFilter::TRACKING_STRATEGY_ATOMIC},
-    {1, kProtectedAtomic, PrefHashFilter::ENFORCE_ON_LOAD,
-     PrefHashFilter::TRACKING_STRATEGY_ATOMIC},
-    {2, kProtectedSplit, PrefHashFilter::ENFORCE_ON_LOAD,
-     PrefHashFilter::TRACKING_STRATEGY_SPLIT}};
+    {1u, kProtectedAtomic, PrefHashFilter::ENFORCE_ON_LOAD,
+     PrefHashFilter::TRACKING_STRATEGY_ATOMIC}};
+
+const size_t kExtraReportingId = 2u;
+const size_t kReportingIdCount = 3u;
 
 }  // namespace
 
@@ -101,17 +104,49 @@ class ProfilePrefStoreManagerTest : public testing::Test {
             it->name, user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
       }
     }
+    profile_pref_registry_->RegisterStringPref(
+        kUnprotectedAtomic,
+        std::string(),
+        user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+
     ASSERT_TRUE(profile_dir_.CreateUniqueTempDir());
+    ReloadConfiguration();
+  }
 
+  void ReloadConfiguration() {
     manager_.reset(new ProfilePrefStoreManager(profile_dir_.path(),
                                                configuration_,
-                                               configuration_.size(),
+                                               kReportingIdCount,
                                                "seed",
                                                "device_id",
                                                &local_state_));
   }
 
-  virtual void TearDown() OVERRIDE {
+  virtual void TearDown() OVERRIDE { DestroyPrefStore(); }
+
+ protected:
+  bool WasResetRecorded() {
+    base::PrefServiceFactory pref_service_factory;
+    pref_service_factory.set_user_prefs(pref_store_);
+
+    scoped_ptr<PrefService> pref_service(
+        pref_service_factory.Create(profile_pref_registry_));
+
+    return !ProfilePrefStoreManager::GetResetTime(pref_service.get()).is_null();
+  }
+
+  void InitializePrefs() {
+    // According to the implementation of ProfilePrefStoreManager, this is
+    // actually a SegregatedPrefStore backed by two underlying pref stores.
+    scoped_refptr<PersistentPrefStore> pref_store =
+        manager_->CreateProfilePrefStore(
+            main_message_loop_.message_loop_proxy());
+    InitializePrefStore(pref_store);
+    pref_store = NULL;
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void DestroyPrefStore() {
     if (pref_store_) {
       // Force everything to be written to disk, triggering the PrefHashFilter
       // while our RegistryVerifier is watching.
@@ -126,42 +161,49 @@ class ProfilePrefStoreManagerTest : public testing::Test {
     }
   }
 
- protected:
-  void InitializePrefs() {
+  void InitializeDeprecatedCombinedProfilePrefStore() {
     scoped_refptr<PersistentPrefStore> pref_store =
-        manager_->CreateProfilePrefStore(
+        manager_->CreateDeprecatedCombinedProfilePrefStore(
             main_message_loop_.message_loop_proxy());
+    InitializePrefStore(pref_store);
+    pref_store = NULL;
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void InitializePrefStore(PersistentPrefStore* pref_store) {
     pref_store->AddObserver(&registry_verifier_);
     PersistentPrefStore::PrefReadError error = pref_store->ReadPrefs();
-    ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NO_FILE, error);
+    EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NO_FILE, error);
     pref_store->SetValue(kTrackedAtomic, new base::StringValue(kFoobar));
     pref_store->SetValue(kProtectedAtomic, new base::StringValue(kHelloWorld));
+    pref_store->SetValue(kUnprotectedAtomic, new base::StringValue(kFoobar));
     pref_store->RemoveObserver(&registry_verifier_);
-    pref_store = NULL;
+    pref_store->CommitPendingWrite();
     base::RunLoop().RunUntilIdle();
   }
 
   void LoadExistingPrefs() {
+    DestroyPrefStore();
     pref_store_ = manager_->CreateProfilePrefStore(
         main_message_loop_.message_loop_proxy());
     pref_store_->AddObserver(&registry_verifier_);
-    EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE,
-              pref_store_->ReadPrefs());
+    pref_store_->ReadPrefs();
   }
 
   void ReplaceStringInPrefs(const std::string& find,
                             const std::string& replace) {
-    // Tamper with the file's contents
-    base::FilePath pref_file_path =
-        ProfilePrefStoreManager::GetPrefFilePathFromProfilePath(
-            profile_dir_.path());
-    std::string pref_file_contents;
-    EXPECT_TRUE(base::ReadFileToString(pref_file_path, &pref_file_contents));
-    ReplaceSubstringsAfterOffset(&pref_file_contents, 0u, find, replace);
-    EXPECT_EQ(static_cast<int>(pref_file_contents.length()),
-              base::WriteFile(pref_file_path,
-                              pref_file_contents.c_str(),
-                              pref_file_contents.length()));
+    base::FileEnumerator file_enum(
+        profile_dir_.path(), true, base::FileEnumerator::FILES);
+
+    for (base::FilePath path = file_enum.Next(); !path.empty();
+         path = file_enum.Next()) {
+      // Tamper with the file's contents
+      std::string contents;
+      EXPECT_TRUE(base::ReadFileToString(path, &contents));
+      ReplaceSubstringsAfterOffset(&contents, 0u, find, replace);
+      EXPECT_EQ(static_cast<int>(contents.length()),
+                base::WriteFile(path, contents.c_str(), contents.length()));
+    }
   }
 
   void ExpectStringValueEquals(const std::string& name,
@@ -194,6 +236,7 @@ TEST_F(ProfilePrefStoreManagerTest, StoreValues) {
 
   ExpectStringValueEquals(kTrackedAtomic, kFoobar);
   ExpectStringValueEquals(kProtectedAtomic, kHelloWorld);
+  EXPECT_FALSE(WasResetRecorded());
 }
 
 TEST_F(ProfilePrefStoreManagerTest, GetPrefFilePathFromProfilePath) {
@@ -201,11 +244,11 @@ TEST_F(ProfilePrefStoreManagerTest, GetPrefFilePathFromProfilePath) {
       ProfilePrefStoreManager::GetPrefFilePathFromProfilePath(
           profile_dir_.path());
 
-  ASSERT_FALSE(base::PathExists(pref_file_path));
+  EXPECT_FALSE(base::PathExists(pref_file_path));
 
   InitializePrefs();
 
-  ASSERT_TRUE(base::PathExists(pref_file_path));
+  EXPECT_TRUE(base::PathExists(pref_file_path));
 }
 
 TEST_F(ProfilePrefStoreManagerTest, ProtectValues) {
@@ -222,8 +265,10 @@ TEST_F(ProfilePrefStoreManagerTest, ProtectValues) {
 
   // If preference tracking is supported, the tampered value of kProtectedAtomic
   // will be discarded at load time, leaving this preference undefined.
-  EXPECT_EQ(!ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+  EXPECT_NE(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
             pref_store_->GetValue(kProtectedAtomic, NULL));
+  EXPECT_EQ(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+            WasResetRecorded());
 }
 
 TEST_F(ProfilePrefStoreManagerTest, ResetPrefHashStore) {
@@ -237,8 +282,10 @@ TEST_F(ProfilePrefStoreManagerTest, ResetPrefHashStore) {
   ExpectStringValueEquals(kTrackedAtomic, kFoobar);
   // If preference tracking is supported, the tampered value of kProtectedAtomic
   // will be discarded at load time, leaving this preference undefined.
-  EXPECT_EQ(!ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+  EXPECT_NE(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
             pref_store_->GetValue(kProtectedAtomic, NULL));
+  EXPECT_EQ(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+            WasResetRecorded());
 }
 
 TEST_F(ProfilePrefStoreManagerTest, ResetAllPrefHashStores) {
@@ -252,14 +299,32 @@ TEST_F(ProfilePrefStoreManagerTest, ResetAllPrefHashStores) {
   ExpectStringValueEquals(kTrackedAtomic, kFoobar);
   // If preference tracking is supported, kProtectedAtomic will be undefined
   // because the value was discarded due to loss of the hash store contents.
-  EXPECT_EQ(!ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+  EXPECT_NE(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
             pref_store_->GetValue(kProtectedAtomic, NULL));
+  EXPECT_EQ(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+            WasResetRecorded());
 }
 
-TEST_F(ProfilePrefStoreManagerTest, UpdateProfileHashStoreIfRequired) {
-  InitializePrefs();
+TEST_F(ProfilePrefStoreManagerTest, MigrateFromOneFile) {
+  InitializeDeprecatedCombinedProfilePrefStore();
 
-  manager_->ResetPrefHashStore();
+  LoadExistingPrefs();
+
+  ExpectStringValueEquals(kTrackedAtomic, kFoobar);
+  ExpectStringValueEquals(kProtectedAtomic, kHelloWorld);
+  EXPECT_FALSE(WasResetRecorded());
+}
+
+TEST_F(ProfilePrefStoreManagerTest, UpdateProfileHashStoreIfRequired) {
+  scoped_refptr<JsonPrefStore> legacy_prefs(
+      new JsonPrefStore(ProfilePrefStoreManager::GetPrefFilePathFromProfilePath(
+                            profile_dir_.path()),
+                        main_message_loop_.message_loop_proxy(),
+                        scoped_ptr<PrefFilter>()));
+  legacy_prefs->SetValue(kTrackedAtomic, new base::StringValue(kFoobar));
+  legacy_prefs->SetValue(kProtectedAtomic, new base::StringValue(kHelloWorld));
+  legacy_prefs = NULL;
+  base::RunLoop().RunUntilIdle();
 
   // This is a no-op if !kPlatformSupportsPreferenceTracking.
   manager_->UpdateProfileHashStoreIfRequired(
@@ -273,6 +338,7 @@ TEST_F(ProfilePrefStoreManagerTest, UpdateProfileHashStoreIfRequired) {
   // These expectations hold whether or not tracking is supported.
   ExpectStringValueEquals(kTrackedAtomic, kFoobar);
   ExpectStringValueEquals(kProtectedAtomic, kHelloWorld);
+  EXPECT_FALSE(WasResetRecorded());
 }
 
 TEST_F(ProfilePrefStoreManagerTest, InitializePrefsFromMasterPrefs) {
@@ -280,7 +346,7 @@ TEST_F(ProfilePrefStoreManagerTest, InitializePrefsFromMasterPrefs) {
       new base::DictionaryValue);
   master_prefs->Set(kTrackedAtomic, new base::StringValue(kFoobar));
   master_prefs->Set(kProtectedAtomic, new base::StringValue(kHelloWorld));
-  ASSERT_TRUE(
+  EXPECT_TRUE(
       manager_->InitializePrefsFromMasterPrefs(*master_prefs));
 
   LoadExistingPrefs();
@@ -289,4 +355,113 @@ TEST_F(ProfilePrefStoreManagerTest, InitializePrefsFromMasterPrefs) {
   // necessary to authenticate these values.
   ExpectStringValueEquals(kTrackedAtomic, kFoobar);
   ExpectStringValueEquals(kProtectedAtomic, kHelloWorld);
+  EXPECT_FALSE(WasResetRecorded());
+}
+
+TEST_F(ProfilePrefStoreManagerTest, UnprotectedToProtected) {
+  InitializePrefs();
+  LoadExistingPrefs();
+  ExpectStringValueEquals(kUnprotectedAtomic, kFoobar);
+
+  // Ensure everything is written out to disk.
+  DestroyPrefStore();
+
+  ReplaceStringInPrefs(kFoobar, kBarfoo);
+
+  // It's unprotected, so we can load the modified value.
+  LoadExistingPrefs();
+  ExpectStringValueEquals(kUnprotectedAtomic, kBarfoo);
+
+  // Now update the configuration to protect it.
+  PrefHashFilter::TrackedPreferenceMetadata new_protected = {
+      kExtraReportingId, kUnprotectedAtomic, PrefHashFilter::ENFORCE_ON_LOAD,
+      PrefHashFilter::TRACKING_STRATEGY_ATOMIC};
+  configuration_.push_back(new_protected);
+  ReloadConfiguration();
+
+  // And try loading with the new configuration.
+  LoadExistingPrefs();
+
+  // Since there was a valid super MAC we were able to extend the existing trust
+  // to the newly proteted preference.
+  ExpectStringValueEquals(kUnprotectedAtomic, kBarfoo);
+  EXPECT_FALSE(WasResetRecorded());
+
+  // Ensure everything is written out to disk.
+  DestroyPrefStore();
+
+  // It's protected now, so (if the platform supports it) any tampering should
+  // lead to a reset.
+  ReplaceStringInPrefs(kBarfoo, kFoobar);
+  LoadExistingPrefs();
+  EXPECT_NE(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+            pref_store_->GetValue(kUnprotectedAtomic, NULL));
+  EXPECT_EQ(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+            WasResetRecorded());
+}
+
+TEST_F(ProfilePrefStoreManagerTest, UnprotectedToProtectedWithoutTrust) {
+  InitializePrefs();
+
+  // Now update the configuration to protect it.
+  PrefHashFilter::TrackedPreferenceMetadata new_protected = {
+      kExtraReportingId, kUnprotectedAtomic, PrefHashFilter::ENFORCE_ON_LOAD,
+      PrefHashFilter::TRACKING_STRATEGY_ATOMIC};
+  configuration_.push_back(new_protected);
+  ReloadConfiguration();
+  ProfilePrefStoreManager::ResetAllPrefHashStores(&local_state_);
+
+  // And try loading with the new configuration.
+  LoadExistingPrefs();
+
+  // If preference tracking is supported, kUnprotectedAtomic will have been
+  // discarded because new values are not accepted without a valid super MAC.
+  EXPECT_NE(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+            pref_store_->GetValue(kUnprotectedAtomic, NULL));
+  EXPECT_EQ(ProfilePrefStoreManager::kPlatformSupportsPreferenceTracking,
+            WasResetRecorded());
+}
+
+// This test does not directly verify that the values are moved from one pref
+// store to the other. segregated_pref_store_unittest.cc _does_ verify that
+// functionality.
+//
+// _This_ test verifies that preference values are correctly maintained when a
+// preference's protection state changes from protected to unprotected.
+TEST_F(ProfilePrefStoreManagerTest, ProtectedToUnprotected) {
+  InitializePrefs();
+  DestroyPrefStore();
+
+  // Unconfigure protection for kProtectedAtomic
+  for (std::vector<PrefHashFilter::TrackedPreferenceMetadata>::iterator it =
+           configuration_.begin();
+       it != configuration_.end();
+       ++it) {
+    if (it->name == kProtectedAtomic) {
+      configuration_.erase(it);
+      break;
+    }
+  }
+  ReloadConfiguration();
+
+  // Reset the hash stores and then try loading the prefs.
+  ProfilePrefStoreManager::ResetAllPrefHashStores(&local_state_);
+  LoadExistingPrefs();
+
+  // Verify that the value was not reset.
+  ExpectStringValueEquals(kProtectedAtomic, kHelloWorld);
+  EXPECT_FALSE(WasResetRecorded());
+
+  // Accessing the value of the previously protected pref didn't trigger its
+  // move to the unprotected preferences file, though the loading of the pref
+  // store should still have caused the MAC store to be recalculated.
+  LoadExistingPrefs();
+  ExpectStringValueEquals(kProtectedAtomic, kHelloWorld);
+
+  // Trigger the logic that migrates it back to the unprotected preferences
+  // file.
+  pref_store_->SetValue(kProtectedAtomic, new base::StringValue(kGoodbyeWorld));
+  LoadExistingPrefs();
+  ExpectStringValueEquals(kProtectedAtomic, kGoodbyeWorld);
+  EXPECT_FALSE(WasResetRecorded());
 }
diff --git a/chrome/browser/prefs/tracked/segregated_pref_store.cc b/chrome/browser/prefs/tracked/segregated_pref_store.cc
index 5c51da9..13d5ecf 100644
--- a/chrome/browser/prefs/tracked/segregated_pref_store.cc
+++ b/chrome/browser/prefs/tracked/segregated_pref_store.cc
@@ -119,20 +119,20 @@ bool SegregatedPrefStore::ReadOnly() const {
 PersistentPrefStore::PrefReadError SegregatedPrefStore::GetReadError() const {
   PersistentPrefStore::PrefReadError read_error =
       default_pref_store_->GetReadError();
-  return read_error != PersistentPrefStore::PREF_READ_ERROR_NONE
-             ? read_error
-             : selected_pref_store_->GetReadError();
+  if (read_error == PersistentPrefStore::PREF_READ_ERROR_NONE) {
+    read_error = selected_pref_store_->GetReadError();
+    // Ignore NO_FILE from selected_pref_store_.
+    if (read_error == PersistentPrefStore::PREF_READ_ERROR_NO_FILE)
+      read_error = PersistentPrefStore::PREF_READ_ERROR_NONE;
+  }
+  return read_error;
 }
 
 PersistentPrefStore::PrefReadError SegregatedPrefStore::ReadPrefs() {
-  PersistentPrefStore::PrefReadError unselected_read_error =
-      default_pref_store_->ReadPrefs();
-  PersistentPrefStore::PrefReadError selected_read_error =
-      selected_pref_store_->ReadPrefs();
-
-  return unselected_read_error != PersistentPrefStore::PREF_READ_ERROR_NONE
-             ? unselected_read_error
-             : selected_read_error;
+  default_pref_store_->ReadPrefs();
+  selected_pref_store_->ReadPrefs();
+
+  return GetReadError();
 }
 
 void SegregatedPrefStore::ReadPrefsAsync(ReadErrorDelegate* error_delegate) {
diff --git a/chrome/browser/prefs/tracked/segregated_pref_store.h b/chrome/browser/prefs/tracked/segregated_pref_store.h
index b7bc6e9..f2f701b 100644
--- a/chrome/browser/prefs/tracked/segregated_pref_store.h
+++ b/chrome/browser/prefs/tracked/segregated_pref_store.h
@@ -18,6 +18,16 @@
 // Provides a unified PersistentPrefStore implementation that splits its storage
 // and retrieval between two underlying PersistentPrefStore instances: a set of
 // preference names is used to partition the preferences.
+//
+// Combines properties of the two stores as follows:
+//   * The unified read error will be:
+//                           Selected Store Error
+//    Default Store Error | NO_ERROR      | NO_FILE       | other selected |
+//               NO_ERROR | NO_ERROR      | NO_ERROR      | other selected |
+//               NO_FILE  | NO_FILE       | NO_FILE       | NO_FILE        |
+//          other default | other default | other default | other default  |
+//   * The unified initialization success, initialization completion, and
+//     read-only state are the boolean OR of the underlying stores' properties.
 class SegregatedPrefStore : public PersistentPrefStore {
  public:
   // Creates an instance that delegates to |selected_pref_store| for the
diff --git a/chrome/browser/prefs/tracked/segregated_pref_store_unittest.cc b/chrome/browser/prefs/tracked/segregated_pref_store_unittest.cc
index ab91dfc..f1c27c7 100644
--- a/chrome/browser/prefs/tracked/segregated_pref_store_unittest.cc
+++ b/chrome/browser/prefs/tracked/segregated_pref_store_unittest.cc
@@ -184,8 +184,52 @@ TEST_F(SegregatedPrefStoreTest, Observer) {
   observer_.VerifyAndResetChangedKey(kUnselectedPref);
 }
 
+TEST_F(SegregatedPrefStoreTest, SelectedPrefReadNoFileError) {
+  // PREF_READ_ERROR_NO_FILE for the selected prefs file is silently converted
+  // to PREF_READ_ERROR_NONE.
+  selected_store_->set_read_error(
+      PersistentPrefStore::PREF_READ_ERROR_NO_FILE);
+  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE,
+            segregated_store_->ReadPrefs());
+  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE,
+            segregated_store_->GetReadError());
+}
+
 TEST_F(SegregatedPrefStoreTest, SelectedPrefReadError) {
   selected_store_->set_read_error(
+      PersistentPrefStore::PREF_READ_ERROR_ACCESS_DENIED);
+  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_ACCESS_DENIED,
+            segregated_store_->ReadPrefs());
+  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_ACCESS_DENIED,
+            segregated_store_->GetReadError());
+}
+
+TEST_F(SegregatedPrefStoreTest, SelectedPrefReadNoFileErrorAsync) {
+  // PREF_READ_ERROR_NO_FILE for the selected prefs file is silently converted
+  // to PREF_READ_ERROR_NONE.
+  selected_store_->set_read_error(
+      PersistentPrefStore::PREF_READ_ERROR_NO_FILE);
+
+  default_store_->SetBlockAsyncRead(true);
+
+  EXPECT_FALSE(read_error_delegate_data_.invoked);
+
+  segregated_store_->ReadPrefsAsync(GetReadErrorDelegate().release());
+
+  EXPECT_FALSE(read_error_delegate_data_.invoked);
+
+  default_store_->SetBlockAsyncRead(false);
+
+  // ReadErrorDelegate is not invoked for ERROR_NONE.
+  EXPECT_FALSE(read_error_delegate_data_.invoked);
+  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE,
+            segregated_store_->GetReadError());
+  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE,
+            segregated_store_->GetReadError());
+}
+
+TEST_F(SegregatedPrefStoreTest, UnselectedPrefReadNoFileError) {
+  default_store_->set_read_error(
       PersistentPrefStore::PREF_READ_ERROR_NO_FILE);
   EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NO_FILE,
             segregated_store_->ReadPrefs());
@@ -195,10 +239,10 @@ TEST_F(SegregatedPrefStoreTest, SelectedPrefReadError) {
 
 TEST_F(SegregatedPrefStoreTest, UnselectedPrefReadError) {
   default_store_->set_read_error(
-      PersistentPrefStore::PREF_READ_ERROR_NO_FILE);
-  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NO_FILE,
+      PersistentPrefStore::PREF_READ_ERROR_ACCESS_DENIED);
+  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_ACCESS_DENIED,
             segregated_store_->ReadPrefs());
-  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NO_FILE,
+  EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_ACCESS_DENIED,
             segregated_store_->GetReadError());
 }
 
diff --git a/chrome/common/chrome_constants.cc b/chrome/common/chrome_constants.cc
index 53a35eb..92d88e7 100644
--- a/chrome/common/chrome_constants.cc
+++ b/chrome/common/chrome_constants.cc
@@ -179,6 +179,8 @@ const base::FilePath::CharType kNewTabThumbnailsFilename[] =
     FPL("Top Thumbnails");
 const base::FilePath::CharType kOBCertFilename[] = FPL("Origin Bound Certs");
 const base::FilePath::CharType kPreferencesFilename[] = FPL("Preferences");
+const base::FilePath::CharType kProtectedPreferencesFilename[] =
+    FPL("Protected Preferences");
 const base::FilePath::CharType kReadmeFilename[] = FPL("README");
 const base::FilePath::CharType kResetPromptMementoFilename[] =
     FPL("Reset Prompt Memento");
diff --git a/chrome/common/chrome_constants.h b/chrome/common/chrome_constants.h
index e3c5111..638edbc 100644
--- a/chrome/common/chrome_constants.h
+++ b/chrome/common/chrome_constants.h
@@ -77,6 +77,7 @@ extern const base::FilePath::CharType kMediaCacheDirname[];
 extern const base::FilePath::CharType kNewTabThumbnailsFilename[];
 extern const base::FilePath::CharType kOBCertFilename[];
 extern const base::FilePath::CharType kPreferencesFilename[];
+extern const base::FilePath::CharType kProtectedPreferencesFilename[];
 extern const base::FilePath::CharType kReadmeFilename[];
 extern const base::FilePath::CharType kResetPromptMementoFilename[];
 extern const base::FilePath::CharType kSafeBrowsingBaseFilename[];