Propagate more information about ValueStore errors to callers, notably an

chrome/browser/extensions/api/storage/syncable_settings_storage.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/extensions/api/storage/syncable_settings_storage.h"
 
+#include "base/strings/stringprintf.h"
 #include "chrome/browser/extensions/api/storage/settings_namespace.h"
 #include "chrome/browser/extensions/api/storage/settings_sync_processor.h"
 #include "chrome/browser/extensions/api/storage/settings_sync_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "sync/api/sync_data.h"
 #include "sync/protocol/extension_setting_specifics.pb.h"
 
 namespace extensions {
 
 using content::BrowserThread;
@@ skipping 118 matching lines @@
   DCHECK(!sync_processor_.get());
 
   sync_processor_ = sync_processor.Pass();
   sync_processor_->Init(sync_state);
 
   ReadResult maybe_settings = delegate_->Get();
   if (maybe_settings->HasError()) {
     return syncer::SyncError(
         FROM_HERE,
         syncer::SyncError::DATATYPE_ERROR,
-        std::string("Failed to get settings: ") + maybe_settings->error(),
+        base::StringPrintf("Failed to get settings: %s",
+            maybe_settings->error().message.c_str()),
         sync_processor_->type());
   }
 
-  const base::DictionaryValue& settings = *maybe_settings->settings().get();
-  if (sync_state.empty())
-    return SendLocalSettingsToSync(settings);
-  else
-    return OverwriteLocalSettingsWithSync(sync_state, settings);
+  const base::DictionaryValue& settings = maybe_settings->settings();
+  return sync_state.empty() ?
+      SendLocalSettingsToSync(settings) :
+      OverwriteLocalSettingsWithSync(sync_state, settings);
 }
 
 syncer::SyncError SyncableSettingsStorage::SendLocalSettingsToSync(
     const base::DictionaryValue& settings) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   ValueStoreChangeList changes;
   for (base::DictionaryValue::Iterator i(settings); !i.IsAtEnd(); i.Advance()) {
     changes.push_back(ValueStoreChange(i.key(), NULL, i.value().DeepCopy()));
   }
@@ skipping 90 matching lines @@
     const std::string& key = it->key();
     const Value& value = it->value();
 
     scoped_ptr<Value> current_value;
     {
       ReadResult maybe_settings = Get(it->key());
       if (maybe_settings->HasError()) {
         errors.push_back(syncer::SyncError(
             FROM_HERE,
             syncer::SyncError::DATATYPE_ERROR,
-            std::string("Error getting current sync state for ") +
-                extension_id_ + "/" + key + ": " + maybe_settings->error(),
+            base::StringPrintf("Error getting current sync state for %s/%s: %s",
+                extension_id_.c_str(), key.c_str(),
+                maybe_settings->error().message.c_str()),
             sync_processor_->type()));
         continue;
       }
-      Value* value = NULL;
-      if (maybe_settings->settings()->GetWithoutPathExpansion(key, &value)) {
-        current_value.reset(value->DeepCopy());
-      }
+      maybe_settings->settings().RemoveWithoutPathExpansion(key,
+                                                            &current_value);
     }
 
     syncer::SyncError error;
 
     switch (it->change_type()) {
       case syncer::SyncChange::ACTION_ADD:
         if (!current_value.get()) {
           error = OnSyncAdd(key, value.DeepCopy(), &changes);
         } else {
           // Already a value; hopefully a local change has beaten sync in a
@@ skipping 51 matching lines @@
 syncer::SyncError SyncableSettingsStorage::OnSyncAdd(
     const std::string& key,
     Value* new_value,
     ValueStoreChangeList* changes) {
   DCHECK(new_value);
   WriteResult result = delegate_->Set(IGNORE_QUOTA, key, *new_value);
   if (result->HasError()) {
     return syncer::SyncError(
         FROM_HERE,
         syncer::SyncError::DATATYPE_ERROR,
-        std::string("Error pushing sync add to local settings: ") +
-            result->error(),
+        base::StringPrintf("Error pushing sync add to local settings: %s",
+            result->error().message.c_str()),
         sync_processor_->type());
   }
   changes->push_back(ValueStoreChange(key, NULL, new_value));
   return syncer::SyncError();
 }
 
 syncer::SyncError SyncableSettingsStorage::OnSyncUpdate(
     const std::string& key,
     Value* old_value,
     Value* new_value,
     ValueStoreChangeList* changes) {
   DCHECK(old_value);
   DCHECK(new_value);
   WriteResult result = delegate_->Set(IGNORE_QUOTA, key, *new_value);
   if (result->HasError()) {
     return syncer::SyncError(
         FROM_HERE,
         syncer::SyncError::DATATYPE_ERROR,
-        std::string("Error pushing sync update to local settings: ") +
-            result->error(),
+        base::StringPrintf("Error pushing sync update to local settings: %s",
+            result->error().message.c_str()),
         sync_processor_->type());
   }
   changes->push_back(ValueStoreChange(key, old_value, new_value));
   return syncer::SyncError();
 }
 
 syncer::SyncError SyncableSettingsStorage::OnSyncDelete(
     const std::string& key,
     Value* old_value,
     ValueStoreChangeList* changes) {
   DCHECK(old_value);
   WriteResult result = delegate_->Remove(key);
   if (result->HasError()) {
     return syncer::SyncError(
         FROM_HERE,
         syncer::SyncError::DATATYPE_ERROR,
-        std::string("Error pushing sync remove to local settings: ") +
-            result->error(),
+        base::StringPrintf("Error pushing sync remove to local settings: %s",
+            result->error().message.c_str()),
         sync_processor_->type());
   }
   changes->push_back(ValueStoreChange(key, old_value, NULL));
   return syncer::SyncError();
 }
 
 }  // namespace extensions