chromeos/dbus: Update Bluetooth GATT API clients to upstream definition

This CL ports the GATT API bindings from the D-Bus API in our Chrome OS fork to
the API definition in doc/gatt-api.txt in BlueZ upstream.

TBR=keybuk@chromium.org
BUG=440968

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

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

chromeos/dbus/bluetooth_gatt_characteristic_client.cc

@@ -13,6 +13,13 @@
 
 namespace chromeos {
 
+namespace {
+
+// TODO(armansito): Move this constant to cros_system_api.
+const char kValueProperty[] = "Value";
+
+}  // namespace
+
 // static
 const char BluetoothGattCharacteristicClient::kNoResponseError[] =
     "org.chromium.Error.NoResponse";
@@ -27,6 +34,7 @@ BluetoothGattCharacteristicClient::Properties::Properties(
     : dbus::PropertySet(object_proxy, interface_name, callback) {
   RegisterProperty(bluetooth_gatt_characteristic::kUUIDProperty, &uuid);
   RegisterProperty(bluetooth_gatt_characteristic::kServiceProperty, &service);
+  RegisterProperty(kValueProperty, &value);
   RegisterProperty(bluetooth_gatt_characteristic::kNotifyingProperty,
                    &notifying);
   RegisterProperty(bluetooth_gatt_characteristic::kFlagsProperty, &flags);
@@ -210,21 +218,6 @@ class BluetoothGattCharacteristicClientImpl
     VLOG(2) << "Remote GATT characteristic added: " << object_path.value();
     FOR_EACH_OBSERVER(BluetoothGattCharacteristicClient::Observer, observers_,
                       GattCharacteristicAdded(object_path));
-
-    // Connect the "ValueUpdated" signal.
-    dbus::ObjectProxy* object_proxy =
-        object_manager_->GetObjectProxy(object_path);
-    DCHECK(object_proxy);
-
-    object_proxy->ConnectToSignal(
-        bluetooth_gatt_characteristic::kBluetoothGattCharacteristicInterface,
-        bluetooth_gatt_characteristic::kValueUpdatedSignal,
-        base::Bind(&BluetoothGattCharacteristicClientImpl::ValueUpdatedReceived,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   object_path),
-        base::Bind(
-            &BluetoothGattCharacteristicClientImpl::ValueUpdatedConnected,
-            weak_ptr_factory_.GetWeakPtr()));
   }
 
   // dbus::ObjectManager::Interface override.
@@ -260,35 +253,6 @@ class BluetoothGattCharacteristicClientImpl
                                                         property_name));
   }
 
