Clear a preference when sync tries to delete a preference

chrome/browser/prefs/pref_model_associator.cc

Index: chrome/browser/prefs/pref_model_associator.cc
diff --git a/chrome/browser/prefs/pref_model_associator.cc b/chrome/browser/prefs/pref_model_associator.cc
index bcf87eae3940054d6e437bfa7e327325cb99a16c..5057e361ed465d8660450d31f7f635ddea8b5272 100644
--- a/chrome/browser/prefs/pref_model_associator.cc
+++ b/chrome/browser/prefs/pref_model_associator.cc
@@ -478,19 +478,14 @@ syncer::SyncError PrefModelAssociator::ProcessSyncChanges(
     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.";
+    if (iter->change_type() != syncer::SyncChange::ACTION_DELETE &&

[Nicolas Zea, 2014/08/08 17:38:32]

does the ACTION_DELETE condition still need to be there?

+        !value.get()) {
+      // Skip values we can't deserialize.
+      // TODO(zea): consider taking some further action such as erasing the bad
+      // data.
       continue;
     }
 
-    // Skip values we can't deserialize.
-    // TODO(zea): consider taking some further action such as erasing the bad
-    // data.
-    if (!value.get())
-      continue;
-
     // 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
@@ -513,7 +508,12 @@ syncer::SyncError PrefModelAssociator::ProcessSyncChanges(
     // This will only modify the user controlled value store, which takes
     // priority over the default value but is ignored if the preference is
     // policy controlled.
-    pref_service_->Set(pref_name, *value);
+    if (iter->change_type() != syncer::SyncChange::ACTION_DELETE) {
+      pref_service_->Set(pref_name, *value);
+    } else {
+      pref_service_->Set(pref_name,
+                         *pref_service_->GetDefaultPrefValue(pref_name));

[gab, 2014/08/08 12:55:43]

You should just do pref_service_->ClearPref(pref_name); this will make it such
that there is no user-selected value and the default will thus kick in; as-is
otherwise this will explicitly make the default value the user-selected value
(i.e. subtle difference which matters when the default itself is changed in the
future).

[Nicolas Zea, 2014/08/08 17:38:32]

Agreed. This will actually sync the default value around since it's being stored
in the user store, even though different platforms sometimes have different
default values.

+    }
 
     NotifySyncedPrefObservers(name, true /*from_sync*/);