Clear a preference when sync tries to delete a preference
Preference can be deleted during sync rollback if the preference was not set before
first signin. Clear the preference if that happens.
BUG=362679
Review URL: https://codereview.chromium.org/444763005
Cr-Commit-Position: refs/heads/master@{#289048}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289048 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/prefs/pref_model_associator.cc b/chrome/browser/prefs/pref_model_associator.cc
index bcf87ea..5ddbf1f 100644
--- a/chrome/browser/prefs/pref_model_associator.cc
+++ b/chrome/browser/prefs/pref_model_associator.cc
@@ -471,26 +471,10 @@
for (iter = change_list.begin(); iter != change_list.end(); ++iter) {
DCHECK_EQ(type_, iter->sync_data().GetDataType());
- std::string name;
const sync_pb::PreferenceSpecifics& pref_specifics =
GetSpecifics(iter->sync_data());
- scoped_ptr<base::Value> value(ReadPreferenceSpecifics(pref_specifics,
- &name));
-
- if (iter->change_type() == syncer::SyncChange::ACTION_DELETE) {
- // We never delete preferences.
- NOTREACHED() << "Attempted to process sync delete change for " << name
- << ". Skipping.";
- continue;
- }
-
- // Skip values we can't deserialize.
- // TODO(zea): consider taking some further action such as erasing the bad
- // data.
- if (!value.get())
- continue;
-
+ std::string name = pref_specifics.name();
// It is possible that we may receive a change to a preference we do not
// want to sync. For example if the user is syncing a Mac client and a
// Windows client, the Windows client does not support
@@ -506,9 +490,18 @@
if (!IsPrefRegistered(pref_name))
continue;
- const PrefService::Preference* pref =
- pref_service_->FindPreference(pref_name);
- DCHECK(pref);
+ if (iter->change_type() == syncer::SyncChange::ACTION_DELETE) {
+ pref_service_->ClearPref(pref_name);
+ continue;
+ }
+
+ scoped_ptr<base::Value> value(ReadPreferenceSpecifics(pref_specifics));
+ if (!value.get()) {
+ // Skip values we can't deserialize.
+ // TODO(zea): consider taking some further action such as erasing the bad
+ // data.
+ continue;
+ }
// This will only modify the user controlled value store, which takes
// priority over the default value but is ignored if the preference is
@@ -526,8 +519,7 @@
}
base::Value* PrefModelAssociator::ReadPreferenceSpecifics(
- const sync_pb::PreferenceSpecifics& preference,
- std::string* name) {
+ const sync_pb::PreferenceSpecifics& preference) {
base::JSONReader reader;
scoped_ptr<base::Value> value(reader.ReadToValue(preference.value()));
if (!value.get()) {
@@ -536,7 +528,6 @@
LOG(ERROR) << err;
return NULL;
}
- *name = preference.name();
return value.release();
}
diff --git a/chrome/browser/prefs/pref_model_associator.h b/chrome/browser/prefs/pref_model_associator.h
index 3c514e67..049b5c3 100644
--- a/chrome/browser/prefs/pref_model_associator.h
+++ b/chrome/browser/prefs/pref_model_associator.h
@@ -92,10 +92,9 @@
const base::Value& value,
syncer::SyncData* sync_data) const;
- // Extract preference value and name from sync specifics.
+ // Extract preference value from sync specifics.
base::Value* ReadPreferenceSpecifics(
- const sync_pb::PreferenceSpecifics& specifics,
- std::string* name);
+ const sync_pb::PreferenceSpecifics& specifics);
// Returns true if the pref under the given name is pulled down from sync.
// Note this does not refer to SYNCABLE_PREF.
diff --git a/chrome/browser/prefs/prefs_syncable_service_unittest.cc b/chrome/browser/prefs/prefs_syncable_service_unittest.cc
index e9650ee..e5d06a7 100644
--- a/chrome/browser/prefs/prefs_syncable_service_unittest.cc
+++ b/chrome/browser/prefs/prefs_syncable_service_unittest.cc
@@ -693,7 +693,7 @@
// Switch kHomePage to managed and set a different value.
base::StringValue managed_value("http://example.com/managed");
GetTestingPrefService()->SetManagedPref(prefs::kHomePage,
- managed_value.DeepCopy());
+ managed_value.DeepCopy());
// The pref value should be the one dictated by policy.
EXPECT_TRUE(managed_value.Equals(&GetPreferenceValue(prefs::kHomePage)));
EXPECT_FALSE(pref->IsDefaultValue());
@@ -706,3 +706,19 @@
// There should still be no synced value.
EXPECT_FALSE(FindValue(prefs::kHomePage, out).get());
}
+
+TEST_F(PrefsSyncableServiceTest, DeletePreference) {
+ prefs_.SetString(prefs::kHomePage, kExampleUrl0);
+ const PrefService::Preference* pref =
+ prefs_.FindPreference(prefs::kHomePage);
+ EXPECT_FALSE(pref->IsDefaultValue());
+
+ InitWithNoSyncData();
+
+ scoped_ptr<base::Value> null_value(base::Value::CreateNullValue());
+ syncer::SyncChangeList list;
+ list.push_back(MakeRemoteChange(
+ 1, prefs::kHomePage, *null_value, SyncChange::ACTION_DELETE));
+ pref_sync_service_->ProcessSyncChanges(FROM_HERE, list);
+ EXPECT_TRUE(pref->IsDefaultValue());
+}