-  // Called by dbus:: when a "ValueUpdated" signal is received.
-  void ValueUpdatedReceived(const dbus::ObjectPath& object_path,
-                            dbus::Signal* signal) {
-    DCHECK(signal);
-    const uint8* bytes = NULL;
-    size_t length = 0;
-    dbus::MessageReader reader(signal);
-    if (!reader.PopArrayOfBytes(&bytes, &length)) {
-      LOG(WARNING) << "ValueUpdated signal has incorrect parameters: "
-                   << signal->ToString();
-      return;
-    }
-
-    std::vector<uint8> value;
-    if (bytes)
-      value.assign(bytes, bytes + length);
-
-    FOR_EACH_OBSERVER(BluetoothGattCharacteristicClient::Observer,
-                      observers_,
-                      GattCharacteristicValueUpdated(object_path, value));
-  }
-
-  // Called by dbus:: when the "ValueUpdated" signal is initially connected.
-  void ValueUpdatedConnected(const std::string& interface_name,
-                             const std::string& signal_name,
-                             bool success) {
-    LOG_IF(WARNING, !success) << "Failed to connect to the ValueUpdated signal";
-  }
-
   // Called when a response for successful method call is received.
   void OnSuccess(const base::Closure& callback, dbus::Response* response) {
     DCHECK(response);

chromeos/dbus/bluetooth_gatt_characteristic_client.h

@@ -6,6 +6,7 @@
 #define CHROMEOS_DBUS_BLUETOOTH_GATT_CHARACTERISTIC_CLIENT_H_
 
 #include <string>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
@@ -29,6 +30,11 @@ class CHROMEOS_EXPORT BluetoothGattCharacteristicClient : public DBusClient {
     // [read-only]
     dbus::Property<dbus::ObjectPath> service;
 
+    // The cached value of the characteristic. This property gets updated only
+    // after a successful read request and when a notification or indication is
+    // received. [read-only]
+    dbus::Property<std::vector<uint8_t>> value;
+
     // Whether or not this characteristic is currently sending ValueUpdated
     // signals. [read-only]
     dbus::Property<bool> notifying;
@@ -36,11 +42,11 @@ class CHROMEOS_EXPORT BluetoothGattCharacteristicClient : public DBusClient {
     // List of flags representing the GATT "Characteristic Properties bit field"
     // and properties read from the GATT "Characteristic Extended Properties"
     // descriptor bit field. [read-only, optional]
-    dbus::Property<std::vector<std::string> > flags;
+    dbus::Property<std::vector<std::string>> flags;
 
     // Array of object paths representing the descriptors of this
     // characteristic. [read-only]
-    dbus::Property<std::vector<dbus::ObjectPath> > descriptors;
+    dbus::Property<std::vector<dbus::ObjectPath>> descriptors;
 
     Properties(dbus::ObjectProxy* object_proxy,
                const std::string& interface_name,
@@ -67,13 +73,6 @@ class CHROMEOS_EXPORT BluetoothGattCharacteristicClient : public DBusClient {
     virtual void GattCharacteristicPropertyChanged(
         const dbus::ObjectPath& object_path,
         const std::string& property_name) {}
-
-    // Called when a "ValueUpdated" signal is received from the remote GATT
-    // characteristic with object path |object_path| with characteristic value
-    // |value|.
-    virtual void GattCharacteristicValueUpdated(
-        const dbus::ObjectPath& object_path,
-        const std::vector<uint8>& value) {}
   };
 
   // Callbacks used to report the result of asynchronous methods.

chromeos/dbus/bluetooth_gatt_descriptor_client.cc

@@ -13,6 +13,13 @@
 
 namespace chromeos {
 
+namespace {
+
+// TODO(armansito): Move this constant to cros_system_api.
+const char kValueProperty[] = "Value";
+
+}  // namespace
+
 // static
 const char BluetoothGattDescriptorClient::kNoResponseError[] =
     "org.chromium.Error.NoResponse";
@@ -28,6 +35,7 @@ BluetoothGattDescriptorClient::Properties::Properties(
   RegisterProperty(bluetooth_gatt_descriptor::kUUIDProperty, &uuid);
   RegisterProperty(bluetooth_gatt_descriptor::kCharacteristicProperty,
                    &characteristic);
+  RegisterProperty(kValueProperty, &value);
 }
 
 BluetoothGattDescriptorClient::Properties::~Properties() {

chromeos/dbus/bluetooth_gatt_descriptor_client.h

@@ -6,6 +6,7 @@
 #define CHROMEOS_DBUS_BLUETOOTH_GATT_DESCRIPTOR_CLIENT_H_
 
 #include <string>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
@@ -29,6 +30,10 @@ class CHROMEOS_EXPORT BluetoothGattDescriptorClient : public DBusClient {
     // [read-only]
     dbus::Property<dbus::ObjectPath> characteristic;
 
+    // The cached value of the descriptor. This property gets updated only after
+    // a successful read request. [read-only]
+    dbus::Property<std::vector<uint8_t>> value;
+
     Properties(dbus::ObjectProxy* object_proxy,
                const std::string& interface_name,
                const PropertyChangedCallback& callback);

chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc

@@ -176,14 +176,16 @@ void FakeBluetoothGattCharacteristicClient::ReadValue(
     }
     return;
   }
+
   base::Closure completed_callback;
   if (!IsHeartRateVisible()) {
     completed_callback =
         base::Bind(error_callback, kUnknownCharacteristicError, "");
   } else {
-    std::vector<uint8> value;
-    value.push_back(0x06);  // Location is "foot".
-    completed_callback = base::Bind(callback, value);
+    std::vector<uint8> value = {0x06};  // Location is "foot".
+    completed_callback = base::Bind(
+        &FakeBluetoothGattCharacteristicClient::DelayedReadValueCallback,
+        weak_ptr_factory_.GetWeakPtr(), object_path, callback, value);
   }
 
   if (extra_requests_ > 0) {
@@ -191,6 +193,7 @@ void FakeBluetoothGattCharacteristicClient::ReadValue(
         new DelayedCallback(completed_callback, extra_requests_);
     return;
   }
+
   completed_callback.Run();
 }
 
@@ -488,12 +491,7 @@ void FakeBluetoothGattCharacteristicClient::
 
   VLOG(2) << "Updating heart rate value.";
   std::vector<uint8> measurement = GetHeartRateMeasurementValue();
-
-  FOR_EACH_OBSERVER(
-      BluetoothGattCharacteristicClient::Observer,
-      observers_,
-      GattCharacteristicValueUpdated(
-          dbus::ObjectPath(heart_rate_measurement_path_), measurement));
+  heart_rate_measurement_properties_->value.ReplaceValue(measurement);
 
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
@@ -504,6 +502,17 @@ void FakeBluetoothGattCharacteristicClient::
                      kHeartRateMeasurementNotificationIntervalMs));
 }
 
+void FakeBluetoothGattCharacteristicClient::DelayedReadValueCallback(
+    const dbus::ObjectPath& object_path,
+    const ValueCallback& callback,
+    const std::vector<uint8_t>& value) {
+  Properties* properties = GetProperties(object_path);
+  DCHECK(properties);
+
+  properties->value.ReplaceValue(value);
+  callback.Run(value);
+}
+
 std::vector<uint8>
 FakeBluetoothGattCharacteristicClient::GetHeartRateMeasurementValue() {
   // TODO(armansito): We should make sure to properly pack this struct to ensure

chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h

@@ -128,6 +128,12 @@ class CHROMEOS_EXPORT FakeBluetoothGattCharacteristicClient
   // is a random value within a reasonable range.
   std::vector<uint8> GetHeartRateMeasurementValue();
 
+  // Callback that executes a delayed ReadValue action by updating the
+  // appropriate "Value" property and invoking the ValueCallback.
+  void DelayedReadValueCallback(const dbus::ObjectPath& object_path,
+                                const ValueCallback& callback,
+                                const std::vector<uint8_t>& value);
+
   // If true, characteristics of the Heart Rate Service are visible. Use
   // IsHeartRateVisible() to check the value.
   bool heart_rate_visible_;

chromeos/dbus/fake_bluetooth_gatt_descriptor_client.cc

@@ -6,6 +6,8 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "chromeos/dbus/bluetooth_gatt_characteristic_client.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
@@ -101,7 +103,25 @@ void FakeBluetoothGattDescriptorClient::ReadValue(
     return;
   }
 
-  callback.Run(iter->second->value);
+  // Assign the value of the descriptor as necessary
+  Properties* properties = iter->second->properties.get();
+  if (properties->uuid.value() == kClientCharacteristicConfigurationUUID) {
+    BluetoothGattCharacteristicClient::Properties* chrc_props =
+        DBusThreadManager::Get()
+            ->GetBluetoothGattCharacteristicClient()
+            ->GetProperties(properties->characteristic.value());
+    DCHECK(chrc_props);
+
+    uint8_t value_byte = chrc_props->notifying.value() ? 0x01 : 0x00;
+    const std::vector<uint8_t>& cur_value = properties->value.value();
+
+    if (!cur_value.size() || cur_value[0] != value_byte) {
+      std::vector<uint8_t> value = {value_byte, 0x00};
+      properties->value.ReplaceValue(value);
+    }
+  }
+
+  callback.Run(iter->second->properties->value.value());
 }
 
 void FakeBluetoothGattDescriptorClient::WriteValue(
@@ -151,9 +171,6 @@ dbus::ObjectPath FakeBluetoothGattDescriptorClient::ExposeDescriptor(
   DescriptorData* data = new DescriptorData();
   data->properties.reset(properties);
 
-  data->value.push_back(1);  // Notifications enabled.
-  data->value.push_back(0);
-
   properties_[object_path] = data;
 
   NotifyDescriptorAdded(object_path);

chromeos/dbus/fake_bluetooth_gatt_descriptor_client.h

@@ -82,7 +82,6 @@ class CHROMEOS_EXPORT FakeBluetoothGattDescriptorClient
     ~DescriptorData();
 
     scoped_ptr<Properties> properties;
-    std::vector<uint8> value;
   };
   typedef std::map<dbus::ObjectPath, DescriptorData*> PropertiesMap;
   PropertiesMap properties_;

device/bluetooth/bluetooth_gatt_chromeos_unittest.cc

@@ -1090,15 +1090,14 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
           GetHeartRateMeasurementPath().value());
   ASSERT_TRUE(characteristic);
   EXPECT_EQ(1U, characteristic->GetDescriptors().size());
+  EXPECT_FALSE(characteristic->IsNotifying());
 
   BluetoothGattDescriptor* descriptor = characteristic->GetDescriptors()[0];
   EXPECT_FALSE(descriptor->IsLocal());
   EXPECT_EQ(BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid(),
             descriptor->GetUUID());
 
-  std::vector<uint8> desc_value;
-  desc_value.push_back(1);
-  desc_value.push_back(0);
+  std::vector<uint8_t> desc_value = {0x00, 0x00};
 
   /* The cached value will be empty until the first read request */
   EXPECT_FALSE(ValuesEqual(desc_value, descriptor->GetValue()));
@@ -1139,7 +1138,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   EXPECT_EQ(0, observer.gatt_service_changed_count_);
   EXPECT_EQ(1, observer.gatt_descriptor_value_changed_count_);
 
-  // Read new value.
+  // Read value. The value should remain unchanged.
   descriptor->ReadRemoteDescriptor(
       base::Bind(&BluetoothGattChromeOSTest::ValueCallback,
                  base::Unretained(this)),
@@ -1150,6 +1149,32 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_FALSE(ValuesEqual(desc_value, descriptor->GetValue()));
   EXPECT_EQ(0, observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, observer.gatt_descriptor_value_changed_count_);
+
+  // Start notifications on the descriptor's characteristic. The descriptor
+  // value should change.
+  characteristic->StartNotifySession(
+      base::Bind(&BluetoothGattChromeOSTest::NotifySessionCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
+                 base::Unretained(this)));
+  base::MessageLoop::current()->Run();
+  EXPECT_EQ(3, success_callback_count_);
+  EXPECT_EQ(1, error_callback_count_);
+  EXPECT_EQ(1U, update_sessions_.size());
+  EXPECT_TRUE(characteristic->IsNotifying());
+
+  // Read the new descriptor value. We should receive a value updated event.
+  descriptor->ReadRemoteDescriptor(
+      base::Bind(&BluetoothGattChromeOSTest::ValueCallback,
+                 base::Unretained(this)),
+      base::Bind(&BluetoothGattChromeOSTest::ServiceErrorCallback,
+                 base::Unretained(this)));
+  EXPECT_EQ(4, success_callback_count_);
+  EXPECT_EQ(1, error_callback_count_);
+  EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
+  EXPECT_FALSE(ValuesEqual(desc_value, descriptor->GetValue()));
+  EXPECT_EQ(0, observer.gatt_service_changed_count_);
   EXPECT_EQ(2, observer.gatt_descriptor_value_changed_count_);
 }
 

device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc

@@ -12,6 +12,7 @@
 #include "device/bluetooth/bluetooth_adapter_chromeos.h"
 #include "device/bluetooth/bluetooth_device.h"
 #include "device/bluetooth/bluetooth_gatt_notify_session_chromeos.h"
+#include "device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h"
 #include "device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.h"
 #include "device/bluetooth/bluetooth_remote_gatt_service_chromeos.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
@@ -43,8 +44,6 @@ BluetoothRemoteGattCharacteristicChromeOS::
       weak_ptr_factory_(this) {
   VLOG(1) << "Creating remote GATT characteristic with identifier: "
           << GetIdentifier() << ", UUID: " << GetUUID().canonical_value();
-  DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
-      AddObserver(this);
   DBusThreadManager::Get()->GetBluetoothGattDescriptorClient()->
       AddObserver(this);
 
@@ -61,8 +60,6 @@ BluetoothRemoteGattCharacteristicChromeOS::
     ~BluetoothRemoteGattCharacteristicChromeOS() {
   DBusThreadManager::Get()->GetBluetoothGattDescriptorClient()->
       RemoveObserver(this);
-  DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
-      RemoveObserver(this);
 
   // Clean up all the descriptors. There isn't much point in notifying service
   // observers for each descriptor that gets removed, so just delete them.
@@ -97,7 +94,14 @@ bool BluetoothRemoteGattCharacteristicChromeOS::IsLocal() const {
 
 const std::vector<uint8>&
 BluetoothRemoteGattCharacteristicChromeOS::GetValue() const {
-  return cached_value_;
+  BluetoothGattCharacteristicClient::Properties* properties =
+      DBusThreadManager::Get()
+          ->GetBluetoothGattCharacteristicClient()
+          ->GetProperties(object_path_);
+
+  DCHECK(properties);
+
+  return properties->value.value();
 }
 
 device::BluetoothGattService*
@@ -198,13 +202,9 @@ void BluetoothRemoteGattCharacteristicChromeOS::ReadRemoteCharacteristic(
           << ".";
 
   DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->ReadValue(
-      object_path_,
-      base::Bind(&BluetoothRemoteGattCharacteristicChromeOS::OnValueSuccess,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 callback),
+      object_path_, callback,
       base::Bind(&BluetoothRemoteGattCharacteristicChromeOS::OnError,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 error_callback));
+                 weak_ptr_factory_.GetWeakPtr(), error_callback));
 }
 
 void BluetoothRemoteGattCharacteristicChromeOS::WriteRemoteCharacteristic(
@@ -315,20 +315,6 @@ void BluetoothRemoteGattCharacteristicChromeOS::RemoveNotifySession(
                  callback));
 }
 
-void BluetoothRemoteGattCharacteristicChromeOS::GattCharacteristicValueUpdated(
-    const dbus::ObjectPath& object_path,
-    const std::vector<uint8>& value) {
-  if (object_path != object_path_)
-    return;
-
-  cached_value_ = value;
-
-  VLOG(1) << "GATT characteristic value has changed: " << object_path.value()
-          << ": " << value;
-  DCHECK(service_);
-  service_->NotifyCharacteristicValueChanged(this, value);
-}
-
 void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorAdded(
     const dbus::ObjectPath& object_path) {
   if (descriptors_.find(object_path) != descriptors_.end()) {
@@ -380,16 +366,28 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorRemoved(
   delete descriptor;
 }
 
-void BluetoothRemoteGattCharacteristicChromeOS::OnValueSuccess(
-    const ValueCallback& callback,
-    const std::vector<uint8>& value) {
-  VLOG(1) << "Characteristic value read: " << value;
-  cached_value_ = value;
+void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorPropertyChanged(
+    const dbus::ObjectPath& object_path,
+    const std::string& property_name) {
+  DescriptorMap::iterator iter = descriptors_.find(object_path);
+  if (iter == descriptors_.end()) {
+    VLOG(2) << "Unknown descriptor removed: " << object_path.value();
+    return;
+  }
 
-  DCHECK(service_);
-  service_->NotifyCharacteristicValueChanged(this, cached_value_);
+  BluetoothGattDescriptorClient::Properties* properties =
+      DBusThreadManager::Get()
+          ->GetBluetoothGattDescriptorClient()
+          ->GetProperties(object_path);
 
-  callback.Run(value);
+  DCHECK(properties);
+
+  if (property_name != properties->value.name())
+    return;
+
+  DCHECK(service_);
+  service_->NotifyDescriptorValueChanged(this, iter->second,
+                                         properties->value.value());
 }
 
 void BluetoothRemoteGattCharacteristicChromeOS::OnError(

device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h

@@ -12,7 +12,6 @@
 #include <vector>
 
 #include "base/memory/weak_ptr.h"
-#include "chromeos/dbus/bluetooth_gatt_characteristic_client.h"
 #include "chromeos/dbus/bluetooth_gatt_descriptor_client.h"
 #include "dbus/object_path.h"
 #include "device/bluetooth/bluetooth_gatt_characteristic.h"
@@ -35,7 +34,6 @@ class BluetoothRemoteGattServiceChromeOS;
 // platform.
 class BluetoothRemoteGattCharacteristicChromeOS
     : public device::BluetoothGattCharacteristic,
-      public BluetoothGattCharacteristicClient::Observer,
       public BluetoothGattDescriptorClient::Observer {
  public:
   // device::BluetoothGattCharacteristic overrides.
@@ -77,18 +75,11 @@ class BluetoothRemoteGattCharacteristicChromeOS
       const dbus::ObjectPath& object_path);
   ~BluetoothRemoteGattCharacteristicChromeOS() override;
 
-  // BluetoothGattCharacteristicClient::Observer overrides.
-  void GattCharacteristicValueUpdated(const dbus::ObjectPath& object_path,
-                                      const std::vector<uint8>& value) override;
-
   // BluetoothGattDescriptorClient::Observer overrides.
   void GattDescriptorAdded(const dbus::ObjectPath& object_path) override;
   void GattDescriptorRemoved(const dbus::ObjectPath& object_path) override;
-
-  // Called by dbus:: on successful completion of a request to read
-  // the characteristic value.
-  void OnValueSuccess(const ValueCallback& callback,
-                      const std::vector<uint8>& value);
+  void GattDescriptorPropertyChanged(const dbus::ObjectPath& object_path,
+                                     const std::string& property_name) override;
 
   // Called by dbus:: on unsuccessful completion of a request to read or write
   // the characteristic value.
@@ -125,10 +116,6 @@ class BluetoothRemoteGattCharacteristicChromeOS
   // The GATT service this GATT characteristic belongs to.
   BluetoothRemoteGattServiceChromeOS* service_;
 
-  // The cached characteristic value based on the most recent read or
-  // notification.
-  std::vector<uint8> cached_value_;
-
   // The total number of currently active value update sessions.
   size_t num_notify_sessions_;
 

device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.cc

@@ -60,7 +60,14 @@ bool BluetoothRemoteGattDescriptorChromeOS::IsLocal() const {
 
 const std::vector<uint8>&
 BluetoothRemoteGattDescriptorChromeOS::GetValue() const {
-  return cached_value_;
+  BluetoothGattDescriptorClient::Properties* properties =
+      DBusThreadManager::Get()
+          ->GetBluetoothGattDescriptorClient()
+          ->GetProperties(object_path_);
+
+  DCHECK(properties);
+
+  return properties->value.value();
 }
 
 device::BluetoothGattCharacteristic*
@@ -83,13 +90,9 @@ void BluetoothRemoteGattDescriptorChromeOS::ReadRemoteDescriptor(
           << GetUUID().canonical_value();
 
   DBusThreadManager::Get()->GetBluetoothGattDescriptorClient()->ReadValue(
-      object_path_,
-      base::Bind(&BluetoothRemoteGattDescriptorChromeOS::OnValueSuccess,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 callback),
+      object_path_, callback,
       base::Bind(&BluetoothRemoteGattDescriptorChromeOS::OnError,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 error_callback));
+                 weak_ptr_factory_.GetWeakPtr(), error_callback));
 }
 
 void BluetoothRemoteGattDescriptorChromeOS::WriteRemoteDescriptor(
@@ -110,21 +113,6 @@ void BluetoothRemoteGattDescriptorChromeOS::WriteRemoteDescriptor(
                  error_callback));
 }
 
-void BluetoothRemoteGattDescriptorChromeOS::OnValueSuccess(
-    const ValueCallback& callback,
-    const std::vector<uint8>& value) {
-  VLOG(1) << "Descriptor value read: " << value;
-  cached_value_ = value;
-
-  DCHECK(characteristic_);
-  BluetoothRemoteGattServiceChromeOS* service =
-      static_cast<BluetoothRemoteGattServiceChromeOS*>(
-          characteristic_->GetService());
-  DCHECK(service);
-  service->NotifyDescriptorValueChanged(characteristic_, this, value);
-  callback.Run(value);
-}
-
 void BluetoothRemoteGattDescriptorChromeOS::OnError(
     const ErrorCallback& error_callback,
     const std::string& error_name,

device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.h

@@ -54,11 +54,6 @@ class BluetoothRemoteGattDescriptorChromeOS
       const dbus::ObjectPath& object_path);
   ~BluetoothRemoteGattDescriptorChromeOS() override;
 
-  // Called by dbus:: on successful completion of a request to read
-  // the descriptor value.
-  void OnValueSuccess(const ValueCallback& callback,
-                      const std::vector<uint8>& value);
-
   // Called by dbus:: on unsuccessful completion of a request to read or write
   // the descriptor value.
   void OnError(const ErrorCallback& error_callback,
@@ -71,9 +66,6 @@ class BluetoothRemoteGattDescriptorChromeOS
   // The GATT characteristic this descriptor belongs to.
   BluetoothRemoteGattCharacteristicChromeOS* characteristic_;
 
-  // The cached characteristic value based on the most recent read request.
-  std::vector<uint8> cached_value_;
-
   // 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<BluetoothRemoteGattDescriptorChromeOS> weak_ptr_factory_;

device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc

@@ -195,14 +195,6 @@ void BluetoothRemoteGattServiceChromeOS::NotifyServiceChanged() {
   adapter_->NotifyGattServiceChanged(this);
 }
 
-void BluetoothRemoteGattServiceChromeOS::NotifyCharacteristicValueChanged(
-    BluetoothRemoteGattCharacteristicChromeOS* characteristic,
-    const std::vector<uint8>& value) {
-  DCHECK(characteristic->GetService() == this);
-  DCHECK(adapter_);
-  adapter_->NotifyGattCharacteristicValueChanged(characteristic, value);
-}
-
 void BluetoothRemoteGattServiceChromeOS::NotifyDescriptorAddedOrRemoved(
     BluetoothRemoteGattCharacteristicChromeOS* characteristic,
     BluetoothRemoteGattDescriptorChromeOS* descriptor,
@@ -311,7 +303,8 @@ void BluetoothRemoteGattServiceChromeOS::GattCharacteristicRemoved(
 void BluetoothRemoteGattServiceChromeOS::GattCharacteristicPropertyChanged(
     const dbus::ObjectPath& object_path,
     const std::string& property_name) {
-  if (characteristics_.find(object_path) == characteristics_.end()) {
+  CharacteristicMap::iterator iter = characteristics_.find(object_path);
+  if (iter == characteristics_.end()) {
     VLOG(3) << "Properties of unknown characteristic changed";
     return;
   }
@@ -324,11 +317,15 @@ void BluetoothRemoteGattServiceChromeOS::GattCharacteristicPropertyChanged(
   BluetoothGattCharacteristicClient::Properties* properties =
       DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
           GetProperties(object_path);
+
   DCHECK(properties);
-  if (property_name != properties->flags.name())
-    return;
+  DCHECK(adapter_);
 
-  NotifyServiceChanged();
+  if (property_name == properties->flags.name())
+    NotifyServiceChanged();
+  else if (property_name == properties->value.name())
+    adapter_->NotifyGattCharacteristicValueChanged(iter->second,
+                                                   properties->value.value());
 }
 
 }  // namespace chromeos

device/bluetooth/bluetooth_remote_gatt_service_chromeos.h

@@ -74,14 +74,6 @@ class BluetoothRemoteGattServiceChromeOS
   // service observers when characteristic descriptors get added and removed.
   void NotifyServiceChanged();
 
-  // Notifies its observers that the value of a characteristic has changed.
-  // Called by BluetoothRemoteGattCharacteristicChromeOS instances to notify
-  // service observers when their cached value is updated after a successful
-  // read request or when a "ValueUpdated" signal is received.
-  void NotifyCharacteristicValueChanged(
-      BluetoothRemoteGattCharacteristicChromeOS* characteristic,
-      const std::vector<uint8>& value);
-
   // Notifies its observers that a descriptor |descriptor| belonging to
   // characteristic |characteristic| has been added or removed. This is used
   // by BluetoothRemoteGattCharacteristicChromeOS instances to notify service
@@ -94,8 +86,8 @@ class BluetoothRemoteGattServiceChromeOS
       bool added);
 
   // Notifies its observers that the value of a descriptor has changed. Called
-  // by BluetoothRemoteGattDescriptorChromeOS instances to notify service
-  // observers when their cached value gets updated after a read request.
+  // by BluetoothRemoteGattCharacteristicChromeOS instances to notify service
+  // observers.
   void NotifyDescriptorValueChanged(
       BluetoothRemoteGattCharacteristicChromeOS* characteristic,
       BluetoothRemoteGattDescriptorChromeOS* descriptor,