Rework chromeos::FlimflamDevice/ServiceClient with CallMethodWithErrorCallback

Modify FlimflamDeviceClient's PIN operation methods and Register to use CallMethodWithErrorCallback
Modify FlimflamServiceClient::Connect to use CallMethodWithErrorCallback

BUG=chromium-os:16557
TEST=chromeos_tests

Review URL: https://chromiumcodereview.appspot.com/10170020

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

12 files changed, 220 insertions, 49 deletions

diff --git a/chromeos/dbus/flimflam_client_helper.cc b/chromeos/dbus/flimflam_client_helper.cc
index 97c91dd..89162cb 100644
--- a/chromeos/dbus/flimflam_client_helper.cc
+++ b/chromeos/dbus/flimflam_client_helper.cc
@@ -70,6 +70,20 @@ void FlimflamClientHelper::CallDictionaryValueMethod(
                                 callback));
 }
 
+void FlimflamClientHelper::CallVoidMethodWithErrorCallback(
+    dbus::MethodCall* method_call,
+    const base::Closure& callback,
+    const ErrorCallback& error_callback) {
+  proxy_->CallMethodWithErrorCallback(
+      method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+      base::Bind(&FlimflamClientHelper::OnVoidMethodWithErrorCallback,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 callback),
+      base::Bind(&FlimflamClientHelper::OnError,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 error_callback));
+}
+
 bool FlimflamClientHelper::CallVoidMethodAndBlock(
     dbus::MethodCall* method_call) {
   scoped_ptr<dbus::Response> response(
@@ -218,4 +232,23 @@ void FlimflamClientHelper::OnDictionaryValueMethod(
   callback.Run(DBUS_METHOD_CALL_SUCCESS, *result);
 }
 
+void FlimflamClientHelper::OnVoidMethodWithErrorCallback(
+    const base::Closure& callback,
+    dbus::Response* response) {
+  callback.Run();
+}
+
+void FlimflamClientHelper::OnError(const ErrorCallback& error_callback,
+                                   dbus::ErrorResponse* response) {
+  std::string error_name;
+  std::string error_message;
+  if (response) {
+    // Error message may contain the error message as string.
+    dbus::MessageReader reader(response);
+    error_name = response->GetErrorName();
+    reader.PopString(&error_message);
+  }
+  error_callback.Run(error_name, error_message);
+}
+
 }  // namespace chromeos
diff --git a/chromeos/dbus/flimflam_client_helper.h b/chromeos/dbus/flimflam_client_helper.h
index dd2a261..859d95e 100644
--- a/chromeos/dbus/flimflam_client_helper.h
+++ b/chromeos/dbus/flimflam_client_helper.h
@@ -23,6 +23,7 @@ class DictionaryValue;
 namespace dbus {
 
 class Bus;
+class ErrorResponse;
 class MessageWriter;
 class MethodCall;
 class ObjectPath;
@@ -54,6 +55,10 @@ class FlimflamClientHelper {
       DBusMethodCallStatus call_status,
       const base::DictionaryValue& result)> DictionaryValueCallback;
 
+  // A callback to handle erros for method call.
+  typedef base::Callback<void(const std::string& error_name,
+                              const std::string& error_message)> ErrorCallback;
+
   FlimflamClientHelper(dbus::Bus* bus, dbus::ObjectProxy* proxy);
 
   virtual ~FlimflamClientHelper();
@@ -79,6 +84,11 @@ class FlimflamClientHelper {
   void CallDictionaryValueMethod(dbus::MethodCall* method_call,
                                  const DictionaryValueCallback& callback);
 
+  // Calls a method without results with error callback.
+  void CallVoidMethodWithErrorCallback(dbus::MethodCall* method_call,
+                                       const base::Closure& callback,
+                                       const ErrorCallback& error_callback);
+
   // DEPRECATED DO NOT USE: Calls a method without results.
   bool CallVoidMethodAndBlock(dbus::MethodCall* method_call);
 
@@ -116,6 +126,15 @@ class FlimflamClientHelper {
   void OnDictionaryValueMethod(const DictionaryValueCallback& callback,
                                dbus::Response* response);
 
+  // Handles responses for methods without results.
+  // Used by CallVoidMethodWithErrorCallback().
+  void OnVoidMethodWithErrorCallback(const base::Closure& callback,
+                                     dbus::Response* response);
+
+  // Handles errors for method calls.
+  void OnError(const ErrorCallback& error_callback,
+               dbus::ErrorResponse* response);
+
   base::WeakPtrFactory<FlimflamClientHelper> weak_ptr_factory_;
   // TODO(hashimoto): Remove this when we no longer need to make blocking calls.
   BlockingMethodCaller blocking_method_caller_;
diff --git a/chromeos/dbus/flimflam_client_unittest_base.cc b/chromeos/dbus/flimflam_client_unittest_base.cc
index dc4473b..67046f9 100644
--- a/chromeos/dbus/flimflam_client_unittest_base.cc
+++ b/chromeos/dbus/flimflam_client_unittest_base.cc
@@ -30,11 +30,31 @@ void RunTask(const tracked_objects::Location& from_here,
 
 }  // namespace
 
+FlimflamClientUnittestBase::MockClosure::MockClosure() {}
+
+FlimflamClientUnittestBase::MockClosure::~MockClosure() {}
+
+base::Closure FlimflamClientUnittestBase::MockClosure::GetCallback() {
+  return base::Bind(&MockClosure::Run, base::Unretained(this));
+}
+
+
+FlimflamClientUnittestBase::MockErrorCallback::MockErrorCallback() {}
+
+FlimflamClientUnittestBase::MockErrorCallback::~MockErrorCallback() {}
+
+FlimflamClientHelper::ErrorCallback
+FlimflamClientUnittestBase::MockErrorCallback::GetCallback() {
+  return base::Bind(&MockErrorCallback::Run, base::Unretained(this));
+}
+
+
 FlimflamClientUnittestBase::FlimflamClientUnittestBase(
     const std::string& interface_name,
     const dbus::ObjectPath& object_path)
     : interface_name_(interface_name),
-      object_path_(object_path) {
+      object_path_(object_path),
+      response_(NULL) {
 }
 
 FlimflamClientUnittestBase::~FlimflamClientUnittestBase() {
@@ -63,6 +83,12 @@ void FlimflamClientUnittestBase::SetUp() {
   EXPECT_CALL(*mock_proxy_, CallMethod(_, _, _))
       .WillRepeatedly(Invoke(this, &FlimflamClientUnittestBase::OnCallMethod));
 
+  // Set an expectation so mock_proxy's CallMethodWithErrorCallback() will use
+  // OnCallMethodWithErrorCallback() to return responses.
+  EXPECT_CALL(*mock_proxy_, CallMethodWithErrorCallback(_, _, _, _))
+      .WillRepeatedly(Invoke(
+          this, &FlimflamClientUnittestBase::OnCallMethodWithErrorCallback));
+
   // Set an expectation so mock_proxy's ConnectToSignal() will use
   // OnConnectToSignal() to run the callback.
   EXPECT_CALL(*mock_proxy_, ConnectToSignal(
@@ -208,6 +234,14 @@ void FlimflamClientUnittestBase::OnCallMethod(
                          base::Bind(response_callback, response_));
 }
 
+void FlimflamClientUnittestBase::OnCallMethodWithErrorCallback(
+    dbus::MethodCall* method_call,
+    int timeout_ms,
+    const dbus::ObjectProxy::ResponseCallback& response_callback,
+    const dbus::ObjectProxy::ErrorCallback& error_callback) {
+  OnCallMethod(method_call, timeout_ms, response_callback);
+}
+
 dbus::Response* FlimflamClientUnittestBase::OnCallMethodAndBlock(
     dbus::MethodCall* method_call,
     int timeout_ms) {
diff --git a/chromeos/dbus/flimflam_client_unittest_base.h b/chromeos/dbus/flimflam_client_unittest_base.h
index 82b9e5e..68ac59c 100644
--- a/chromeos/dbus/flimflam_client_unittest_base.h
+++ b/chromeos/dbus/flimflam_client_unittest_base.h
@@ -11,6 +11,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "chromeos/dbus/dbus_method_call_status.h"
+#include "chromeos/dbus/flimflam_client_helper.h"
 #include "dbus/mock_bus.h"
 #include "dbus/mock_object_proxy.h"
 #include "dbus/object_proxy.h"
@@ -34,6 +35,25 @@ namespace chromeos {
 // A class to provide functionalities needed for testing Flimflam D-Bus clients.
 class FlimflamClientUnittestBase : public testing::Test {
  public:
+  // A mock Closure.
+  class MockClosure {
+   public:
+    MockClosure();
+    ~MockClosure();
+    MOCK_METHOD0(Run, void());
+    base::Closure GetCallback();
+  };
+
+  // A mock ErrorCallback.
+  class MockErrorCallback {
+   public:
+    MockErrorCallback();
+    ~MockErrorCallback();
+    MOCK_METHOD2(Run, void(const std::string& error_name,
+                           const std::string& error_mesage));
+    FlimflamClientHelper::ErrorCallback GetCallback();
+  };
+
   explicit FlimflamClientUnittestBase(const std::string& interface_name,
                                       const dbus::ObjectPath& object_path);
   virtual ~FlimflamClientUnittestBase();
@@ -113,6 +133,14 @@ class FlimflamClientUnittestBase : public testing::Test {
 
   // Checks the content of the method call and returns the response.
   // Used to implement the mock proxy.
+  void OnCallMethodWithErrorCallback(
+      dbus::MethodCall* method_call,
+      int timeout_ms,
+      const dbus::ObjectProxy::ResponseCallback& response_callback,
+      const dbus::ObjectProxy::ErrorCallback& error_callback);
+
+  // Checks the content of the method call and returns the response.
+  // Used to implement the mock proxy.
   dbus::Response* OnCallMethodAndBlock(dbus::MethodCall* method_call,
                                        int timeout_ms);
 
diff --git a/chromeos/dbus/flimflam_device_client.cc b/chromeos/dbus/flimflam_device_client.cc
index bc57475..eccc7ef 100644
--- a/chromeos/dbus/flimflam_device_client.cc
+++ b/chromeos/dbus/flimflam_device_client.cc
@@ -115,61 +115,71 @@ class FlimflamDeviceClientImpl : public FlimflamDeviceClient {
   virtual void RequirePin(const dbus::ObjectPath& device_path,
                           const std::string& pin,
                           bool require,
-                          const VoidCallback& callback) OVERRIDE  {
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamDeviceInterface,
                                  flimflam::kRequirePinFunction);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(pin);
     writer.AppendBool(require);
-    GetHelper(device_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(device_path)->CallVoidMethodWithErrorCallback(
+        &method_call, callback, error_callback);
   }
 
   // FlimflamProfileClient override.
   virtual void EnterPin(const dbus::ObjectPath& device_path,
                         const std::string& pin,
-                        const VoidCallback& callback) OVERRIDE {
+                        const base::Closure& callback,
+                        const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamDeviceInterface,
                                  flimflam::kEnterPinFunction);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(pin);
-    GetHelper(device_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(device_path)->CallVoidMethodWithErrorCallback(
+        &method_call, callback, error_callback);
   }
 
   // FlimflamProfileClient override.
   virtual void UnblockPin(const dbus::ObjectPath& device_path,
                           const std::string& puk,
                           const std::string& pin,
-                          const VoidCallback& callback) OVERRIDE {
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamDeviceInterface,
                                  flimflam::kUnblockPinFunction);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(puk);
     writer.AppendString(pin);
-    GetHelper(device_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(device_path)->CallVoidMethodWithErrorCallback(
+        &method_call, callback, error_callback);
   }
 
   // FlimflamProfileClient override.
   virtual void ChangePin(const dbus::ObjectPath& device_path,
                          const std::string& old_pin,
                          const std::string& new_pin,
-                         const VoidCallback& callback) OVERRIDE {
+                         const base::Closure& callback,
+                         const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamDeviceInterface,
                                  flimflam::kChangePinFunction);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(old_pin);
     writer.AppendString(new_pin);
-    GetHelper(device_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(device_path)->CallVoidMethodWithErrorCallback(
+        &method_call, callback, error_callback);
   }
 
   // FlimflamProfileClient override.
   virtual void Register(const dbus::ObjectPath& device_path,
                         const std::string& network_id,
-                        const VoidCallback& callback) OVERRIDE {
+                        const base::Closure& callback,
+                        const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamDeviceInterface,
                                  flimflam::kRegisterFunction);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(network_id);
-    GetHelper(device_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(device_path)->CallVoidMethodWithErrorCallback(
+        &method_call, callback, error_callback);
   }
 
  private:
@@ -271,38 +281,43 @@ class FlimflamDeviceClientStubImpl : public FlimflamDeviceClient {
   virtual void RequirePin(const dbus::ObjectPath& device_path,
                           const std::string& pin,
                           bool require,
-                          const VoidCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) OVERRIDE {
+    MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
   // FlimflamDeviceClient override.
   virtual void EnterPin(const dbus::ObjectPath& device_path,
                         const std::string& pin,
-                        const VoidCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                        const base::Closure& callback,
+                        const ErrorCallback& error_callback) OVERRIDE {
+    MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
   // FlimflamDeviceClient override.
   virtual void UnblockPin(const dbus::ObjectPath& device_path,
                           const std::string& puk,
                           const std::string& pin,
-                          const VoidCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) OVERRIDE {
+    MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
   // FlimflamDeviceClient override.
   virtual void ChangePin(const dbus::ObjectPath& device_path,
                          const std::string& old_pin,
                          const std::string& new_pin,
-                         const VoidCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                         const base::Closure& callback,
+                         const ErrorCallback& error_callback) OVERRIDE {
+    MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
   // FlimflamDeviceClient override.
   virtual void Register(const dbus::ObjectPath& device_path,
                         const std::string& network_id,
-                        const VoidCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                        const base::Closure& callback,
+                        const ErrorCallback& error_callback) OVERRIDE {
+    MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
  private:
diff --git a/chromeos/dbus/flimflam_device_client.h b/chromeos/dbus/flimflam_device_client.h
index 451a031..904d55d 100644
--- a/chromeos/dbus/flimflam_device_client.h
+++ b/chromeos/dbus/flimflam_device_client.h
@@ -38,6 +38,7 @@ class CHROMEOS_EXPORT FlimflamDeviceClient {
   typedef FlimflamClientHelper::VoidCallback VoidCallback;
   typedef FlimflamClientHelper::ObjectPathCallback ObjectPathCallback;
   typedef FlimflamClientHelper::DictionaryValueCallback DictionaryValueCallback;
+  typedef FlimflamClientHelper::ErrorCallback ErrorCallback;
 
   virtual ~FlimflamDeviceClient();
 
@@ -106,33 +107,38 @@ class CHROMEOS_EXPORT FlimflamDeviceClient {
   virtual void RequirePin(const dbus::ObjectPath& device_path,
                           const std::string& pin,
                           bool require,
-                          const VoidCallback& callback) = 0;
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) = 0;
 
   // Calls EnterPin method.
   // |callback| is called after the method call finishes.
   virtual void EnterPin(const dbus::ObjectPath& device_path,
                         const std::string& pin,
-                        const VoidCallback& callback) = 0;
+                        const base::Closure& callback,
+                        const ErrorCallback& error_callback) = 0;
 
   // Calls UnblockPin method.
   // |callback| is called after the method call finishes.
   virtual void UnblockPin(const dbus::ObjectPath& device_path,
                           const std::string& puk,
                           const std::string& pin,
-                          const VoidCallback& callback) = 0;
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) = 0;
 
   // Calls ChangePin method.
   // |callback| is called after the method call finishes.
   virtual void ChangePin(const dbus::ObjectPath& device_path,
                          const std::string& old_pin,
                          const std::string& new_pin,
-                         const VoidCallback& callback) = 0;
+                         const base::Closure& callback,
+                         const ErrorCallback& error_callback) = 0;
 
   // Calls Register method.
   // |callback| is called after the method call finishes.
   virtual void Register(const dbus::ObjectPath& device_path,
                         const std::string& network_id,
-                        const VoidCallback& callback) = 0;
+                        const base::Closure& callback,
+                        const ErrorCallback& error_callback) = 0;
 
  protected:
   // Create() should be used instead.
diff --git a/chromeos/dbus/flimflam_device_client_unittest.cc b/chromeos/dbus/flimflam_device_client_unittest.cc
index 201abb3..4ef5e68 100644
--- a/chromeos/dbus/flimflam_device_client_unittest.cc
+++ b/chromeos/dbus/flimflam_device_client_unittest.cc
@@ -245,16 +245,20 @@ TEST_F(FlimflamDeviceClientTest, RequirePin) {
   scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
 
   // Set expectations.
+  MockClosure mock_closure;
+  MockErrorCallback mock_error_callback;
   PrepareForMethodCall(flimflam::kRequirePinFunction,
                        base::Bind(&ExpectStringAndBoolArguments,
                                   kPin,
                                   kRequired),
                        response.get());
+  EXPECT_CALL(mock_closure, Run()).Times(1);
   // Call method.
   client_->RequirePin(dbus::ObjectPath(kExampleDevicePath),
                       kPin,
                       kRequired,
-                      base::Bind(&ExpectNoResultValue));
+                      mock_closure.GetCallback(),
+                      mock_error_callback.GetCallback());
   // Run the message loop.
   message_loop_.RunAllPending();
 }
@@ -265,14 +269,18 @@ TEST_F(FlimflamDeviceClientTest, EnterPin) {
   scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
 
   // Set expectations.
+  MockClosure mock_closure;
+  MockErrorCallback mock_error_callback;
   PrepareForMethodCall(flimflam::kEnterPinFunction,
                        base::Bind(&ExpectStringArgument,
                                   kPin),
                        response.get());
+  EXPECT_CALL(mock_closure, Run()).Times(1);
   // Call method.
   client_->EnterPin(dbus::ObjectPath(kExampleDevicePath),
                     kPin,
-                    base::Bind(&ExpectNoResultValue));
+                    mock_closure.GetCallback(),
+                    mock_error_callback.GetCallback());
   // Run the message loop.
   message_loop_.RunAllPending();
 }
@@ -284,14 +292,18 @@ TEST_F(FlimflamDeviceClientTest, UnblockPin) {
   scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
 
   // Set expectations.
+  MockClosure mock_closure;
+  MockErrorCallback mock_error_callback;
   PrepareForMethodCall(flimflam::kUnblockPinFunction,
                        base::Bind(&ExpectTwoStringArguments, kPuk, kPin),
                        response.get());
+  EXPECT_CALL(mock_closure, Run()).Times(1);
   // Call method.
   client_->UnblockPin(dbus::ObjectPath(kExampleDevicePath),
                       kPuk,
                       kPin,
-                      base::Bind(&ExpectNoResultValue));
+                      mock_closure.GetCallback(),
+                      mock_error_callback.GetCallback());
   // Run the message loop.
   message_loop_.RunAllPending();
 }
@@ -303,16 +315,20 @@ TEST_F(FlimflamDeviceClientTest, ChangePin) {
   scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
 
   // Set expectations.
+  MockClosure mock_closure;
+  MockErrorCallback mock_error_callback;
   PrepareForMethodCall(flimflam::kChangePinFunction,
                        base::Bind(&ExpectTwoStringArguments,
                                   kOldPin,
                                   kNewPin),
                        response.get());
+  EXPECT_CALL(mock_closure, Run()).Times(1);
   // Call method.
   client_->ChangePin(dbus::ObjectPath(kExampleDevicePath),
                      kOldPin,
                      kNewPin,
-                     base::Bind(&ExpectNoResultValue));
+                     mock_closure.GetCallback(),
+                     mock_error_callback.GetCallback());
   // Run the message loop.
   message_loop_.RunAllPending();
 }
@@ -323,13 +339,17 @@ TEST_F(FlimflamDeviceClientTest, Register) {
   scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
 
   // Set expectations.
+  MockClosure mock_closure;
+  MockErrorCallback mock_error_callback;
   PrepareForMethodCall(flimflam::kRegisterFunction,
                        base::Bind(&ExpectStringArgument, kNetworkId),
                        response.get());
+  EXPECT_CALL(mock_closure, Run()).Times(1);
   // Call method.
   client_->Register(dbus::ObjectPath(kExampleDevicePath),
                     kNetworkId,
-                    base::Bind(&ExpectNoResultValue));
+                    mock_closure.GetCallback(),
+                    mock_error_callback.GetCallback());
   // Run the message loop.
   message_loop_.RunAllPending();
 }
diff --git a/chromeos/dbus/flimflam_service_client.cc b/chromeos/dbus/flimflam_service_client.cc
index 40ff77e..ec8e1c9 100644
--- a/chromeos/dbus/flimflam_service_client.cc
+++ b/chromeos/dbus/flimflam_service_client.cc
@@ -73,10 +73,12 @@ class FlimflamServiceClientImpl : public FlimflamServiceClient {
 
   // FlimflamServiceClient override.
   virtual void Connect(const dbus::ObjectPath& service_path,
-                       const VoidCallback& callback) OVERRIDE {
+                       const base::Closure& callback,
+                       const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamServiceInterface,
                                  flimflam::kConnectFunction);
-    GetHelper(service_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(service_path)->CallVoidMethodWithErrorCallback(
+        &method_call, callback, error_callback);
   }
 
   // FlimflamServiceClient override.
@@ -174,8 +176,9 @@ class FlimflamServiceClientStubImpl : public FlimflamServiceClient {
 
   // FlimflamServiceClient override.
   virtual void Connect(const dbus::ObjectPath& service_path,
-                       const VoidCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                       const base::Closure& callback,
+                       const ErrorCallback& error_callback) OVERRIDE {
+    MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
   // FlimflamServiceClient override.
diff --git a/chromeos/dbus/flimflam_service_client.h b/chromeos/dbus/flimflam_service_client.h
index 081816a..786c0bb 100644
--- a/chromeos/dbus/flimflam_service_client.h
+++ b/chromeos/dbus/flimflam_service_client.h
@@ -38,6 +38,7 @@ class CHROMEOS_EXPORT FlimflamServiceClient {
   typedef FlimflamClientHelper::PropertyChangedHandler PropertyChangedHandler;
   typedef FlimflamClientHelper::VoidCallback VoidCallback;
   typedef FlimflamClientHelper::DictionaryValueCallback DictionaryValueCallback;
+  typedef FlimflamClientHelper::ErrorCallback ErrorCallback;
 
   virtual ~FlimflamServiceClient();
 
@@ -76,7 +77,8 @@ class CHROMEOS_EXPORT FlimflamServiceClient {
   // Calls Connect method.
   // |callback| is called after the method call succeeds.
   virtual void Connect(const dbus::ObjectPath& service_path,
-                       const VoidCallback& callback) = 0;
+                       const base::Closure& callback,
+                       const ErrorCallback& error_callback) = 0;
 
   // Calls Disconnect method.
   // |callback| is called after the method call succeeds.
diff --git a/chromeos/dbus/flimflam_service_client_unittest.cc b/chromeos/dbus/flimflam_service_client_unittest.cc
index 92b802e..dafea40 100644
--- a/chromeos/dbus/flimflam_service_client_unittest.cc
+++ b/chromeos/dbus/flimflam_service_client_unittest.cc
@@ -138,12 +138,17 @@ TEST_F(FlimflamServiceClientTest, Connect) {
   scoped_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
 
   // Set expectations.
+  MockClosure mock_closure;
+  MockErrorCallback mock_error_callback;
   PrepareForMethodCall(flimflam::kConnectFunction,
                        base::Bind(&ExpectNoArgument),
                        response.get());
+  EXPECT_CALL(mock_closure, Run()).Times(1);
   // Call method.
   client_->Connect(dbus::ObjectPath(kExampleServicePath),
-                   base::Bind(&ExpectNoResultValue));
+                   mock_closure.GetCallback(),
+                   mock_error_callback.GetCallback());
+
   // Run the message loop.
   message_loop_.RunAllPending();
 }
diff --git a/chromeos/dbus/mock_flimflam_device_client.h b/chromeos/dbus/mock_flimflam_device_client.h
index 0360000..c68ebd20 100644
--- a/chromeos/dbus/mock_flimflam_device_client.h
+++ b/chromeos/dbus/mock_flimflam_device_client.h
@@ -41,24 +41,29 @@ class MockFlimflamDeviceClient : public FlimflamDeviceClient {
   MOCK_METHOD2(CallAddIPConfigAndBlock,
                dbus::ObjectPath(const dbus::ObjectPath& device_path,
                                 const std::string& method));
-  MOCK_METHOD4(RequirePin, void(const dbus::ObjectPath& device_path,
+  MOCK_METHOD5(RequirePin, void(const dbus::ObjectPath& device_path,
                                 const std::string& pin,
                                 bool require,
-                                const VoidCallback& callback));
-  MOCK_METHOD3(EnterPin, void(const dbus::ObjectPath& device_path,
+                                const base::Closure& callback,
+                                const ErrorCallback& error_callback));
+  MOCK_METHOD4(EnterPin, void(const dbus::ObjectPath& device_path,
                               const std::string& pin,
-                              const VoidCallback& callback));
-  MOCK_METHOD4(UnblockPin, void(const dbus::ObjectPath& device_path,
+                              const base::Closure& callback,
+                              const ErrorCallback& error_callback));
+  MOCK_METHOD5(UnblockPin, void(const dbus::ObjectPath& device_path,
                                 const std::string& puk,
                                 const std::string& pin,
-                                const VoidCallback& callback));
-  MOCK_METHOD4(ChangePin, void(const dbus::ObjectPath& device_path,
+                                const base::Closure& callback,
+                                const ErrorCallback& error_callback));
+  MOCK_METHOD5(ChangePin, void(const dbus::ObjectPath& device_path,
                                const std::string& old_pin,
                                const std::string& new_pin,
-                               const VoidCallback& callback));
-  MOCK_METHOD3(Register, void(const dbus::ObjectPath& device_path,
+                               const base::Closure& callback,
+                               const ErrorCallback& error_callback));
+  MOCK_METHOD4(Register, void(const dbus::ObjectPath& device_path,
                               const std::string& network_id,
-                              const VoidCallback& callback));
+                              const base::Closure& callback,
+                              const ErrorCallback& error_callback));
 };
 
 }  // namespace chromeos
diff --git a/chromeos/dbus/mock_flimflam_service_client.h b/chromeos/dbus/mock_flimflam_service_client.h
index 4a67b4b..bbfe6ec 100644
--- a/chromeos/dbus/mock_flimflam_service_client.h
+++ b/chromeos/dbus/mock_flimflam_service_client.h
@@ -31,8 +31,9 @@ class MockFlimflamServiceClient : public FlimflamServiceClient {
   MOCK_METHOD3(ClearProperty, void(const dbus::ObjectPath& service_path,
                                    const std::string& name,
                                    const VoidCallback& callback));
-  MOCK_METHOD2(Connect, void(const dbus::ObjectPath& service_path,
-                             const VoidCallback& callback));
+  MOCK_METHOD3(Connect, void(const dbus::ObjectPath& service_path,
+                             const base::Closure& callback,
+                             const ErrorCallback& error_callback));
   MOCK_METHOD2(Disconnect, void(const dbus::ObjectPath& service_path,
                                 const VoidCallback& callback));
   MOCK_METHOD2(Remove, void(const dbus::ObjectPath& service_path,