[Chrome OS] Plumb new error codes from SignedSettings to consumers of the API

Finish taking new SignedSettings::ReturnCode values all the way out to consumers of the SignedSettingsHelper API

BUG=chromium-os:9666
TEST=unit tests, install on device, ensure that Guest mode setting can be updated and is honored.

Review URL: http://codereview.chromium.org/5671003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68866 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 119 insertions, 153 deletions

diff --git a/chrome/browser/chromeos/login/login_performer.cc b/chrome/browser/chromeos/login/login_performer.cc
index 10f0120..5dd85ff 100644
--- a/chrome/browser/chromeos/login/login_performer.cc
+++ b/chrome/browser/chromeos/login/login_performer.cc
@@ -156,9 +156,9 @@ void LoginPerformer::OnPasswordChangeDetected(
 ////////////////////////////////////////////////////////////////////////////////
 // LoginPerformer, SignedSettingsHelper::Callback implementation:
 
-void LoginPerformer::OnCheckWhiteListCompleted(bool success,
+void LoginPerformer::OnCheckWhitelistCompleted(SignedSettings::ReturnCode code,
                                                const std::string& email) {
-  if (success) {
+  if (code == SignedSettings::SUCCESS) {
     // Whitelist check passed, continue with authentication.
     StartAuthentication();
   } else {
diff --git a/chrome/browser/chromeos/login/login_performer.h b/chrome/browser/chromeos/login/login_performer.h
index c67a173..6824503 100644
--- a/chrome/browser/chromeos/login/login_performer.h
+++ b/chrome/browser/chromeos/login/login_performer.h
@@ -83,7 +83,7 @@ class LoginPerformer : public LoginStatusConsumer,
       const GaiaAuthConsumer::ClientLoginResult& credentials);
 
   // SignedSettingsHelper::Callback implementation:
-  virtual void OnCheckWhiteListCompleted(bool success,
+  virtual void OnCheckWhitelistCompleted(SignedSettings::ReturnCode code,
                                          const std::string& email);
 
   // NotificationObserver implementation:
diff --git a/chrome/browser/chromeos/login/signed_settings.cc b/chrome/browser/chromeos/login/signed_settings.cc
index 582a0db..8e400e4 100644
--- a/chrome/browser/chromeos/login/signed_settings.cc
+++ b/chrome/browser/chromeos/login/signed_settings.cc
@@ -24,7 +24,7 @@ SignedSettings::SignedSettings()
 
 SignedSettings::~SignedSettings() {}
 
-SignedSettings::FailureCode SignedSettings::MapKeyOpCode(
+SignedSettings::ReturnCode SignedSettings::MapKeyOpCode(
     OwnerManager::KeyOpCode return_code) {
   return (return_code == OwnerManager::KEY_UNAVAILABLE ?
           KEY_UNAVAILABLE : OPERATION_FAILED);
@@ -149,7 +149,7 @@ void CheckWhitelistOp::Execute() {
   CHECK(chromeos::CrosLibrary::Get()->EnsureLoaded());
   std::vector<uint8> sig;
   if (!CrosLibrary::Get()->GetLoginLibrary()->CheckWhitelist(email_, &sig)) {
-    d_->OnSettingsOpFailed(NOT_FOUND);
+    d_->OnSettingsOpCompleted(NOT_FOUND, false);
     return;
   }
   // Posts a task to the FILE thread to verify |sig|.
@@ -169,9 +169,9 @@ void CheckWhitelistOp::OnKeyOpComplete(
     return;
   }
   if (return_code == OwnerManager::SUCCESS) {
-    d_->OnSettingsOpSucceeded(true);
+    d_->OnSettingsOpCompleted(SUCCESS, true);
   } else {
-    d_->OnSettingsOpFailed(SignedSettings::MapKeyOpCode(return_code));
+    d_->OnSettingsOpCompleted(SignedSettings::MapKeyOpCode(return_code), false);
   }
 }
 
@@ -205,17 +205,17 @@ void WhitelistOp::OnKeyOpComplete(const OwnerManager::KeyOpCode return_code,
   if (return_code == OwnerManager::SUCCESS) {
     // OnComplete() will be called by this call, if it succeeds.
     if (!InitiateWhitelistOp(payload))
-      d_->OnSettingsOpFailed(OPERATION_FAILED);
+      d_->OnSettingsOpCompleted(OPERATION_FAILED, false);
   } else {
-    d_->OnSettingsOpFailed(SignedSettings::MapKeyOpCode(return_code));
+    d_->OnSettingsOpCompleted(SignedSettings::MapKeyOpCode(return_code), false);
   }
 }
 
 void WhitelistOp::OnComplete(bool value) {
   if (value)
-    d_->OnSettingsOpSucceeded(value);
+    d_->OnSettingsOpCompleted(SUCCESS, value);
   else
-    d_->OnSettingsOpFailed(NOT_FOUND);
+    d_->OnSettingsOpCompleted(NOT_FOUND, false);
 }
 
 bool WhitelistOp::InitiateWhitelistOp(const std::vector<uint8>& signature) {
@@ -241,7 +241,7 @@ void StorePropertyOp::Execute() {
         g_browser_process->local_state() &&
         SignedSettingsTempStorage::Store(name_, value_,
                                          g_browser_process->local_state())) {
-      d_->OnSettingsOpSucceeded(true);
+      d_->OnSettingsOpCompleted(SUCCESS, true);
       return;
     }
   }
@@ -263,6 +263,7 @@ void StorePropertyOp::OnKeyOpComplete(const OwnerManager::KeyOpCode return_code,
                           return_code, payload));
     return;
   }
+  VLOG(2) << "StorePropertyOp::OnKeyOpComplete return_code = " << return_code;
   // Now, sure we're on the UI thread.
   if (return_code == OwnerManager::SUCCESS) {
     // OnComplete() will be called by this call, if it succeeds.
@@ -270,18 +271,18 @@ void StorePropertyOp::OnKeyOpComplete(const OwnerManager::KeyOpCode return_code,
                                                                    value_,
                                                                    payload,
                                                                    this)) {
-      d_->OnSettingsOpFailed(OPERATION_FAILED);
+      d_->OnSettingsOpCompleted(OPERATION_FAILED, false);
     }
   } else {
-    d_->OnSettingsOpFailed(SignedSettings::MapKeyOpCode(return_code));
+    d_->OnSettingsOpCompleted(SignedSettings::MapKeyOpCode(return_code), false);
   }
 }
 
 void StorePropertyOp::OnComplete(bool value) {
   if (value)
-    d_->OnSettingsOpSucceeded(value);
+    d_->OnSettingsOpCompleted(SUCCESS, value);
   else
-    d_->OnSettingsOpFailed(NOT_FOUND);
+    d_->OnSettingsOpCompleted(NOT_FOUND, false);
 }
 
 RetrievePropertyOp::RetrievePropertyOp(const std::string& name,
@@ -316,7 +317,7 @@ void RetrievePropertyOp::Execute() {
   if (!CrosLibrary::Get()->GetLoginLibrary()->RetrieveProperty(name_,
                                                                &value_,
                                                                &sig)) {
-    d_->OnSettingsOpFailed(NOT_FOUND);
+    d_->OnSettingsOpCompleted(NOT_FOUND, std::string());
     return;
   }
   std::string to_verify = base::StringPrintf("%s=%s",
@@ -339,9 +340,10 @@ void RetrievePropertyOp::OnKeyOpComplete(
   }
   // Now, sure we're on the UI thread.
   if (return_code == OwnerManager::SUCCESS)
-    d_->OnSettingsOpSucceeded(value_);
+    d_->OnSettingsOpCompleted(SUCCESS, value_);
   else
-    d_->OnSettingsOpFailed(SignedSettings::MapKeyOpCode(return_code));
+    d_->OnSettingsOpCompleted(SignedSettings::MapKeyOpCode(return_code),
+                              std::string());
 }
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/login/signed_settings.h b/chrome/browser/chromeos/login/signed_settings.h
index 69f2bd0..61d5e15 100644
--- a/chrome/browser/chromeos/login/signed_settings.h
+++ b/chrome/browser/chromeos/login/signed_settings.h
@@ -35,7 +35,8 @@ class OwnershipService;
 class SignedSettings : public base::RefCountedThreadSafe<SignedSettings>,
                        public OwnerManager::Delegate {
  public:
-  enum FailureCode {
+  enum ReturnCode {
+    SUCCESS,
     NOT_FOUND,        // Email address or property name not found.
     KEY_UNAVAILABLE,  // Owner key not yet configured.
     OPERATION_FAILED  // Signature op or IPC to signed settings daemon failed.
@@ -44,9 +45,8 @@ class SignedSettings : public base::RefCountedThreadSafe<SignedSettings>,
   template <class T>
   class Delegate {
    public:
-    // These methods will be called on the UI thread.
-    virtual void OnSettingsOpSucceeded(T value) {}
-    virtual void OnSettingsOpFailed(FailureCode code) {}
+    // This method will be called on the UI thread.
+    virtual void OnSettingsOpCompleted(ReturnCode code, T value) {}
   };
 
   SignedSettings();
@@ -73,7 +73,7 @@ class SignedSettings : public base::RefCountedThreadSafe<SignedSettings>,
       const std::string& name,
       SignedSettings::Delegate<std::string>* d);
 
-  static FailureCode MapKeyOpCode(OwnerManager::KeyOpCode code);
+  static ReturnCode MapKeyOpCode(OwnerManager::KeyOpCode code);
 
   virtual void Execute() = 0;
 
diff --git a/chrome/browser/chromeos/login/signed_settings_helper.cc b/chrome/browser/chromeos/login/signed_settings_helper.cc
index b2075f7..ad86e83 100644
--- a/chrome/browser/chromeos/login/signed_settings_helper.cc
+++ b/chrome/browser/chromeos/login/signed_settings_helper.cc
@@ -90,11 +90,6 @@ class OpContext {
     delete this;
   }
 
-  // Callback on op failure.
-  virtual void OnOpFailed() {
-    OnOpCompleted();
-  }
-
   bool executing_;
   Delegate* delegate_;
 
@@ -121,31 +116,27 @@ class WhitelistOpContext : public SignedSettings::Delegate<bool>,
   }
 
   // chromeos::SignedSettings::Delegate implementation
-  virtual void OnSettingsOpSucceeded(bool value) {
+  virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code,
+                                     bool value) {
     if (callback_) {
       switch (type_) {
         case CHECK:
-          callback_->OnCheckWhiteListCompleted(value, email_);
+          callback_->OnCheckWhitelistCompleted(code, email_);
           break;
         case ADD:
-          callback_->OnWhitelistCompleted(value, email_);
+          callback_->OnWhitelistCompleted(code, email_);
           break;
         case REMOVE:
-          callback_->OnUnwhitelistCompleted(value, email_);
+          callback_->OnUnwhitelistCompleted(code, email_);
           break;
         default:
           LOG(ERROR) << "Unknown WhitelistOpContext type " << type_;
           break;
       }
     }
-
     OnOpCompleted();
   }
 
-  virtual void OnSettingsOpFailed(SignedSettings::FailureCode code) {
-    OnOpFailed();
-  }
-
  protected:
   // OpContext implemenetation
   virtual void CreateOp() {
@@ -165,27 +156,6 @@ class WhitelistOpContext : public SignedSettings::Delegate<bool>,
     }
   }
 
-  virtual void OnOpFailed() {
-    if (callback_) {
-      switch (type_) {
-        case CHECK:
-          callback_->OnCheckWhiteListCompleted(false, email_);
-          break;
-        case ADD:
-          callback_->OnWhitelistCompleted(false, email_);
-          break;
-        case REMOVE:
-          callback_->OnUnwhitelistCompleted(false, email_);
-          break;
-        default:
-          LOG(ERROR) << "Unknown WhitelistOpContext type " << type_;
-          break;
-      }
-    }
-
-    OnOpCompleted();
-  }
-
  private:
   Type type_;
   std::string email_;
@@ -206,28 +176,20 @@ class StorePropertyOpContext : public SignedSettings::Delegate<bool>,
   }
 
   // chromeos::SignedSettings::Delegate implementation
-  virtual void OnSettingsOpSucceeded(bool value) {
+  virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code,
+                                     bool unused) {
+    VLOG(2) << "OnSettingsOpCompleted, code = " << code;
     if (callback_)
-      callback_->OnStorePropertyCompleted(true, name_, value_);
+      callback_->OnStorePropertyCompleted(code, name_, value_);
     OnOpCompleted();
   }
 
-  virtual void OnSettingsOpFailed(SignedSettings::FailureCode code) {
-    OnOpFailed();
-  }
-
  protected:
   // OpContext implemenetation
   virtual void CreateOp() {
     op_ = SignedSettings::CreateStorePropertyOp(name_, value_, this);
   }
 
-  virtual void OnOpFailed() {
-    if (callback_)
-      callback_->OnStorePropertyCompleted(false, name_, value_);
-    OnOpCompleted();
-  }
-
  private:
   std::string name_;
   std::string value_;
@@ -247,29 +209,20 @@ class RetrievePropertyOpContext
   }
 
   // chromeos::SignedSettings::Delegate implementation
-  virtual void OnSettingsOpSucceeded(std::string value) {
+  virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code,
+                                     std::string value) {
     if (callback_)
-      callback_->OnRetrievePropertyCompleted(true, name_, value);
+      callback_->OnRetrievePropertyCompleted(code, name_, value);
 
     OnOpCompleted();
   }
 
-  virtual void OnSettingsOpFailed(SignedSettings::FailureCode code) {
-    OnOpFailed();
-  }
-
  protected:
   // OpContext implemenetation
   virtual void CreateOp() {
     op_ = SignedSettings::CreateRetrievePropertyOp(name_, this);
   }
 
-  virtual void OnOpFailed() {
-    if (callback_)
-      callback_->OnRetrievePropertyCompleted(false, name_, std::string());
-    OnOpCompleted();
-  }
-
  private:
   std::string name_;
 
diff --git a/chrome/browser/chromeos/login/signed_settings_helper.h b/chrome/browser/chromeos/login/signed_settings_helper.h
index cfe99e4..d65b677 100644
--- a/chrome/browser/chromeos/login/signed_settings_helper.h
+++ b/chrome/browser/chromeos/login/signed_settings_helper.h
@@ -6,7 +6,7 @@
 #define CHROME_BROWSER_CHROMEOS_LOGIN_SIGNED_SETTINGS_HELPER_H_
 #pragma once
 
-#include <string>
+#include "chrome/browser/chromeos/login/signed_settings.h"
 
 namespace chromeos {
 
@@ -20,24 +20,29 @@ class SignedSettingsHelper {
    public:
     // Callback of CheckWhitelistOp. |success| indicates whether the op succeeds
     // or not. |email| is the email that is checked against.
-    virtual void OnCheckWhiteListCompleted(
-        bool success, const std::string& email) {}
+    virtual void OnCheckWhitelistCompleted(
+        SignedSettings::ReturnCode code,
+        const std::string& email) {}
 
     // Callback of WhitelistOp that adds |email| to the whitelist.
     virtual void OnWhitelistCompleted(
-        bool success, const std::string& email) {}
+        SignedSettings::ReturnCode code, const std::string& email) {}
 
     // Callback of WhitelistOp that removes |email| to the whitelist.
     virtual void OnUnwhitelistCompleted(
-        bool success, const std::string& email) {}
+        SignedSettings::ReturnCode code, const std::string& email) {}
 
     // Callback of StorePropertyOp.
     virtual void OnStorePropertyCompleted(
-        bool success, const std::string& name, const std::string& value) {}
+        SignedSettings::ReturnCode code,
+        const std::string& name,
+        const std::string& value) {}
 
     // Callback of RetrievePropertyOp.
     virtual void OnRetrievePropertyCompleted(
-        bool success, const std::string& name, const std::string& value) {}
+        SignedSettings::ReturnCode code,
+        const std::string& name,
+        const std::string& value) {}
   };
 
   // Class factory
