bluetooth: obtain D-Bus error for select methods

For the CreateDevice, CreatePairedDevice and Connect methods use
CallMethodWithErrorCallback rather than CallMethod so that we obtain
richer error information from the BlueZ service.

BUG=chromium-os:27902
TEST=make chrome

Change-Id: If801d834604e4c2142c20169603bc527bde09403

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

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

6 files changed, 187 insertions, 70 deletions

diff --git a/chromeos/dbus/bluetooth_adapter_client.cc b/chromeos/dbus/bluetooth_adapter_client.cc
index 566d470..7939d24 100644
--- a/chromeos/dbus/bluetooth_adapter_client.cc
+++ b/chromeos/dbus/bluetooth_adapter_client.cc
@@ -20,6 +20,11 @@
 
 namespace chromeos {
 
+const char BluetoothAdapterClient::kNoResponseError[] =
+    "org.chromium.Error.NoResponse";
+const char BluetoothAdapterClient::kBadResponseError[] =
+    "org.chromium.Error.BadResponse";
+
 BluetoothAdapterClient::Properties::Properties(dbus::ObjectProxy* object_proxy,
                                                PropertyChangedCallback callback)
     : BluetoothPropertySet(object_proxy,
@@ -175,7 +180,9 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
   // BluetoothAdapterClient override.
   virtual void CreateDevice(const dbus::ObjectPath& object_path,
                             const std::string& address,
-                            const DeviceCallback& callback) OVERRIDE {
+                            const CreateDeviceCallback& callback,
+                            const CreateDeviceErrorCallback& error_callback)
+      OVERRIDE {
     dbus::MethodCall method_call(
         bluetooth_adapter::kBluetoothAdapterInterface,
         bluetooth_adapter::kCreateDevice);
@@ -185,19 +192,23 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
 
     dbus::ObjectProxy* object_proxy = GetObjectProxy(object_path);
 
-    object_proxy->CallMethod(
+    object_proxy->CallMethodWithErrorCallback(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(&BluetoothAdapterClientImpl::OnCreateDevice,
-                   weak_ptr_factory_.GetWeakPtr(), object_path, callback));
+                   weak_ptr_factory_.GetWeakPtr(), object_path,
+                   callback, error_callback),
+        base::Bind(&BluetoothAdapterClientImpl::OnCreateDeviceError,
+                   weak_ptr_factory_.GetWeakPtr(), object_path,
+                   error_callback));
   }
 
   // BluetoothAdapterClient override.
-  virtual void CreatePairedDevice(const dbus::ObjectPath& object_path,
-                                  const std::string& address,
-                                  const dbus::ObjectPath& agent_path,
-                                  const std::string& capability,
-                                  const DeviceCallback& callback) OVERRIDE {
+  virtual void CreatePairedDevice(
+      const dbus::ObjectPath& object_path, const std::string& address,
+      const dbus::ObjectPath& agent_path, const std::string& capability,
+      const CreateDeviceCallback& callback,
+      const CreateDeviceErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(
         bluetooth_adapter::kBluetoothAdapterInterface,
         bluetooth_adapter::kCreatePairedDevice);
@@ -209,11 +220,15 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
 
     dbus::ObjectProxy* object_proxy = GetObjectProxy(object_path);
 
-    object_proxy->CallMethod(
+    object_proxy->CallMethodWithErrorCallback(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(&BluetoothAdapterClientImpl::OnCreatePairedDevice,
-                   weak_ptr_factory_.GetWeakPtr(), object_path, callback));
+                   weak_ptr_factory_.GetWeakPtr(), object_path,
+                   callback, error_callback),
+        base::Bind(&BluetoothAdapterClientImpl::OnCreatePairedDeviceError,
+                   weak_ptr_factory_.GetWeakPtr(), object_path,
+                   error_callback));
   }
 
   // BluetoothAdapterClient override.
@@ -294,7 +309,7 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
     object_proxy->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&BluetoothAdapterClientImpl::OnCreateDevice,
+        base::Bind(&BluetoothAdapterClientImpl::OnUnregisterAgent,
                    weak_ptr_factory_.GetWeakPtr(), object_path, callback));
   }
 
