This converts the Shill clients to allow propagation of shill errors

chromeos/dbus/shill_service_client.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 "chromeos/dbus/shill_service_client.h"
 
 #include "base/bind.h"
 #include "base/chromeos/chromeos_version.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
@@ skipping 29 matching lines @@
 }
 
 // The ShillServiceClient implementation.
 class ShillServiceClientImpl : public ShillServiceClient {
  public:
   explicit ShillServiceClientImpl(dbus::Bus* bus)
       : bus_(bus),
         helpers_deleter_(&helpers_) {
   }
 
-  // ShillServiceClient override.
-  virtual void SetPropertyChangedHandler(
+  /////////////////////////////////////
+  // ShillServiceClient overrides.
+  virtual void AddPropertyChangedObserver(
       const dbus::ObjectPath& service_path,
-      const PropertyChangedHandler& handler) OVERRIDE {
-    GetHelper(service_path)->SetPropertyChangedHandler(handler);
+      PropertyChangedObserver* observer) OVERRIDE {
+    GetHelper(service_path)->AddPropertyChangedObserver(observer);
   }
 
-  // ShillServiceClient override.
-  virtual void ResetPropertyChangedHandler(
-      const dbus::ObjectPath& service_path) OVERRIDE {
-    GetHelper(service_path)->ResetPropertyChangedHandler();
+  virtual void RemovePropertyChangedObserver(
+      const dbus::ObjectPath& service_path,
+      PropertyChangedObserver* observer) OVERRIDE {
+    GetHelper(service_path)->RemovePropertyChangedObserver(observer);
   }
 
-  // ShillServiceClient override.
   virtual void GetProperties(const dbus::ObjectPath& service_path,
                              const DictionaryValueCallback& callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamServiceInterface,
                                  flimflam::kGetPropertiesFunction);
     GetHelper(service_path)->CallDictionaryValueMethodWithErrorCallback(
         &method_call,
         base::Bind(callback, DBUS_METHOD_CALL_SUCCESS),
         base::Bind(&OnGetPropertiesError, service_path, callback));
   }
 
-  // ShillServiceClient override.
   virtual void SetProperty(const dbus::ObjectPath& service_path,
                            const std::string& name,
                            const base::Value& value,
-                           const VoidDBusMethodCallback& callback) OVERRIDE {
+                           const base::Closure& callback,
+                           const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamServiceInterface,
                                  flimflam::kSetPropertyFunction);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(name);
     ShillClientHelper::AppendValueDataAsVariant(&writer, value);
-    GetHelper(service_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(service_path)->CallVoidMethodWithErrorCallback(&method_call,
+                                                             callback,
+                                                             error_callback);
   }
 
-  // ShillServiceClient override.
   virtual void ClearProperty(const dbus::ObjectPath& service_path,
                              const std::string& name,
-                             const VoidDBusMethodCallback& callback) OVERRIDE {
+                             const base::Closure& callback,
+                             const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamServiceInterface,
                                  flimflam::kClearPropertyFunction);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(name);
-    GetHelper(service_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(service_path)->CallVoidMethodWithErrorCallback(&method_call,
+                                                             callback,
+                                                             error_callback);
   }
 
-  // ShillServiceClient override.
   virtual void Connect(const dbus::ObjectPath& service_path,
                        const base::Closure& callback,
                        const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamServiceInterface,
                                  flimflam::kConnectFunction);
     GetHelper(service_path)->CallVoidMethodWithErrorCallback(
         &method_call, callback, error_callback);
   }
 
-  // ShillServiceClient override.
   virtual void Disconnect(const dbus::ObjectPath& service_path,
-                          const VoidDBusMethodCallback& callback) OVERRIDE {
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamServiceInterface,
                                  flimflam::kDisconnectFunction);
-    GetHelper(service_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(service_path)->CallVoidMethodWithErrorCallback(&method_call,
+                                                             callback,
+                                                             error_callback);
   }
 
-  // ShillServiceClient override.
   virtual void Remove(const dbus::ObjectPath& service_path,
-                      const VoidDBusMethodCallback& callback) OVERRIDE {
+                      const base::Closure& callback,
+                      const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamServiceInterface,
                                  flimflam::kRemoveServiceFunction);
-    GetHelper(service_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(service_path)->CallVoidMethodWithErrorCallback(&method_call,
+                                                             callback,
+                                                             error_callback);
   }
 
-  // ShillServiceClient override.
   virtual void ActivateCellularModem(
       const dbus::ObjectPath& service_path,
       const std::string& carrier,
-      const VoidDBusMethodCallback& callback) OVERRIDE {
+      const base::Closure& callback,
+      const ErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamServiceInterface,
                                  flimflam::kActivateCellularModemFunction);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(carrier);
-    GetHelper(service_path)->CallVoidMethod(&method_call, callback);
+    GetHelper(service_path)->CallVoidMethodWithErrorCallback(&method_call,
+                                                             callback,
+                                                             error_callback);
   }
 
