Extension Settings API: make it so that when changes received from Sync fail to

chrome/browser/extensions/settings/syncable_settings_storage.cc

 // Copyright (c) 2011 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/extensions/settings/syncable_settings_storage.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/extensions/settings/settings_sync_util.h"
 #include "chrome/browser/sync/api/sync_data.h"
 #include "chrome/browser/sync/protocol/extension_setting_specifics.pb.h"
@@ skipping 128 matching lines @@
 
   if (sync_state.empty()) {
     return SendLocalSettingsToSync(maybe_settings.settings());
   }
   return OverwriteLocalSettingsWithSync(sync_state, maybe_settings.settings());
 }
 
 SyncError SyncableSettingsStorage::SendLocalSettingsToSync(
     const DictionaryValue& settings) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK(sync_processor_);
+
   SyncChangeList changes;
   for (DictionaryValue::key_iterator it = settings.begin_keys();
       it != settings.end_keys(); ++it) {
     Value* value;
     settings.GetWithoutPathExpansion(*it, &value);
     changes.push_back(
         settings_sync_util::CreateAdd(extension_id_, *it, *value));
   }
 
   if (changes.empty()) {
     return SyncError();
   }
 
   SyncError error = sync_processor_->ProcessSyncChanges(FROM_HERE, changes);
   if (error.IsSet()) {
+    StopSyncing();
     return error;
   }
 
   for (DictionaryValue::key_iterator it = settings.begin_keys();
       it != settings.end_keys(); ++it) {
     synced_keys_.insert(*it);
   }
   return SyncError();
 }
 
@@ skipping 41 matching lines @@
     Value* value = NULL;
     CHECK(new_sync_state->RemoveWithoutPathExpansion(key, &value));
     changes.push_back(
         SettingSyncData(
             SyncChange::ACTION_ADD, extension_id_, key, value));
   }
 
   if (changes.empty()) {
     return SyncError();
   }
-
-  std::vector<SyncError> sync_errors(ProcessSyncChanges(changes));
-  if (sync_errors.empty()) {
-    return SyncError();
-  }
-  // TODO(kalman): something sensible with multiple errors.
-  return sync_errors[0];
+  return ProcessSyncChanges(changes);
 }
 
 void SyncableSettingsStorage::StopSyncing() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  DCHECK(sync_type_ == syncable::EXTENSION_SETTINGS ||
-         sync_type_ == syncable::APP_SETTINGS);
-  DCHECK(sync_processor_);
 
+  // Stop syncing is allowed to be called multiple times without StartSyncing,
+  // so don't DCHECK that these values aren't already disabled.
   sync_type_ = syncable::UNSPECIFIED;
   sync_processor_ = NULL;
   synced_keys_.clear();
 }
 
-std::vector<SyncError> SyncableSettingsStorage::ProcessSyncChanges(
+SyncError SyncableSettingsStorage::ProcessSyncChanges(
     const SettingSyncDataList& sync_changes) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  DCHECK(sync_processor_);
   DCHECK(!sync_changes.empty()) << "No sync changes for " << extension_id_;
 
+  if (!sync_processor_) {
+    return SyncError(
+        FROM_HERE,
+        std::string("Sync is inactive for ") + extension_id_,
+        sync_type_);
+  }
+
   std::vector<SyncError> errors;
   SettingChangeList changes;
 
   for (SettingSyncDataList::const_iterator it = sync_changes.begin();
       it != sync_changes.end(); ++it) {
     DCHECK_EQ(extension_id_, it->extension_id());
 
     const std::string& key = it->key();
     const Value& value = it->value();
 
@@ skipping 59 matching lines @@
     if (error.IsSet()) {
       errors.push_back(error);
     }
   }
 
   observers_->Notify(
       &SettingsObserver::OnSettingsChanged,
       extension_id_,
       SettingChange::GetEventJson(changes));
 
-  return errors;
+  // TODO(kalman): Something sensible with multiple errors.
+  return errors.empty() ? SyncError() : errors[0];
 }
 
 void SyncableSettingsStorage::SendChangesToSync(
     const SettingChangeList& changes) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DCHECK(sync_processor_);
   DCHECK(!changes.empty());
 
   SyncChangeList sync_changes;
   std::set<std::string> added_keys;
@@ skipping 28 matching lines @@
     }
   }
 
   if (sync_changes.empty()) {
     return;
   }
 
   SyncError error =
       sync_processor_->ProcessSyncChanges(FROM_HERE, sync_changes);
   if (error.IsSet()) {
-    LOG(WARNING) << "Failed to send changes to sync: " << error.message();
+    StopSyncing();
     return;
   }
 
   synced_keys_.insert(added_keys.begin(), added_keys.end());
   for (std::set<std::string>::iterator it = deleted_keys.begin();
       it != deleted_keys.end(); ++it) {
     synced_keys_.erase(*it);
   }
 }
 
@@ skipping 46 matching lines @@
         FROM_HERE,
         std::string("Error pushing sync remove to local settings: ") +
             result.error(),
         sync_type_);
   }
   changes->push_back(SettingChange(key, old_value, NULL));
   return SyncError();
 }
 
 }  // namespace extensions