Separate storage for protected preferences into Protected Preferences file.

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 019b9306465f69be0aad5542f66da33cb0ae81f8..fc350a04902b467d889d39525de8ece17c21cc98 100644
--- a/chrome/browser/prefs/profile_pref_store_manager.cc
+++ b/chrome/browser/prefs/profile_pref_store_manager.cc
@@ -158,11 +158,24 @@ void ProfilePrefStoreManager::ResetAllPrefHashStores(PrefService* local_state) {
 
 //  static
 base::Time ProfilePrefStoreManager::GetResetTime(PrefService* pref_service) {
+  // It's a bit of a coincidence that this (and ClearResetTime) work(s). The

[gab, 2014/04/01 18:55:06]

:( -- to avoid all of this madness how about we use PrefService to Set/Get (we
can't do this directly at the time of Set() since the PrefService doesn't exist
yet, but we could post a task to have an anonymous method kick in to do this
properly when the message loop starts running).

Feels cleaner than depending so tightly on the implementation for this to work.

[erikwright (departed), 2014/04/01 19:46:08]

The current implementation is cleaner, easier to understand, and has less moving
parts than what you propose.

If I were really worried I would just expose the name of the pref from
PrefHashFilter and then add it to the list of selected prefs that I pass to the
segregated pref store. Then I would know for certain that it was stored in the
right place.

But it's not really tightly coupled to anything too far removed. Remember that
it is _this_ class (ProfilePrefStoreManager) that is responsible for doing the
configuration - so if the behaviour changes, this class will be part of the
change.

Finally, there are unit tests that prevent unintended regressions of this
functionality.

[gab, 2014/04/02 17:02:03]

Okay, makes sense, I agree with you now that I understand the
SegregatedPrefStore (I hadn't reviewed that change yet when I wrote this comment
originally).

This comment might sound more alarming than it needs to be, but that's okay.

+  // PrefHashFilter attached to the protected pref store will store the reset
+  // time directly in the protected pref store without going through the
+  // SegregatedPrefStore.
+
+  // PrefHashFilter::GetResetTime will read the value through the pref service,
+  // and thus through the SegregatedPrefStore. Even though it's not listed as
+  // "protected" it will be read from the protected store prefentially to the

[gab, 2014/04/01 18:55:06]

s/prefentially/preferentially

+  // (NULL) value in the unprotected pref store.
   return PrefHashFilter::GetResetTime(pref_service);
 }
 
 // static
 void ProfilePrefStoreManager::ClearResetTime(PrefService* pref_service) {
+  // PrefHashFilter::ClearResetTime will clear the value through the pref
+  // service, and thus through the SegregatedPrefStore. Since it's not listed as
+  // "protected" it will be migrated from the protected store to the unprotected
+  // pref store before being deleted from the latter.
   PrefHashFilter::ClearResetTime(pref_service);
 }