-  // ShillServiceClient override.
   virtual bool CallActivateCellularModemAndBlock(
       const dbus::ObjectPath& service_path,
       const std::string& carrier) OVERRIDE {
     dbus::MethodCall method_call(flimflam::kFlimflamServiceInterface,
                                  flimflam::kActivateCellularModemFunction);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(carrier);
     return GetHelper(service_path)->CallVoidMethodAndBlock(&method_call);
   }
 
@@ skipping 22 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(ShillServiceClientImpl);
 };
 
 // A stub implementation of ShillServiceClient.
 class ShillServiceClientStubImpl : public ShillServiceClient {
  public:
   ShillServiceClientStubImpl() : weak_ptr_factory_(this) {}
 
   virtual ~ShillServiceClientStubImpl() {}
 
-  // ShillServiceClient override.
-  virtual void SetPropertyChangedHandler(
+  ///////////////////////////////////
+  // ShillServiceClient overrides.
+  virtual void AddPropertyChangedObserver(
       const dbus::ObjectPath& service_path,
-      const PropertyChangedHandler& handler) OVERRIDE {}
+      PropertyChangedObserver* observer) OVERRIDE {}
 
-  // ShillServiceClient override.
-  virtual void ResetPropertyChangedHandler(
-      const dbus::ObjectPath& service_path) OVERRIDE {}
+  virtual void RemovePropertyChangedObserver(
+      const dbus::ObjectPath& service_path,
+      PropertyChangedObserver* observer) OVERRIDE {}
 
-  // ShillServiceClient override.
   virtual void GetProperties(const dbus::ObjectPath& service_path,
                              const DictionaryValueCallback& callback) OVERRIDE {
     MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(&ShillServiceClientStubImpl::PassEmptyDictionaryValue,
                    weak_ptr_factory_.GetWeakPtr(),
                    callback));
   }
 
-  // ShillServiceClient override.
   virtual void SetProperty(const dbus::ObjectPath& service_path,
                            const std::string& name,
                            const base::Value& value,
-                           const VoidDBusMethodCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                           const base::Closure& callback,
+                           const ErrorCallback& error_callback) OVERRIDE {
+    PostClosure(callback);
   }
 
-  // ShillServiceClient override.
   virtual void ClearProperty(const dbus::ObjectPath& service_path,
                              const std::string& name,
-                             const VoidDBusMethodCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                             const base::Closure& callback,
+                             const ErrorCallback& error_callback) OVERRIDE {
+    PostClosure(callback);
   }
 
-  // ShillServiceClient override.
   virtual void Connect(const dbus::ObjectPath& service_path,
                        const base::Closure& callback,
                        const ErrorCallback& error_callback) OVERRIDE {
     MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
-  // ShillServiceClient override.
   virtual void Disconnect(const dbus::ObjectPath& service_path,
-                          const VoidDBusMethodCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) OVERRIDE {
+    PostClosure(callback);
   }
 
-  // ShillServiceClient override.
   virtual void Remove(const dbus::ObjectPath& service_path,
-                      const VoidDBusMethodCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+                      const base::Closure& callback,
+                      const ErrorCallback& error_callback) OVERRIDE {
+    PostClosure(callback);
   }
 
-  // ShillServiceClient override.
   virtual void ActivateCellularModem(
       const dbus::ObjectPath& service_path,
       const std::string& carrier,
-      const VoidDBusMethodCallback& callback) OVERRIDE {
-    PostSuccessVoidCallback(callback);
+      const base::Closure& callback,
+      const ErrorCallback& error_callback) OVERRIDE {
+    PostClosure(callback);
   }
 
-  // ShillServiceClient override.
   virtual bool CallActivateCellularModemAndBlock(
       const dbus::ObjectPath& service_path,
       const std::string& carrier) OVERRIDE {
     return true;
   }
 
  private:
   void PassEmptyDictionaryValue(const DictionaryValueCallback& callback) const {
     base::DictionaryValue dictionary;
     callback.Run(DBUS_METHOD_CALL_SUCCESS, dictionary);
   }
 
-  // Posts a task to run a void callback with success status code.
-  void PostSuccessVoidCallback(const VoidDBusMethodCallback& callback) {
-    MessageLoop::current()->PostTask(FROM_HERE,
-                                     base::Bind(callback,
-                                                DBUS_METHOD_CALL_SUCCESS));
+  // Posts a task to run a closure.
+  void PostClosure(const base::Closure& callback) {

[hashimoto, 2012/09/21 11:52:01]

Since this method became very simple, we no longer need this?

[Greg Spencer (Chromium), 2012/09/21 22:03:47]

Removed.

+    MessageLoop::current()->PostTask(FROM_HERE, callback);
   }
 
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<ShillServiceClientStubImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ShillServiceClientStubImpl);
 };
 
 }  // namespace
 
 ShillServiceClient::ShillServiceClient() {}
 
 ShillServiceClient::~ShillServiceClient() {}
 
 // static
 ShillServiceClient* ShillServiceClient::Create(
     DBusClientImplementationType type,
     dbus::Bus* bus) {
   if (type == REAL_DBUS_CLIENT_IMPLEMENTATION)
     return new ShillServiceClientImpl(bus);
   DCHECK_EQ(STUB_DBUS_CLIENT_IMPLEMENTATION, type);
   return new ShillServiceClientStubImpl();
 }
 
 }  // namespace chromeos