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

chrome/browser/prefs/pref_model_associator.cc

@@ -471,26 +471,10 @@ syncer::SyncError PrefModelAssociator::ProcessSyncChanges(
   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 @@ syncer::SyncError PrefModelAssociator::ProcessSyncChanges(
     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 @@ syncer::SyncError PrefModelAssociator::ProcessSyncChanges(
 }
 
 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 @@ base::Value* PrefModelAssociator::ReadPreferenceSpecifics(
     LOG(ERROR) << err;
     return NULL;
   }
-  *name = preference.name();
   return value.release();
 }
 

chrome/browser/prefs/pref_model_associator.h

@@ -92,10 +92,9 @@ class PrefModelAssociator
                           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.

chrome/browser/prefs/prefs_syncable_service_unittest.cc

@@ -693,7 +693,7 @@ TEST_F(PrefsSyncableServiceTest, DynamicManagedDefaultPreferences) {
   // 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 @@ TEST_F(PrefsSyncableServiceTest, DynamicManagedDefaultPreferences) {
   // 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());
+}