diff --git a/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc b/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc
index d877008..df8ad6c 100644
--- a/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc
+++ b/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc
@@ -19,16 +19,20 @@ namespace chromeos {
 
 class MockSignedSettingsHelperCallback : public SignedSettingsHelper::Callback {
  public:
-  MOCK_METHOD2(OnCheckWhiteListCompleted, void(
-      bool success, const std::string& email));
+  MOCK_METHOD2(OnCheckWhitelistCompleted, void(
+      SignedSettings::ReturnCode code, const std::string& email));
   MOCK_METHOD2(OnWhitelistCompleted, void(
-      bool success, const std::string& email));
+      SignedSettings::ReturnCode code, const std::string& email));
   MOCK_METHOD2(OnUnwhitelistCompleted, void(
-      bool success, const std::string& email));
+      SignedSettings::ReturnCode code, const std::string& email));
   MOCK_METHOD3(OnStorePropertyCompleted, void(
-      bool success, const std::string& name, const std::string& value));
+      SignedSettings::ReturnCode code,
+      const std::string& name,
+      const std::string& value));
   MOCK_METHOD3(OnRetrievePropertyCompleted, void(
-      bool success, const std::string& name, const std::string& value));
+      SignedSettings::ReturnCode code,
+      const std::string& name,
+      const std::string& value));
 };
 
 class SignedSettingsHelperTest : public ::testing::Test,
