First part of errors coming up from the dbus API

BUG=377232

Review URL: https://codereview.chromium.org/604023004

Cr-Commit-Position: refs/heads/master@{#297656}

8 files changed, 135 insertions, 26 deletions

diff --git a/device/bluetooth/bluetooth_gatt_characteristic.h b/device/bluetooth/bluetooth_gatt_characteristic.h
index 7b952e0..d6140a3 100644
--- a/device/bluetooth/bluetooth_gatt_characteristic.h
+++ b/device/bluetooth/bluetooth_gatt_characteristic.h
@@ -11,12 +11,12 @@
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
+#include "device/bluetooth/bluetooth_gatt_service.h"
 #include "device/bluetooth/bluetooth_uuid.h"
 
 namespace device {
 
 class BluetoothGattDescriptor;
-class BluetoothGattService;
 class BluetoothGattNotifySession;
 
 // BluetoothGattCharacteristic represents a local or remote GATT characteristic.
@@ -33,6 +33,9 @@ class BluetoothGattNotifySession;
 //      BluetoothGattService instance that represents a local service.
 class BluetoothGattCharacteristic {
  public:
+  // TODO(jamuraa): per chromium.org/developers/coding-style these should
+  // be MACRO_STYLE instead of kCamelCase. (crbug.com/418696)
+
   // Values representing the possible properties of a characteristic, which
   // define how the characteristic can be used. Each of these properties serve
   // a role as defined in the Bluetooth Specification.
@@ -78,7 +81,8 @@ class BluetoothGattCharacteristic {
   typedef uint32 Permissions;
 
   // The ErrorCallback is used by methods to asynchronously report errors.
-  typedef base::Closure ErrorCallback;
+  typedef base::Callback<void(BluetoothGattService::GattErrorCode)>
+      ErrorCallback;
 
   // The ValueCallback is used to return the value of a remote characteristic
   // upon a read request.
diff --git a/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc b/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
index 8022842..2947f16 100644
--- a/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
+++ b/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
@@ -359,6 +359,11 @@ class BluetoothGattChromeOSTest : public testing::Test {
     QuitMessageLoop();
   }
 
+  void ServiceErrorCallback(BluetoothGattService::GattErrorCode err) {
+    ++error_callback_count_;
+    last_service_error_ = err;
+  }
+
   void ErrorCallback() {
     ++error_callback_count_;
   }
@@ -393,6 +398,7 @@ class BluetoothGattChromeOSTest : public testing::Test {
   int success_callback_count_;
   int error_callback_count_;
   std::vector<uint8> last_read_value_;
+  enum BluetoothGattService::GattErrorCode last_service_error_;
 };
 
 TEST_F(BluetoothGattChromeOSTest, GattConnection) {
@@ -840,12 +846,14 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicValue) {
       write_value,
       base::Bind(&BluetoothGattChromeOSTest::SuccessCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_TRUE(observer.last_gatt_characteristic_id_.empty());
   EXPECT_FALSE(observer.last_gatt_characteristic_uuid_.IsValid());
   EXPECT_EQ(0, success_callback_count_);
   EXPECT_EQ(1, error_callback_count_);
+  EXPECT_EQ(BluetoothGattService::GattErrorCode::GATT_ERROR_NOT_PERMITTED,
+            last_service_error_);
   EXPECT_EQ(0, observer.gatt_characteristic_value_changed_count_);
 
   characteristic = service->GetCharacteristic(
@@ -860,12 +868,14 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicValue) {
       write_value,
       base::Bind(&BluetoothGattChromeOSTest::SuccessCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_TRUE(observer.last_gatt_characteristic_id_.empty());
   EXPECT_FALSE(observer.last_gatt_characteristic_uuid_.IsValid());
   EXPECT_EQ(0, success_callback_count_);
   EXPECT_EQ(2, error_callback_count_);
+  EXPECT_EQ(BluetoothGattService::GattErrorCode::GATT_ERROR_NOT_PERMITTED,
+            last_service_error_);
   EXPECT_EQ(0, observer.gatt_characteristic_value_changed_count_);
 
   // Issue write request to writeable characteristic. The "Body Sensor Location"
@@ -884,7 +894,7 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicValue) {
       write_value,
       base::Bind(&BluetoothGattChromeOSTest::SuccessCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_TRUE(observer.last_gatt_characteristic_id_.empty());
   EXPECT_FALSE(observer.last_gatt_characteristic_uuid_.IsValid());
@@ -892,6 +902,38 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicValue) {
   EXPECT_EQ(2, error_callback_count_);
   EXPECT_EQ(0, observer.gatt_characteristic_value_changed_count_);
 
+  // Issue some invalid write requests to the characteristic.
+  // The value should still not change.
+
+  std::vector<uint8> invalid_write_length;
+  invalid_write_length.push_back(0x01);
+  invalid_write_length.push_back(0x00);
+  characteristic->WriteRemoteCharacteristic(
+      invalid_write_length,
+      base::Bind(&BluetoothGattChromeOSTest::SuccessCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
+                 base::Unretained(this)));
+  EXPECT_EQ(1, success_callback_count_);
+  EXPECT_EQ(3, error_callback_count_);
+  EXPECT_EQ(BluetoothGattService::GattErrorCode::GATT_ERROR_INVALID_LENGTH,
+            last_service_error_);
+  EXPECT_EQ(0, observer.gatt_characteristic_value_changed_count_);
+
+  std::vector<uint8> invalid_write_value;
+  invalid_write_value.push_back(0x02);
+  characteristic->WriteRemoteCharacteristic(
+      invalid_write_value,
+      base::Bind(&BluetoothGattChromeOSTest::SuccessCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
+                 base::Unretained(this)));
+  EXPECT_EQ(1, success_callback_count_);
+  EXPECT_EQ(4, error_callback_count_);
+  EXPECT_EQ(BluetoothGattService::GattErrorCode::GATT_ERROR_FAILED,
+            last_service_error_);
+  EXPECT_EQ(0, observer.gatt_characteristic_value_changed_count_);
+
   // Issue a read request. A successful read results in a
   // CharacteristicValueChanged notification.
   characteristic = service->GetCharacteristic(
@@ -905,10 +947,10 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicValue) {
   characteristic->ReadRemoteCharacteristic(
       base::Bind(&BluetoothGattChromeOSTest::ValueCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(2, success_callback_count_);
-  EXPECT_EQ(2, error_callback_count_);
+  EXPECT_EQ(4, error_callback_count_);
   EXPECT_EQ(1, observer.gatt_characteristic_value_changed_count_);
   EXPECT_TRUE(ValuesEqual(characteristic->GetValue(), last_read_value_));
 }
@@ -1013,7 +1055,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   descriptor->ReadRemoteDescriptor(
       base::Bind(&BluetoothGattChromeOSTest::ValueCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(0, error_callback_count_);
@@ -1028,10 +1070,12 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
       desc_value,
       base::Bind(&BluetoothGattChromeOSTest::SuccessCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(1, error_callback_count_);
+  EXPECT_EQ(BluetoothGattService::GattErrorCode::GATT_ERROR_NOT_PERMITTED,
+            last_service_error_);
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_FALSE(ValuesEqual(desc_value, descriptor->GetValue()));
   EXPECT_EQ(0, observer.gatt_service_changed_count_);
@@ -1041,7 +1085,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   descriptor->ReadRemoteDescriptor(
       base::Bind(&BluetoothGattChromeOSTest::ValueCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(2, success_callback_count_);
   EXPECT_EQ(1, error_callback_count_);
@@ -1086,7 +1130,7 @@ TEST_F(BluetoothGattChromeOSTest, NotifySessions) {
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
 
   // The operation still hasn't completed but we should have received the first
@@ -1100,12 +1144,12 @@ TEST_F(BluetoothGattChromeOSTest, NotifySessions) {
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(0, success_callback_count_);
   EXPECT_EQ(0, error_callback_count_);
@@ -1158,7 +1202,7 @@ TEST_F(BluetoothGattChromeOSTest, NotifySessions) {
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(0, success_callback_count_);
   EXPECT_EQ(0, error_callback_count_);
@@ -1184,7 +1228,7 @@ TEST_F(BluetoothGattChromeOSTest, NotifySessions) {
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   EXPECT_EQ(2, success_callback_count_);
   EXPECT_EQ(0, error_callback_count_);
@@ -1235,22 +1279,22 @@ TEST_F(BluetoothGattChromeOSTest, NotifySessionsMadeInactive) {
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
 
   // The operation still hasn't completed but we should have received the first
@@ -1298,7 +1342,7 @@ TEST_F(BluetoothGattChromeOSTest, NotifySessionsMadeInactive) {
   characteristic->StartNotifySession(
       base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
                  base::Unretained(this)),
-      base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
                  base::Unretained(this)));
 
   EXPECT_EQ(0, success_callback_count_);
diff --git a/device/bluetooth/bluetooth_gatt_descriptor.h b/device/bluetooth/bluetooth_gatt_descriptor.h
index e98ef29..84139b8 100644
--- a/device/bluetooth/bluetooth_gatt_descriptor.h
+++ b/device/bluetooth/bluetooth_gatt_descriptor.h
@@ -105,7 +105,8 @@ class BluetoothGattDescriptor {
   static const BluetoothUUID& CharacteristicAggregateFormatUuid();
 
   // The ErrorCallback is used by methods to asynchronously report errors.
-  typedef base::Closure ErrorCallback;
+  typedef base::Callback<void(BluetoothGattService::GattErrorCode)>
+      ErrorCallback;
 
   // The ValueCallback is used to return the value of a remote characteristic
   // descriptor upon a read request.
diff --git a/device/bluetooth/bluetooth_gatt_service.h b/device/bluetooth/bluetooth_gatt_service.h
index a62278f..992874a 100644
--- a/device/bluetooth/bluetooth_gatt_service.h
+++ b/device/bluetooth/bluetooth_gatt_service.h
@@ -125,6 +125,19 @@ class BluetoothGattService {
         const ErrorCallback& error_callback) = 0;
   };
 
+  // Interacting with Characteristics and Descriptors can produce
+  // this set of errors.
+  enum GattErrorCode {
+    GATT_ERROR_UNKNOWN = 0,
+    GATT_ERROR_FAILED,
+    GATT_ERROR_IN_PROGRESS,
+    GATT_ERROR_INVALID_LENGTH,
+    GATT_ERROR_NOT_PERMITTED,
+    GATT_ERROR_NOT_AUTHORIZED,
+    GATT_ERROR_NOT_PAIRED,
+    GATT_ERROR_NOT_SUPPORTED
+  };
+
   // The ErrorCallback is used by methods to asynchronously report errors.
   typedef base::Closure ErrorCallback;
 
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
index 1c54a17..f477fa7 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
@@ -74,7 +74,7 @@ BluetoothRemoteGattCharacteristicChromeOS::
   while (!pending_start_notify_calls_.empty()) {
     PendingStartNotifyCall callbacks = pending_start_notify_calls_.front();
     pending_start_notify_calls_.pop();
-    callbacks.second.Run();
+    callbacks.second.Run(device::BluetoothGattService::GATT_ERROR_FAILED);
   }
 }
 
@@ -238,7 +238,7 @@ void BluetoothRemoteGattCharacteristicChromeOS::StartNotifySession(
     if (IsNotifying()) {
       // Check for overflows, though unlikely.
       if (num_notify_sessions_ == std::numeric_limits<size_t>::max()) {
-        error_callback.Run();
+        error_callback.Run(device::BluetoothGattService::GATT_ERROR_FAILED);
         return;
       }
 
@@ -398,7 +398,8 @@ void BluetoothRemoteGattCharacteristicChromeOS::OnError(
     const std::string& error_message) {
   VLOG(1) << "Operation failed: " << error_name << ", message: "
           << error_message;
-  error_callback.Run();
+  error_callback.Run(
+      BluetoothRemoteGattServiceChromeOS::DBusErrorToServiceError(error_name));
 }
 
 void BluetoothRemoteGattCharacteristicChromeOS::OnStartNotifySuccess(
@@ -437,7 +438,9 @@ void BluetoothRemoteGattCharacteristicChromeOS::OnStartNotifyError(
   DCHECK(notify_call_pending_);
 
   notify_call_pending_ = false;
-  error_callback.Run();
+
+  error_callback.Run(
+      BluetoothRemoteGattServiceChromeOS::DBusErrorToServiceError(error_name));
 
   ProcessStartNotifyQueue();
 }
diff --git a/device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.cc b/device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.cc
index 8978cc5..c4f19a8 100644
--- a/device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.cc
@@ -131,7 +131,9 @@ void BluetoothRemoteGattDescriptorChromeOS::OnError(
     const std::string& error_message) {
   VLOG(1) << "Operation failed: " << error_name
           << ", message: " << error_message;
-  error_callback.Run();
+
+  error_callback.Run(
+      BluetoothRemoteGattServiceChromeOS::DBusErrorToServiceError(error_name));
 }
 
 }  // namespace chromeos
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
index b921242..8657abd 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
@@ -15,6 +15,20 @@
 
 namespace chromeos {
 
+namespace {
+
+// TODO(jamuraa) move these to cros_system_api later
+const char kErrorFailed[] = "org.bluez.Error.Failed";
+const char kErrorInProgress[] = "org.bluez.Error.InProgress";
+const char kErrorInvalidValueLength[] = "org.bluez.Error.InvalidValueLength";
+const char kErrorNotAuthorized[] = "org.bluez.Error.NotAuthorized";
+const char kErrorNotPaired[] = "org.bluez.Error.NotPaired";
+const char kErrorNotSupported[] = "org.bluez.Error.NotSupported";
+const char kErrorReadNotPermitted[] = "org.bluez.Error.ReadNotPermitted";
+const char kErrorWriteNotPermitted[] = "org.bluez.Error.WriteNotPermitted";
+
+}  // namespace
+
 BluetoothRemoteGattServiceChromeOS::BluetoothRemoteGattServiceChromeOS(
     BluetoothAdapterChromeOS* adapter,
     BluetoothDeviceChromeOS* device,
@@ -141,6 +155,30 @@ void BluetoothRemoteGattServiceChromeOS::Unregister(
   error_callback.Run();
 }
 
+// static
+device::BluetoothGattService::GattErrorCode
+BluetoothRemoteGattServiceChromeOS::DBusErrorToServiceError(
+    std::string error_name) {
+  device::BluetoothGattService::GattErrorCode code = GATT_ERROR_UNKNOWN;
+  if (error_name == kErrorFailed) {
+    code = GATT_ERROR_FAILED;
+  } else if (error_name == kErrorInProgress) {
+    code = GATT_ERROR_IN_PROGRESS;
+  } else if (error_name == kErrorInvalidValueLength) {
+    code = GATT_ERROR_INVALID_LENGTH;
+  } else if (error_name == kErrorReadNotPermitted ||
+             error_name == kErrorWriteNotPermitted) {
+    code = GATT_ERROR_NOT_PERMITTED;
+  } else if (error_name == kErrorNotAuthorized) {
+    code = GATT_ERROR_NOT_AUTHORIZED;
+  } else if (error_name == kErrorNotPaired) {
+    code = GATT_ERROR_NOT_PAIRED;
+  } else if (error_name == kErrorNotSupported) {
+    code = GATT_ERROR_NOT_SUPPORTED;
+  }
+  return code;
+}
+
 BluetoothAdapterChromeOS*
 BluetoothRemoteGattServiceChromeOS::GetAdapter() const {
   return adapter_;
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
index 54bde89..e02b56c 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
+++ b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
@@ -63,6 +63,10 @@ class BluetoothRemoteGattServiceChromeOS
   // Object path of the underlying service.
   const dbus::ObjectPath& object_path() const { return object_path_; }
 
+  // Parses a named D-Bus error into a service error code.
+  static device::BluetoothGattService::GattErrorCode DBusErrorToServiceError(
+      const std::string error_name);
+
   // Returns the adapter associated with this service.
   BluetoothAdapterChromeOS* GetAdapter() const;