@@ -586,48 +601,79 @@ class BluetoothAdapterClientImpl: public BluetoothAdapterClient,
 
   // Called when a response for CreateDevice() is received.
   void OnCreateDevice(const dbus::ObjectPath& object_path,
-                      const DeviceCallback& callback,
+                      const CreateDeviceCallback& callback,
+                      const CreateDeviceErrorCallback& error_callback,
                       dbus::Response* response) {
     // Parse response.
-    bool success = false;
+    DCHECK(response);
     dbus::ObjectPath device_path;
-    if (response != NULL) {
-      dbus::MessageReader reader(response);
-      if (!reader.PopObjectPath(&device_path)) {
-        LOG(WARNING) << "CreateDevice response has incorrect parameters: "
-                     << response->ToString();
-      } else {
-        success = true;
-      }
-    } else {
-      LOG(WARNING) << "Failed to create device.";
+    dbus::MessageReader reader(response);
+    if (!reader.PopObjectPath(&device_path)) {
+      LOG(WARNING) << "CreateDevice response has incorrect parameters: "
+                   << response->ToString();
+      error_callback.Run(kBadResponseError, "");
+      return;
     }
 
     // Notify client.
-    callback.Run(device_path, success);
+    callback.Run(device_path);
+  }
+
+  // Called when an error for CreateDevice() is received.
+  void OnCreateDeviceError(const dbus::ObjectPath& object_path,
+                           const CreateDeviceErrorCallback& error_callback,
+                           dbus::ErrorResponse* response) {
+    // Error response has optional error message argument.
+    std::string error_name;
+    std::string error_message;
+    if (response) {
+      dbus::MessageReader reader(response);
+      error_name = response->GetErrorName();
+      error_message = reader.PopString(&error_message);
+    } else {
+      error_name = kNoResponseError;
+      error_message = "";
+    }
+    error_callback.Run(error_name, error_message);
   }
 
   // Called when a response for CreatePairedDevice() is received.
   void OnCreatePairedDevice(const dbus::ObjectPath& object_path,
-                            const DeviceCallback& callback,
+                            const CreateDeviceCallback& callback,
+                            const CreateDeviceErrorCallback& error_callback,
                             dbus::Response* response) {
     // Parse response.
-    bool success = false;
+    DCHECK(response);
     dbus::ObjectPath device_path;
-    if (response != NULL) {
-      dbus::MessageReader reader(response);
-      if (!reader.PopObjectPath(&device_path)) {
-        LOG(WARNING) << "CreatePairedDevice response has incorrect parameters: "
-                     << response->ToString();
-      } else {
-        success = true;
-      }
-    } else {
-      LOG(WARNING) << "Failed to create paired device.";
+    dbus::MessageReader reader(response);
+    if (!reader.PopObjectPath(&device_path)) {
+      LOG(WARNING) << "CreatePairedDevice response has incorrect parameters: "
+                   << response->ToString();
+      error_callback.Run(kBadResponseError, "");
+      return;
     }
 
     // Notify client.
-    callback.Run(device_path, success);
+    callback.Run(device_path);
+  }
+
+  // Called when an error for CreatePairedDevice() is received.
+  void OnCreatePairedDeviceError(
+      const dbus::ObjectPath& object_path,
+      const CreateDeviceErrorCallback& error_callback,
+      dbus::ErrorResponse* response) {
+    // Error response has optional error message argument.
+    std::string error_name;
+    std::string error_message;
+    if (response) {
+      dbus::MessageReader reader(response);
+      error_name = response->GetErrorName();
+      error_message = reader.PopString(&error_message);
+    } else {
+      error_name = kNoResponseError;
+      error_message = "";
+    }
+    error_callback.Run(error_name, error_message);
   }
 
   // Called when a response for CancelDeviceCreation() is received.
@@ -736,20 +782,22 @@ class BluetoothAdapterClientStubImpl : public BluetoothAdapterClient {
   // BluetoothAdapterClient override.
   virtual void CreateDevice(const dbus::ObjectPath& object_path,
                             const std::string& address,
-                            const DeviceCallback& callback) OVERRIDE {
+                            const CreateDeviceCallback& callback,
+                            const CreateDeviceErrorCallback& error_callback)
+      OVERRIDE {
     VLOG(1) << "CreateDevice: " << object_path.value() << " " << address;
-    callback.Run(dbus::ObjectPath(), false);
+    error_callback.Run("", "");
   }
 
   // BluetoothAdapterClient override.
-  virtual void CreatePairedDevice(const dbus::ObjectPath& object_path,
-                                  const std::string& address,
-                                  const dbus::ObjectPath& agent_path,
-                                  const std::string& capability,
-                                  const DeviceCallback& callback) OVERRIDE {
+  virtual void CreatePairedDevice(
+      const dbus::ObjectPath& object_path, const std::string& address,
+      const dbus::ObjectPath& agent_path, const std::string& capability,
+      const CreateDeviceCallback& callback,
+      const CreateDeviceErrorCallback& error_callback) OVERRIDE {
     VLOG(1) << "CreatePairedDevice: " << object_path.value() << " " << address
             << " " << agent_path.value() << " " << capability;
-    callback.Run(dbus::ObjectPath(), false);
+    error_callback.Run("", "");
   }
 
   // BluetoothAdapterClient override.
diff --git a/chromeos/dbus/bluetooth_adapter_client.h b/chromeos/dbus/bluetooth_adapter_client.h
index 6e59429..52c8692 100644
--- a/chromeos/dbus/bluetooth_adapter_client.h
+++ b/chromeos/dbus/bluetooth_adapter_client.h
@@ -171,14 +171,30 @@ class CHROMEOS_EXPORT BluetoothAdapterClient {
                           const std::string& address,
                           const DeviceCallback& callback) = 0;
 
+  // The CreateDeviceCallback is used for the CreateDevice and
+  // CreatePairedDevice adapter methods that return a dbus object path for
+  // a remote device on success. It receives a single argument, the
+  // |object_path| of the device returned by the method.
+  typedef base::Callback<void(const dbus::ObjectPath&)> CreateDeviceCallback;
+
+  // The CreateDeviceErrorCallback is used for the CreateDevice and
+  // CreatePairedDevices adapter methods to indicate failure. It receives
+  // two arguments, the name of the error in |error_name| and an optional
+  // message in |error_message|.
+  typedef base::Callback<void(const std::string& error_name,
+                              const std::string& error_message)>
+      CreateDeviceErrorCallback;
+
   // Creates a new dbus object from the adapter with object path |object_path|
   // to the remote device with address |address|, connecting to it and
   // retrieving all SDP records. After a successful call, the device is known
   // and appear's in the adapter's |devices| interface. This is a low-security
   // connection which may not be accepted by the device.
-  virtual void CreateDevice(const dbus::ObjectPath& object_path,
-                            const std::string& address,
-                            const DeviceCallback& callback) = 0;
+  virtual void CreateDevice(
+      const dbus::ObjectPath& object_path,
+      const std::string& address,
+      const CreateDeviceCallback& callback,
+      const CreateDeviceErrorCallback& error_callback) = 0;
 
   // Creates a new dbus object from the adapter with object path |object_path|
   // to the remote device with address |address|, connecting to it, retrieving
@@ -189,11 +205,13 @@ class CHROMEOS_EXPORT BluetoothAdapterClient {
   // must be specified to negotiate the pairing, |capability| specifies the
   // input and display capabilities of that agent and should be one of the
   // constants declared in the bluetooth_agent:: namespace.
-  virtual void CreatePairedDevice(const dbus::ObjectPath& object_path,
-                                  const std::string& address,
-                                  const dbus::ObjectPath& agent_path,
-                                  const std::string& capability,
-                                  const DeviceCallback& callback) = 0;
+  virtual void CreatePairedDevice(
+      const dbus::ObjectPath& object_path,
+      const std::string& address,
+      const dbus::ObjectPath& agent_path,
+      const std::string& capability,
+      const CreateDeviceCallback& callback,
+      const CreateDeviceErrorCallback& error_callback) = 0;
 
   // Cancels the currently in progress call to CreateDevice() or
   // CreatePairedDevice() on the adapter with object path |object_path|
@@ -232,6 +250,10 @@ class CHROMEOS_EXPORT BluetoothAdapterClient {
                                         dbus::Bus* bus,
                                         BluetoothManagerClient* manager_client);
 
+  // Constants used to indicate exceptional error conditions.
+  static const char kNoResponseError[];
+  static const char kBadResponseError[];
+
  protected:
   BluetoothAdapterClient();
 
diff --git a/chromeos/dbus/bluetooth_input_client.cc b/chromeos/dbus/bluetooth_input_client.cc
index 6e9b890..8cdf0a4 100644
--- a/chromeos/dbus/bluetooth_input_client.cc
+++ b/chromeos/dbus/bluetooth_input_client.cc
@@ -19,6 +19,9 @@
 
 namespace chromeos {
 
+const char BluetoothInputClient::kNoResponseError[] =
+    "org.chromium.Error.NoResponse";
+
 BluetoothInputClient::Properties::Properties(dbus::ObjectProxy* object_proxy,
                                              PropertyChangedCallback callback)
     : BluetoothPropertySet(object_proxy,
@@ -77,18 +80,23 @@ class BluetoothInputClientImpl: public BluetoothInputClient,
 
   // BluetoothInputClient override.
   virtual void Connect(const dbus::ObjectPath& object_path,
-                       const InputCallback& callback) OVERRIDE {
+                       const ConnectCallback& callback,
+                       const ConnectErrorCallback& error_callback) OVERRIDE {
     dbus::MethodCall method_call(
         bluetooth_input::kBluetoothInputInterface,
         bluetooth_input::kConnect);
 
     dbus::ObjectProxy* object_proxy = GetObjectProxy(object_path);
 
-    object_proxy->CallMethod(
+    object_proxy->CallMethodWithErrorCallback(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(&BluetoothInputClientImpl::OnConnect,
-                   weak_ptr_factory_.GetWeakPtr(), object_path, callback));
+                   weak_ptr_factory_.GetWeakPtr(), object_path,
+                   callback),
+        base::Bind(&BluetoothInputClientImpl::OnConnectError,
+                   weak_ptr_factory_.GetWeakPtr(), object_path,
+                   error_callback));
   }
 
   // BluetoothInputClient override.
@@ -183,11 +191,28 @@ class BluetoothInputClientImpl: public BluetoothInputClient,
 
   // Called when a response for Connect() is received.
   void OnConnect(const dbus::ObjectPath& object_path,
-                 const InputCallback& callback,
+                 const ConnectCallback& callback,
                  dbus::Response* response) {
-    LOG_IF(WARNING, !response) << object_path.value()
-                               << ": OnConnect: failed.";
-    callback.Run(object_path, response);
+    DCHECK(response);
+    callback.Run(object_path);
+  }
+
+  // Called when an error for Connect() is received.
+  void OnConnectError(const dbus::ObjectPath& object_path,
+                      const ConnectErrorCallback& error_callback,
+                      dbus::ErrorResponse* response) {
+    // Error response has optional error message argument.
+    std::string error_name;
+    std::string error_message;
+    if (response) {
+      dbus::MessageReader reader(response);
+      error_name = response->GetErrorName();
+      error_message = reader.PopString(&error_message);
+    } else {
+      error_name = kNoResponseError;
+      error_message = "";
+    }
+    error_callback.Run(object_path, error_name, error_message);
   }
 
   // Called when a response for Disconnect() is received.
@@ -232,9 +257,10 @@ class BluetoothInputClientStubImpl : public BluetoothInputClient {
 
   // BluetoothInputClient override.
   virtual void Connect(const dbus::ObjectPath& object_path,
-                       const InputCallback& callback) OVERRIDE {
+                       const ConnectCallback& callback,
+                       const ConnectErrorCallback& error_callback) OVERRIDE {
     VLOG(1) << "Connect: " << object_path.value();
-    callback.Run(object_path, false);
+    error_callback.Run(object_path, "", "");
   }
 
   // BluetoothInputClient override.
diff --git a/chromeos/dbus/bluetooth_input_client.h b/chromeos/dbus/bluetooth_input_client.h
index 2c1e015..aece6e6 100644
--- a/chromeos/dbus/bluetooth_input_client.h
+++ b/chromeos/dbus/bluetooth_input_client.h
@@ -65,14 +65,29 @@ class CHROMEOS_EXPORT BluetoothInputClient {
 
   // The InputCallback is used for input device methods that only return to
   // indicate success. It receives two arguments, the |object_path| of the
-  // input devuce the call was made on and |success| which indicates whether
+  // input device the call was made on and |success| which indicates whether
   // or not the request succeeded.
   typedef base::Callback<void(const dbus::ObjectPath&, bool)> InputCallback;
 
+  // The ConnectCallback is used for the Connect input device method to
+  // indicate success. It receives a single argument, the |object_path| of
+  // the input device the call was made on.
+  typedef base::Callback<void(const dbus::ObjectPath&)> ConnectCallback;
+
+  // The ConnectErrorCallback is used for the Connect input device method
+  // to indicate failure. It receives three arguments, the |object_path| of
+  // the input device the call was made on, the name of the error in
+  // |error_name| and an optional message in |error_message|.
+  typedef base::Callback<void(const dbus::ObjectPath& object_path,
+                              const std::string& error_name,
+                              const std::string& error_message)>
+      ConnectErrorCallback;
+
   // Connects the input subsystem to the device with object path
   // |object_path|, which should already be a known device on the adapter.
   virtual void Connect(const dbus::ObjectPath& object_path,
-                       const InputCallback& callback) = 0;
+                       const ConnectCallback& callback,
+                       const ConnectErrorCallback& error_callback) = 0;
 
   // Disconnects the input subsystem from the device with object path
   // |object_path| without terminating the low-level ACL connection,
@@ -84,6 +99,9 @@ class CHROMEOS_EXPORT BluetoothInputClient {
                                       dbus::Bus* bus,
                                       BluetoothAdapterClient* adapter_client);
 
+  // Constants used to indicate exceptional error conditions.
+  static const char kNoResponseError[];
+
  protected:
   BluetoothInputClient();
 
diff --git a/chromeos/dbus/mock_bluetooth_adapter_client.h b/chromeos/dbus/mock_bluetooth_adapter_client.h
index 4c7e389..a823945 100644
--- a/chromeos/dbus/mock_bluetooth_adapter_client.h
+++ b/chromeos/dbus/mock_bluetooth_adapter_client.h
@@ -31,14 +31,16 @@ class MockBluetoothAdapterClient : public BluetoothAdapterClient {
   MOCK_METHOD3(FindDevice, void(const dbus::ObjectPath&,
                                 const std::string&,
                                 const DeviceCallback&));
-  MOCK_METHOD3(CreateDevice, void(const dbus::ObjectPath&,
+  MOCK_METHOD4(CreateDevice, void(const dbus::ObjectPath&,
                                   const std::string&,
-                                  const DeviceCallback&));
-  MOCK_METHOD5(CreatePairedDevice, void(const dbus::ObjectPath&,
+                                  const CreateDeviceCallback&,
+                                  const CreateDeviceErrorCallback&));
+  MOCK_METHOD6(CreatePairedDevice, void(const dbus::ObjectPath&,
                                         const std::string&,
                                         const dbus::ObjectPath&,
                                         const std::string&,
-                                        const DeviceCallback&));
+                                        const CreateDeviceCallback&,
+                                        const CreateDeviceErrorCallback&));
   MOCK_METHOD3(CancelDeviceCreation, void(const dbus::ObjectPath&,
                                           const std::string&,
                                           const AdapterCallback&));
diff --git a/chromeos/dbus/mock_bluetooth_input_client.h b/chromeos/dbus/mock_bluetooth_input_client.h
index 35fe0cb..afa438c 100644
--- a/chromeos/dbus/mock_bluetooth_input_client.h
+++ b/chromeos/dbus/mock_bluetooth_input_client.h
@@ -20,8 +20,9 @@ class MockBluetoothInputClient : public BluetoothInputClient {
   MOCK_METHOD1(AddObserver, void(Observer*));
   MOCK_METHOD1(RemoveObserver, void(Observer*));
   MOCK_METHOD1(GetProperties, Properties*(const dbus::ObjectPath&));
-  MOCK_METHOD2(Connect, void(const dbus::ObjectPath&,
-                             const InputCallback&));
+  MOCK_METHOD3(Connect, void(const dbus::ObjectPath&,
+                             const ConnectCallback&,
+                             const ConnectErrorCallback&));
   MOCK_METHOD2(Disconnect, void(const dbus::ObjectPath&,
                                 const InputCallback&));
 };