Introduce a new framework for back-and-forth tracked preference migration
between Protected Preferences and unprotected Preferences.
Migration from unprotected Preferences to Protected Preferences was previously
done after both stores had been initialized. This was inherently incorrect as
some operations (PrefHashFilter::FilterOnLoad) would occur before the values
had been moved to the proper store. It also introduced a weird method in
PrefHashFilter::MigrateValues which required an independent PrefHashFilter
(backed by a copy of the real PrefHashStore). This after-the-fact migration
caused Settings.TrackedPreferenceCleared spikes when changing a value from
being enforced to not being enforced (as we'd have a MAC, but no value yet in
this store when running FilterOnLoad()) and more importantly it also caused
issue 365769 -- both of these issues highlight the incorrectness of the
current approach.
The migration back from Protected Preferences to unprotected Preferences when
enforcement was disabled was using yet another mechanism which would only kick
in when a given pref was written to (ref. old non-const
SegregatedPrefStore::StoreForKey()).
The new framework intercepts PrefFilter::FilterOnLoad() events for both stores
and does the back-and-forth migration in place before it even hands them back
to the PrefFilter::FinalizeFilterOnLoad() which then hands it back to the
JsonPrefStores (so that they are agnostic to the migration; from their point
of view their values were always in their store as they received it).
Furthermore, this new framework will easily allow us to later move MACs out of
Local State into their respective stores (which is a task on our radar which
we currently have no easy way to accomplish).
The new framework also handles read errors better. For example, it was
previously possible for the unprotected->protected migration to result in data
loss if the protected store was somehow read-only from a read error while the
unprotected store wasn't -- resulting in an in-memory migration only flushed
to disk in the store from which the value was deleted... The new framework
handles those cases, preferring temporary data duplication over potential data
loss (duplicated data is cleaned up once confirmation is obtained that the new
authority for this data has been successfully written to disk -- it will even
try again in following Chrome runs if it doesn't succeed in this one).
Note: This CL helped LSAN discover an existing leak in post_task_and_reply_impl.cc, see issue 371974 for details.
BUG=365769, 371974
TEST=
A) Make sure all kTrackedPrefs consistently report
Settings.TrackedPreferenceUnchanged across changes from various enforcement
levels (using --force-fieldtrials).
B) Make sure the prefs are properly migrated to their new store (and
subsequently cleaned up from their old store) when changing the
enforcement_level across multiple runs.
C) Make sure prefs are properly migrated in a quick startup/shutdown with a
new enforcement_level and that their old value is properly cleaned up in a
subsequent startup at the same enforcement_level (or re-migrated at another
enforcement_level).
R=bauerb@chromium.org, robertshield@chromium.org, stuartmorgan@chromium.org, thakis@chromium.org
Initially Committed in: https://src.chromium.org/viewvc/chrome?view=rev&revision=269346
Reverted in: https://src.chromium.org/viewvc/chrome?view=rev&revision=269367
Review URL: https://codereview.chromium.org/257003007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269415 0039d316-1c4b-4281-b951-d872f2087c98
27 files changed