Clear a preference when sync tries to delete a preference

chrome/browser/prefs/pref_model_associator.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/prefs/pref_model_associator.h"
 
 #include "base/auto_reset.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/location.h"
@@ skipping 460 matching lines @@
   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.";
+    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
     // kConfirmToQuitEnabled. Ignore updates from these preferences.
     const char* pref_name = name.c_str();
     std::string new_name;
     // We migrated this preference name, so do as if the name had not changed.
     if (IsOldMigratedPreference(pref_name)) {
       new_name = GetNewMigratedPreferenceName(pref_name);
       pref_name = new_name.c_str();
     }
 
     if (!IsPrefRegistered(pref_name))
       continue;
 
     const PrefService::Preference* pref =
         pref_service_->FindPreference(pref_name);
     DCHECK(pref);
 
     // 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*/);
 
     // Keep track of any newly synced preferences.
     if (iter->change_type() == syncer::SyncChange::ACTION_ADD) {
       synced_preferences_.insert(name);
     }
   }
   return syncer::SyncError();
 }
@@ skipping 128 matching lines @@
 void PrefModelAssociator::NotifySyncedPrefObservers(const std::string& path,
                                                     bool from_sync) const {
   SyncedPrefObserverMap::const_iterator observer_iter =
       synced_pref_observers_.find(path);
   if (observer_iter == synced_pref_observers_.end())
     return;
   SyncedPrefObserverList* observers = observer_iter->second;
   FOR_EACH_OBSERVER(SyncedPrefObserver, *observers,
                     OnSyncedPrefChanged(path, from_sync));
 }