@@ -88,19 +92,19 @@ TEST_F(SignedSettingsHelperTest, SerializedOps) {
   EXPECT_CALL(m_, IsAlreadyOwned()).Times(2);
   InSequence s;
   EXPECT_CALL(m_, StartVerifyAttempt(_, _, _)).Times(1);
-  EXPECT_CALL(cb, OnCheckWhiteListCompleted(true, _))
+  EXPECT_CALL(cb, OnCheckWhitelistCompleted(SignedSettings::SUCCESS, _))
       .Times(1);
   EXPECT_CALL(m_, StartSigningAttempt(_, _)).Times(1);
-  EXPECT_CALL(cb, OnWhitelistCompleted(true, _))
+  EXPECT_CALL(cb, OnWhitelistCompleted(SignedSettings::SUCCESS, _))
       .Times(1);
   EXPECT_CALL(m_, StartSigningAttempt(_, _)).Times(1);
-  EXPECT_CALL(cb, OnUnwhitelistCompleted(true, _))
+  EXPECT_CALL(cb, OnUnwhitelistCompleted(SignedSettings::SUCCESS, _))
       .Times(1);
   EXPECT_CALL(m_, StartSigningAttempt(_, _)).Times(1);
-  EXPECT_CALL(cb, OnStorePropertyCompleted(true, _, _))
+  EXPECT_CALL(cb, OnStorePropertyCompleted(SignedSettings::SUCCESS, _, _))
       .Times(1);
   EXPECT_CALL(m_, StartVerifyAttempt(_, _, _)).Times(1);
-  EXPECT_CALL(cb, OnRetrievePropertyCompleted(true, _, _))
+  EXPECT_CALL(cb, OnRetrievePropertyCompleted(SignedSettings::SUCCESS, _, _))
       .Times(1);
 
   pending_ops_ = 5;
@@ -120,13 +124,13 @@ TEST_F(SignedSettingsHelperTest, CanceledOps) {
   EXPECT_CALL(m_, IsAlreadyOwned()).Times(2);
   InSequence s;
   EXPECT_CALL(m_, StartVerifyAttempt(_, _, _)).Times(1);
-  EXPECT_CALL(cb, OnCheckWhiteListCompleted(true, _))
+  EXPECT_CALL(cb, OnCheckWhitelistCompleted(SignedSettings::SUCCESS, _))
       .Times(1);
   EXPECT_CALL(m_, StartSigningAttempt(_, _)).Times(1);
-  EXPECT_CALL(cb, OnWhitelistCompleted(true, _))
+  EXPECT_CALL(cb, OnWhitelistCompleted(SignedSettings::SUCCESS, _))
       .Times(1);
   EXPECT_CALL(m_, StartSigningAttempt(_, _)).Times(1);
-  EXPECT_CALL(cb, OnUnwhitelistCompleted(true, _))
+  EXPECT_CALL(cb, OnUnwhitelistCompleted(SignedSettings::SUCCESS, _))
       .Times(1);
 
   // CheckWhitelistOp for cb_to_be_canceled still gets executed but callback
@@ -134,10 +138,10 @@ TEST_F(SignedSettingsHelperTest, CanceledOps) {
   EXPECT_CALL(m_, StartVerifyAttempt(_, _, _)).Times(1);
 
   EXPECT_CALL(m_, StartSigningAttempt(_, _)).Times(1);
-  EXPECT_CALL(cb, OnStorePropertyCompleted(true, _, _))
+  EXPECT_CALL(cb, OnStorePropertyCompleted(SignedSettings::SUCCESS, _, _))
       .Times(1);
   EXPECT_CALL(m_, StartVerifyAttempt(_, _, _)).Times(1);
-  EXPECT_CALL(cb, OnRetrievePropertyCompleted(true, _, _))
+  EXPECT_CALL(cb, OnRetrievePropertyCompleted(SignedSettings::SUCCESS, _, _))
       .Times(1);
 
   pending_ops_ = 6;
diff --git a/chrome/browser/chromeos/login/signed_settings_unittest.cc b/chrome/browser/chromeos/login/signed_settings_unittest.cc
index e36fb76..f3d4a13 100644
--- a/chrome/browser/chromeos/login/signed_settings_unittest.cc
+++ b/chrome/browser/chromeos/login/signed_settings_unittest.cc
@@ -33,26 +33,27 @@ class DummyDelegate : public SignedSettings::Delegate<T> {
  public:
   explicit DummyDelegate(T to_expect)
       : expect_success_(false),
-        expected_failure_(SignedSettings::NOT_FOUND),
+        expected_failure_(SignedSettings::SUCCESS),
         expected_(to_expect),
         run_(false) {}
   virtual ~DummyDelegate() { EXPECT_TRUE(run_); }
-  virtual void OnSettingsOpSucceeded(T value) {
+  virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code,
+                                     T value) {
     run_ = true;
-    EXPECT_TRUE(expect_success_);
-    EXPECT_EQ(expected_, value);
-  }
-  virtual void OnSettingsOpFailed(SignedSettings::FailureCode code) {
-    run_ = true;
-    EXPECT_FALSE(expect_success_);
+    if (expect_success_)
+      EXPECT_EQ(expected_, value);
     EXPECT_EQ(expected_failure_, code);
   }
-  virtual void expect_success() { expect_success_ = true; }
-  virtual void expect_failure(SignedSettings::FailureCode code) {
+  virtual void expect_success() {
+    expect_success_ = true;
+    expected_failure_ = SignedSettings::SUCCESS;
+  }
+  virtual void expect_failure(SignedSettings::ReturnCode code) {
+    expect_success_ = false;
     expected_failure_ = code;
   }
   bool expect_success_;
-  SignedSettings::FailureCode expected_failure_;
+  SignedSettings::ReturnCode expected_failure_;
   T expected_;
   bool run_;
 };
diff --git a/chrome/browser/chromeos/proxy_config_service_impl.cc b/chrome/browser/chromeos/proxy_config_service_impl.cc
index 2d272df..862459c 100644
--- a/chrome/browser/chromeos/proxy_config_service_impl.cc
+++ b/chrome/browser/chromeos/proxy_config_service_impl.cc
@@ -489,36 +489,34 @@ bool ProxyConfigServiceImpl::IOGetProxyConfig(net::ProxyConfig* net_config) {
   return false;
 }
 
-void ProxyConfigServiceImpl::OnSettingsOpSucceeded(bool value) {
-  VLOG(1) << "Stored proxy setting to device";
+void ProxyConfigServiceImpl::OnSettingsOpCompleted(
+    SignedSettings::ReturnCode code,
+    bool value) {
+  if (SignedSettings::SUCCESS == code)
+    VLOG(1) << "Stored proxy setting to device";
+  else
+    LOG(WARNING) << "Error storing proxy setting to device";
   store_property_op_ = NULL;
   if (persist_to_device_pending_)
     PersistConfigToDevice();
 }
 
-void ProxyConfigServiceImpl::OnSettingsOpSucceeded(std::string value) {
-  VLOG(1) << "Retrieved proxy setting from device, value=[" << value << "]";
-  if (reference_config_.Deserialize(value)) {
-    OnUISetProxyConfig(false);
+void ProxyConfigServiceImpl::OnSettingsOpCompleted(
+    SignedSettings::ReturnCode code,
+    std::string value) {
+  if (SignedSettings::SUCCESS == code) {
+    VLOG(1) << "Retrieved proxy setting from device, value=[" << value << "]";
+    if (reference_config_.Deserialize(value)) {
+      OnUISetProxyConfig(false);
+    } else {
+      LOG(WARNING) << "Error deserializing device's proxy setting";
+      InitConfigToDefault(true);
+    }
   } else {
-    LOG(WARNING) << "Error deserializing device's proxy setting";
-    InitConfigToDefault(true);
-  }
-  retrieve_property_op_ = NULL;
-}
-
-void ProxyConfigServiceImpl::OnSettingsOpFailed(
-    SignedSettings::FailureCode code) {
-  if (retrieve_property_op_) {
     LOG(WARNING) << "Error retrieving proxy setting from device";
     InitConfigToDefault(true);
-    retrieve_property_op_ = NULL;
-  } else {
-    LOG(WARNING) << "Error storing proxy setting to device";
-    store_property_op_ = NULL;
-    if (persist_to_device_pending_)
-      PersistConfigToDevice();
   }
+  retrieve_property_op_ = NULL;
 }
 
 //------------------ ProxyConfigServiceImpl: private methods -------------------
diff --git a/chrome/browser/chromeos/proxy_config_service_impl.h b/chrome/browser/chromeos/proxy_config_service_impl.h
index 8c42bd7..2e605a8 100644
--- a/chrome/browser/chromeos/proxy_config_service_impl.h
+++ b/chrome/browser/chromeos/proxy_config_service_impl.h
@@ -190,9 +190,10 @@ class ProxyConfigServiceImpl
   bool UISetProxyConfigBypassRules(const net::ProxyBypassRules& bypass_rules);
 
   // Implementation for SignedSettings::Delegate
-  virtual void OnSettingsOpSucceeded(bool value);
-  virtual void OnSettingsOpSucceeded(std::string value);
-  virtual void OnSettingsOpFailed(SignedSettings::FailureCode code);
+  virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code,
+                                     std::string value);
+  virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code,
+                                     bool value);
 
  private:
   friend class base::RefCountedThreadSafe<ProxyConfigServiceImpl>;
diff --git a/chrome/browser/chromeos/user_cros_settings_provider.cc b/chrome/browser/chromeos/user_cros_settings_provider.cc
index 9aee01b..4c15e01 100644
--- a/chrome/browser/chromeos/user_cros_settings_provider.cc
+++ b/chrome/browser/chromeos/user_cros_settings_provider.cc
@@ -305,7 +305,7 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback,
   }
 
   // Implementation of SignedSettingsHelper::Callback.
-  virtual void OnRetrievePropertyCompleted(bool success,
+  virtual void OnRetrievePropertyCompleted(SignedSettings::ReturnCode code,
                                            const std::string& name,
                                            const std::string& value) {
     if (!IsControlledBooleanSetting(name) && !IsControlledStringSetting(name)) {
@@ -314,6 +314,7 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback,
     }
     DCHECK(GetOwnershipStatus() != OWNERSHIP_UNKNOWN);
 
+    bool success = code == SignedSettings::SUCCESS;
     PrefService* prefs = g_browser_process->local_state();
     if (!success && GetOwnershipStatus() == OWNERSHIP_TAKEN) {
       LOG(ERROR) << "On owned device: failed to retrieve cros "
@@ -366,32 +367,34 @@ class UserCrosSettingsTrust : public SignedSettingsHelper::Callback,
   }
 
   // Implementation of SignedSettingsHelper::Callback.
-  virtual void OnStorePropertyCompleted(bool success,
+  virtual void OnStorePropertyCompleted(SignedSettings::ReturnCode code,
                                         const std::string& name,
                                         const std::string& value) {
-    VLOG(1) << "Store cros setting " << name << "=" << value << ", success="
-            << success;
+    VLOG(1) << "Store cros setting " << name << "=" << value << ", code="
+            << code;
 
     // Reload the setting if store op fails.
-    if (!success)
+    if (code != SignedSettings::SUCCESS)
       SignedSettingsHelper::Get()->StartRetrieveProperty(name, this);
   }
 
   // Implementation of SignedSettingsHelper::Callback.
-  virtual void OnWhitelistCompleted(bool success, const std::string& email) {
-    VLOG(1) << "Add " << email << " to whitelist, success=" << success;
+  virtual void OnWhitelistCompleted(SignedSettings::ReturnCode code,
+                                    const std::string& email) {
+    VLOG(1) << "Add " << email << " to whitelist, code=" << code;
 
     // Reload the whitelist on settings op failure.
-    if (!success)
+    if (code != SignedSettings::SUCCESS)
       CrosSettings::Get()->FireObservers(kAccountsPrefUsers);
   }
 
   // Implementation of SignedSettingsHelper::Callback.
-  virtual void OnUnwhitelistCompleted(bool success, const std::string& email) {
-    VLOG(1) << "Remove " << email << " from whitelist, success=" << success;
+  virtual void OnUnwhitelistCompleted(SignedSettings::ReturnCode code,
+                                      const std::string& email) {
+    VLOG(1) << "Remove " << email << " from whitelist, code=" << code;
 
     // Reload the whitelist on settings op failure.
-    if (!success)
+    if (code != SignedSettings::SUCCESS)
       CrosSettings::Get()->FireObservers(kAccountsPrefUsers);
   }
 
@@ -559,7 +562,6 @@ bool UserCrosSettingsProvider::HandlesSetting(const std::string& path) {
 void UserCrosSettingsProvider::WhitelistUser(const std::string& email) {
   SignedSettingsHelper::Get()->StartWhitelistOp(
       email, true, UserCrosSettingsTrust::GetSharedInstance());
-
   PrefService* prefs = g_browser_process->local_state();
   ListValue* cached_whitelist = prefs->GetMutableList(kAccountsPrefUsers);
   cached_whitelist->Append(Value::CreateStringValue(email));