Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(36)

Unified Diff: chrome/browser/prefs/tracked/tracked_preferences_migration.cc

Issue 324493002: Move preference MACs to the protected preference stores. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Comment typo. Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/prefs/tracked/tracked_preferences_migration.cc
diff --git a/chrome/browser/prefs/tracked/tracked_preferences_migration.cc b/chrome/browser/prefs/tracked/tracked_preferences_migration.cc
index c358256f06b4f50ff9e88b6045c82634d2b41586..382c924bfea130bf7ea6b07235ef2beb53254a7f 100644
--- a/chrome/browser/prefs/tracked/tracked_preferences_migration.cc
+++ b/chrome/browser/prefs/tracked/tracked_preferences_migration.cc
@@ -8,9 +8,13 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram.h"
#include "base/values.h"
#include "chrome/browser/prefs/interceptable_pref_filter.h"
+#include "chrome/browser/prefs/pref_hash_store.h"
+#include "chrome/browser/prefs/pref_hash_store_transaction.h"
+#include "chrome/browser/prefs/tracked/dictionary_hash_store_contents.h"
+#include "chrome/browser/prefs/tracked/hash_store_contents.h"
namespace {
@@ -28,6 +32,9 @@ class TrackedPreferencesMigrator
register_on_successful_unprotected_store_write_callback,
const base::Callback<void(const base::Closure&)>&
register_on_successful_protected_store_write_callback,
+ scoped_ptr<PrefHashStore> unprotected_pref_hash_store,
+ scoped_ptr<PrefHashStore> protected_pref_hash_store,
+ scoped_ptr<HashStoreContents> legacy_pref_hash_store,
InterceptablePrefFilter* unprotected_pref_filter,
InterceptablePrefFilter* protected_pref_filter);
@@ -68,6 +75,10 @@ class TrackedPreferencesMigrator
InterceptablePrefFilter::FinalizeFilterOnLoadCallback
finalize_protected_filter_on_load_;
+ scoped_ptr<PrefHashStore> unprotected_pref_hash_store_;
+ scoped_ptr<PrefHashStore> protected_pref_hash_store_;
+ scoped_ptr<HashStoreContents> legacy_pref_hash_store_;
+
scoped_ptr<base::DictionaryValue> unprotected_prefs_;
scoped_ptr<base::DictionaryValue> protected_prefs_;
@@ -104,37 +115,87 @@ void ScheduleSourcePrefStoreCleanup(
}
}
+// Removes hashes for |migrated_pref_names| from |origin_pref_store| using
+// the configuration/implementation in |origin_pref_hash_store|.
+void CleanupMigratedHashes(const std::set<std::string>& migrated_pref_names,
+ PrefHashStore* origin_pref_hash_store,
+ base::DictionaryValue* origin_pref_store) {
+ scoped_ptr<PrefHashStoreTransaction> transaction(
+ origin_pref_hash_store->BeginTransaction(scoped_ptr<HashStoreContents>(
+ new DictionaryHashStoreContents(origin_pref_store))));
+ for (std::set<std::string>::const_iterator it = migrated_pref_names.begin();
+ it != migrated_pref_names.end();
+ ++it) {
+ transaction->ClearHash(*it);
+ }
+}
+
// Copies the value of each pref in |pref_names| which is set in |old_store|,
// but not in |new_store| into |new_store|. Sets |old_store_needs_cleanup| to
// true if any old duplicates remain in |old_store| and sets |new_store_altered|
// to true if any value was copied to |new_store|.
void MigratePrefsFromOldToNewStore(const std::set<std::string>& pref_names,
- const base::DictionaryValue* old_store,
+ base::DictionaryValue* old_store,
base::DictionaryValue* new_store,
+ PrefHashStore* new_hash_store,
+ HashStoreContents* legacy_hash_store,
bool* old_store_needs_cleanup,
- bool* new_store_altered) {
+ bool* new_store_altered,
+ bool* used_legacy_pref_hashes) {
+ const base::DictionaryValue* old_hash_store_contents =
+ DictionaryHashStoreContents(old_store).GetContents();
+ const base::DictionaryValue* legacy_hash_store_contents =
+ legacy_hash_store->GetContents();
+ scoped_ptr<PrefHashStoreTransaction> new_hash_store_transaction(
+ new_hash_store->BeginTransaction(scoped_ptr<HashStoreContents>(
+ new DictionaryHashStoreContents(new_store))));
+
for (std::set<std::string>::const_iterator it = pref_names.begin();
- it != pref_names.end(); ++it) {
- const std::string& pref_name = *it;
-
- const base::Value* value_in_old_store = NULL;
- if (!old_store->Get(pref_name, &value_in_old_store))
- continue;
-
- // Whether this value ends up being copied below or was left behind by a
- // previous incomplete migration, it should be cleaned up.
- *old_store_needs_cleanup = true;
-
- if (new_store->Get(pref_name, NULL))
- continue;
-
- // Copy the value from |old_store| to |new_store| rather than moving it to
- // avoid data loss should |old_store| be flushed to disk without |new_store|
- // having equivalently been successfully flushed to disk (e.g., on crash or
- // in cases where |new_store| is read-only following a read error on
- // startup).
- new_store->Set(pref_name, value_in_old_store->DeepCopy());
- *new_store_altered = true;
+ it != pref_names.end();
+ ++it) {
+ const std::string& pref_name = *it;
+ const base::Value* value_in_old_store = NULL;
+
+ // If the destination does not have a hash for this pref we will
+ // unconditionally attempt to move it.
+ bool destination_hash_missing =
+ !new_hash_store_transaction->HasHash(pref_name);
+ // If we migrate the value we will also attempt to migrate the hash.
+ bool migrated_value = false;
+ if (old_store->Get(pref_name, &value_in_old_store)) {
+ // Whether this value ends up being copied below or was left behind by a
+ // previous incomplete migration, it should be cleaned up.
+ *old_store_needs_cleanup = true;
+
+ if (!new_store->Get(pref_name, NULL)) {
+ // Copy the value from |old_store| to |new_store| rather than moving it
+ // to avoid data loss should |old_store| be flushed to disk without
+ // |new_store| having equivalently been successfully flushed to disk
+ // (e.g., on crash or in cases where |new_store| is read-only following
+ // a read error on startup).
+ new_store->Set(pref_name, value_in_old_store->DeepCopy());
+ migrated_value = true;
+ *new_store_altered = true;
+ }
+ }
+
+ if (destination_hash_missing || migrated_value) {
+ const base::Value* old_hash = NULL;
+ if (old_hash_store_contents)
+ old_hash_store_contents->Get(pref_name, &old_hash);
+ if (!old_hash && legacy_hash_store_contents) {
+ legacy_hash_store_contents->Get(pref_name, &old_hash);
+ if (old_hash)
+ *used_legacy_pref_hashes = true;
+ }
+ if (old_hash) {
+ new_hash_store_transaction->ImportHash(pref_name, old_hash);
+ *new_store_altered = true;
+ } else if (!destination_hash_missing) {
+ new_hash_store_transaction->ClearHash(pref_name);
+ *new_store_altered = true;
+ }
+ }
}
}
@@ -148,6 +209,9 @@ TrackedPreferencesMigrator::TrackedPreferencesMigrator(
register_on_successful_unprotected_store_write_callback,
const base::Callback<void(const base::Closure&)>&
register_on_successful_protected_store_write_callback,
+ scoped_ptr<PrefHashStore> unprotected_pref_hash_store,
+ scoped_ptr<PrefHashStore> protected_pref_hash_store,
+ scoped_ptr<HashStoreContents> legacy_pref_hash_store,
InterceptablePrefFilter* unprotected_pref_filter,
InterceptablePrefFilter* protected_pref_filter)
: unprotected_pref_names_(unprotected_pref_names),
@@ -157,7 +221,10 @@ TrackedPreferencesMigrator::TrackedPreferencesMigrator(
register_on_successful_unprotected_store_write_callback_(
register_on_successful_unprotected_store_write_callback),
register_on_successful_protected_store_write_callback_(
- register_on_successful_protected_store_write_callback) {
+ register_on_successful_protected_store_write_callback),
+ unprotected_pref_hash_store_(unprotected_pref_hash_store.Pass()),
+ protected_pref_hash_store_(protected_pref_hash_store.Pass()),
+ legacy_pref_hash_store_(legacy_pref_hash_store.Pass()) {
// The callbacks bound below will own this TrackedPreferencesMigrator by
// reference.
unprotected_pref_filter->InterceptNextFilterOnLoad(
@@ -196,20 +263,45 @@ void TrackedPreferencesMigrator::MigrateIfReady() {
if (!protected_prefs_ || !unprotected_prefs_)
return;
+ bool used_legacy_pref_hashes = false;
bool protected_prefs_need_cleanup = false;
bool unprotected_prefs_altered = false;
MigratePrefsFromOldToNewStore(unprotected_pref_names_,
protected_prefs_.get(),
unprotected_prefs_.get(),
+ unprotected_pref_hash_store_.get(),
+ legacy_pref_hash_store_.get(),
&protected_prefs_need_cleanup,
- &unprotected_prefs_altered);
+ &unprotected_prefs_altered,
+ &used_legacy_pref_hashes);
bool unprotected_prefs_need_cleanup = false;
bool protected_prefs_altered = false;
MigratePrefsFromOldToNewStore(protected_pref_names_,
unprotected_prefs_.get(),
protected_prefs_.get(),
+ protected_pref_hash_store_.get(),
+ legacy_pref_hash_store_.get(),
&unprotected_prefs_need_cleanup,
- &protected_prefs_altered);
+ &protected_prefs_altered,
+ &used_legacy_pref_hashes);
+ UMA_HISTOGRAM_BOOLEAN("Settings.MigratedHashesFromLocalState",
+ used_legacy_pref_hashes);
+
+ if (!unprotected_prefs_altered && !protected_prefs_altered) {
+ // Clean up any MACs that might have been previously migrated from the
+ // various stores. It's safe to leave them behind for a little while as they
+ // will be ignored unless the corresponding value is _also_ present. The
+ // cleanup must be deferred until the MACs have been written to their target
+ // stores, and doing so in a subsequent launch is easier than within the
+ // same process.
+ CleanupMigratedHashes(unprotected_pref_names_,
+ protected_pref_hash_store_.get(),
+ protected_prefs_.get());
+ CleanupMigratedHashes(protected_pref_names_,
+ unprotected_pref_hash_store_.get(),
+ unprotected_prefs_.get());
+ legacy_pref_hash_store_->Reset();
+ }
// Hand the processed prefs back to their respective filters.
finalize_unprotected_filter_on_load_.Run(unprotected_prefs_.Pass(),
@@ -252,6 +344,9 @@ void SetupTrackedPreferencesMigration(
register_on_successful_unprotected_store_write_callback,
const base::Callback<void(const base::Closure&)>&
register_on_successful_protected_store_write_callback,
+ scoped_ptr<PrefHashStore> unprotected_pref_hash_store,
+ scoped_ptr<PrefHashStore> protected_pref_hash_store,
+ scoped_ptr<HashStoreContents> legacy_pref_hash_store,
InterceptablePrefFilter* unprotected_pref_filter,
InterceptablePrefFilter* protected_pref_filter) {
scoped_refptr<TrackedPreferencesMigrator> prefs_migrator(
@@ -262,6 +357,9 @@ void SetupTrackedPreferencesMigration(
protected_store_cleaner,
register_on_successful_unprotected_store_write_callback,
register_on_successful_protected_store_write_callback,
+ unprotected_pref_hash_store.Pass(),
+ protected_pref_hash_store.Pass(),
+ legacy_pref_hash_store.Pass(),
unprotected_pref_filter,
protected_pref_filter));
}

Powered by Google App Engine
This is Rietveld 408576698