device/bluetooth: Improvements to GATT descriptor access API.

This CL introduces the following changes to the GATT API:

* New BluetoothGattService::Observer methods:
 - GattDescriptorAdded
 - GattDescriptorRemoved
 - GattDescriptorValueChanged

* New BluetoothGattCharacteristic::GetDescriptor method to obtain a
characteristic's descriptor by descriptor identifier.

BUG=265663
TEST=device_unittests

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

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

8 files changed, 204 insertions, 1 deletions

diff --git a/device/bluetooth/bluetooth_gatt_characteristic.h b/device/bluetooth/bluetooth_gatt_characteristic.h
index 947d8ff..881511c 100644
--- a/device/bluetooth/bluetooth_gatt_characteristic.h
+++ b/device/bluetooth/bluetooth_gatt_characteristic.h
@@ -138,6 +138,11 @@ class BluetoothGattCharacteristic {
   virtual std::vector<BluetoothGattDescriptor*>
       GetDescriptors() const = 0;
 
+  // Returns the GATT characteristic descriptor with identifier |identifier| if
+  // it belongs to this GATT characteristic.
+  virtual BluetoothGattDescriptor* GetDescriptor(
+      const std::string& identifier) const = 0;
+
   // Adds a characteristic descriptor to the locally hosted characteristic
   // represented by this instance. This method only makes sense for local
   // characteristics and won't have an effect if this instance represents a
diff --git a/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc b/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
index 032ed8c..91a4446 100644
--- a/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
+++ b/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
@@ -132,6 +132,9 @@ class TestGattServiceObserver : public BluetoothGattService::Observer {
         gatt_characteristic_added_count_(0),
         gatt_characteristic_removed_count_(0),
         gatt_characteristic_value_changed_count_(0),
+        gatt_descriptor_added_count_(0),
+        gatt_descriptor_removed_count_(0),
+        gatt_descriptor_value_changed_count_(0),
         device_address_(device->GetAddress()),
         gatt_service_id_(service->GetIdentifier()),
         adapter_(adapter) {
@@ -200,10 +203,61 @@ class TestGattServiceObserver : public BluetoothGattService::Observer {
     ++gatt_characteristic_value_changed_count_;
     last_gatt_characteristic_id_ = characteristic->GetIdentifier();
     last_gatt_characteristic_uuid_ = characteristic->GetUUID();
-    last_changed_characteristic_value_ = characteristic->GetValue();
+    last_changed_characteristic_value_ = value;
 
     EXPECT_EQ(service->GetCharacteristic(last_gatt_characteristic_id_),
               characteristic);
+    EXPECT_EQ(service, characteristic->GetService());
+
+    QuitMessageLoop();
+  }
+
+  virtual void GattDescriptorAdded(
+      BluetoothGattCharacteristic* characteristic,
+      BluetoothGattDescriptor* descriptor) OVERRIDE {
+    ASSERT_EQ(gatt_service_id_, characteristic->GetService()->GetIdentifier());
+
+    ++gatt_descriptor_added_count_;
+    last_gatt_descriptor_id_ = descriptor->GetIdentifier();
+    last_gatt_descriptor_uuid_ = descriptor->GetUUID();
+
+    EXPECT_EQ(characteristic->GetDescriptor(last_gatt_descriptor_id_),
+              descriptor);
+    EXPECT_EQ(characteristic, descriptor->GetCharacteristic());
+
+    QuitMessageLoop();
+  }
+
+  virtual void GattDescriptorRemoved(
+      BluetoothGattCharacteristic* characteristic,
+      BluetoothGattDescriptor* descriptor) OVERRIDE {
+    ASSERT_EQ(gatt_service_id_, characteristic->GetService()->GetIdentifier());
+
+    ++gatt_descriptor_removed_count_;
+    last_gatt_descriptor_id_ = descriptor->GetIdentifier();
+    last_gatt_descriptor_uuid_ = descriptor->GetUUID();
+
+    // The characteristic should return NULL for this descriptor..
+    EXPECT_FALSE(characteristic->GetDescriptor(last_gatt_descriptor_id_));
+    EXPECT_EQ(characteristic, descriptor->GetCharacteristic());
+
+    QuitMessageLoop();
+  }
+
+  virtual void GattDescriptorValueChanged(
+      BluetoothGattCharacteristic* characteristic,
+      BluetoothGattDescriptor* descriptor,
+      const std::vector<uint8>& value) OVERRIDE {
+    ASSERT_EQ(gatt_service_id_, characteristic->GetService()->GetIdentifier());
+
+    ++gatt_descriptor_value_changed_count_;
+    last_gatt_descriptor_id_ = descriptor->GetIdentifier();
+    last_gatt_descriptor_uuid_ = descriptor->GetUUID();
+    last_changed_descriptor_value_ = value;
+
+    EXPECT_EQ(characteristic->GetDescriptor(last_gatt_descriptor_id_),
+              descriptor);
+    EXPECT_EQ(characteristic, descriptor->GetCharacteristic());
 
     QuitMessageLoop();
   }
@@ -212,9 +266,15 @@ class TestGattServiceObserver : public BluetoothGattService::Observer {
   int gatt_characteristic_added_count_;
   int gatt_characteristic_removed_count_;
   int gatt_characteristic_value_changed_count_;
+  int gatt_descriptor_added_count_;
+  int gatt_descriptor_removed_count_;
+  int gatt_descriptor_value_changed_count_;
   std::string last_gatt_characteristic_id_;
   BluetoothUUID last_gatt_characteristic_uuid_;
   std::vector<uint8> last_changed_characteristic_value_;
+  std::string last_gatt_descriptor_id_;
+  BluetoothUUID last_gatt_descriptor_uuid_;
+  std::vector<uint8> last_changed_descriptor_value_;
 
  private:
   // Some tests use a message loop since background processing is simulated;
@@ -501,6 +561,10 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorAddedAndRemoved) {
 
   TestGattServiceObserver service_observer(adapter_, device, service);
   EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_descriptor_added_count_);
+  EXPECT_EQ(0, service_observer.gatt_descriptor_removed_count_);
+  EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
+
   EXPECT_TRUE(service->GetCharacteristics().empty());
 
   // Run the message loop so that the characteristics appear.
@@ -508,6 +572,10 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorAddedAndRemoved) {
   EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
 
   // Only the Heart Rate Measurement characteristic has a descriptor.
+  EXPECT_EQ(1, service_observer.gatt_descriptor_added_count_);
+  EXPECT_EQ(0, service_observer.gatt_descriptor_removed_count_);
+  EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
+
   BluetoothGattCharacteristic* characteristic = service->GetCharacteristic(
       fake_bluetooth_gatt_characteristic_client_->
           GetBodySensorLocationPath().value());
@@ -530,25 +598,40 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorAddedAndRemoved) {
   EXPECT_FALSE(descriptor->IsLocal());
   EXPECT_EQ(BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid(),
             descriptor->GetUUID());
+  EXPECT_EQ(descriptor->GetUUID(),
+            service_observer.last_gatt_descriptor_uuid_);
+  EXPECT_EQ(descriptor->GetIdentifier(),
+            service_observer.last_gatt_descriptor_id_);
 
   // Hide the descriptor.
   fake_bluetooth_gatt_descriptor_client_->HideDescriptor(
       dbus::ObjectPath(descriptor->GetIdentifier()));
   EXPECT_TRUE(characteristic->GetDescriptors().empty());
   EXPECT_EQ(5, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, service_observer.gatt_descriptor_added_count_);
+  EXPECT_EQ(1, service_observer.gatt_descriptor_removed_count_);
+  EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
 
   // Expose the descriptor again.
+  service_observer.last_gatt_descriptor_id_.clear();
+  service_observer.last_gatt_descriptor_uuid_ = BluetoothUUID();
   fake_bluetooth_gatt_descriptor_client_->ExposeDescriptor(
       dbus::ObjectPath(characteristic->GetIdentifier()),
       FakeBluetoothGattDescriptorClient::
           kClientCharacteristicConfigurationUUID);
   EXPECT_EQ(6, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(1U, characteristic->GetDescriptors().size());
+  EXPECT_EQ(2, service_observer.gatt_descriptor_added_count_);
+  EXPECT_EQ(1, service_observer.gatt_descriptor_removed_count_);
+  EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
 
   descriptor = characteristic->GetDescriptors()[0];
   EXPECT_FALSE(descriptor->IsLocal());
   EXPECT_EQ(BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid(),
             descriptor->GetUUID());
+  EXPECT_EQ(descriptor->GetUUID(), service_observer.last_gatt_descriptor_uuid_);
+  EXPECT_EQ(descriptor->GetIdentifier(),
+            service_observer.last_gatt_descriptor_id_);
 }
 
 TEST_F(BluetoothGattChromeOSTest, AdapterAddedAfterGattService) {
@@ -798,6 +881,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
 
   TestGattServiceObserver service_observer(adapter_, device, service);
   EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
   EXPECT_TRUE(service->GetCharacteristics().empty());
 
   // Run the message loop so that the characteristics appear.
@@ -834,6 +918,8 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
+  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
 
   // Write value.
   desc_value[0] = 0x03;
@@ -847,6 +933,8 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_FALSE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_TRUE(ValuesEqual(desc_value, descriptor->GetValue()));
+  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, service_observer.gatt_descriptor_value_changed_count_);
 
   // Read new value.
   descriptor->ReadRemoteDescriptor(
@@ -858,6 +946,8 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_TRUE(ValuesEqual(desc_value, descriptor->GetValue()));
+  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, service_observer.gatt_descriptor_value_changed_count_);
 }
 
 }  // namespace chromeos
diff --git a/device/bluetooth/bluetooth_gatt_service.h b/device/bluetooth/bluetooth_gatt_service.h
index 346772e..16f4ae5 100644
--- a/device/bluetooth/bluetooth_gatt_service.h
+++ b/device/bluetooth/bluetooth_gatt_service.h
@@ -173,6 +173,28 @@ class BluetoothGattService {
         BluetoothGattService* service,
         BluetoothGattCharacteristic* characteristic) {}
 
+    // Called when the remote GATT characteristic descriptor |descriptor|
+    // belonging to characteristic |characteristic| has been discovered. Don't
+    // cache the arguments as the pointers may become invalid. Instead, use the
+    // specially assigned identifier to obtain a descriptor and cache that
+    // identifier as necessary.
+    //
+    // This method will always be followed by a call to GattServiceChanged,
+    // which can be used by observers to get all the characteristics of a
+    // service and perform the necessary updates. GattDescriptorAdded exists
+    // mostly for convenience.
+    virtual void GattDescriptorAdded(
+        BluetoothGattCharacteristic* characteristic,
+        BluetoothGattDescriptor* descriptor) {}
+
+    // Called when a GATT characteristic descriptor |descriptor| belonging to
+    // characteristic |characteristic| has been removed. This method is for
+    // convenience and will be followed by a call to GattServiceChanged (except
+    // when called after the service gets removed).
+    virtual void GattDescriptorRemoved(
+        BluetoothGattCharacteristic* characteristic,
+        BluetoothGattDescriptor* descriptor) {}
+
     // Called when the value of a characteristic has changed. This might be a
     // result of a read/write request to, or a notification/indication from, a
     // remote GATT characteristic.
@@ -180,6 +202,12 @@ class BluetoothGattService {
         BluetoothGattService* service,
         BluetoothGattCharacteristic* characteristic,
         const std::vector<uint8>& value) {}
+
+    // Called when the value of a characteristic descriptor has been updated.
+    virtual void GattDescriptorValueChanged(
+        BluetoothGattCharacteristic* characteristic,
+        BluetoothGattDescriptor* descriptor,
+        const std::vector<uint8>& value) {}
   };
 
   // The ErrorCallback is used by methods to asynchronously report errors.
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
index a2b28a1..bb175e3 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
@@ -114,6 +114,16 @@ BluetoothRemoteGattCharacteristicChromeOS::GetDescriptors() const {
   return descriptors;
 }
 
+device::BluetoothGattDescriptor*
+BluetoothRemoteGattCharacteristicChromeOS::GetDescriptor(
+    const std::string& identifier) const {
+  DescriptorMap::const_iterator iter =
+      descriptors_.find(dbus::ObjectPath(identifier));
+  if (iter == descriptors_.end())
+    return NULL;
+  return iter->second;
+}
+
 bool BluetoothRemoteGattCharacteristicChromeOS::AddDescriptor(
     device::BluetoothGattDescriptor* descriptor) {
   VLOG(1) << "Descriptors cannot be added to a remote GATT characteristic.";
@@ -194,6 +204,8 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorAdded(
   DCHECK(descriptor->GetIdentifier() == object_path.value());
   DCHECK(descriptor->GetUUID().IsValid());
   DCHECK(service_);
+
+  service_->NotifyDescriptorAddedOrRemoved(this, descriptor, true /* added */);
   service_->NotifyServiceChanged();
 }
 
@@ -211,9 +223,12 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorRemoved(
   BluetoothRemoteGattDescriptorChromeOS* descriptor = iter->second;
   DCHECK(descriptor->object_path() == object_path);
   descriptors_.erase(iter);
+
+  service_->NotifyDescriptorAddedOrRemoved(this, descriptor, false /* added */);
   delete descriptor;
 
   DCHECK(service_);
+
   service_->NotifyServiceChanged();
 }
 
@@ -224,8 +239,21 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorPropertyChanged(
   if (iter == descriptors_.end())
     return;
 
+  // Ignore all property changes except for "Value".
+  BluetoothGattDescriptorClient::Properties* properties =
+      DBusThreadManager::Get()->GetBluetoothGattDescriptorClient()->
+          GetProperties(object_path);
+  DCHECK(properties);
+  if (property_name != properties->value.name())
+    return;
+
   VLOG(1) << "GATT descriptor property changed: " << object_path.value()
           << ", property: " << property_name;
+
+  DCHECK(service_);
+
+  service_->NotifyDescriptorValueChanged(
+      this, iter->second, properties->value.value());
 }
 
 void BluetoothRemoteGattCharacteristicChromeOS::OnGetValue(
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h
index 1a4f045..849655a 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h
@@ -44,6 +44,8 @@ class BluetoothRemoteGattCharacteristicChromeOS
   virtual Permissions GetPermissions() const OVERRIDE;
   virtual std::vector<device::BluetoothGattDescriptor*>
       GetDescriptors() const OVERRIDE;
+  virtual device::BluetoothGattDescriptor* GetDescriptor(
+      const std::string& identifier) const OVERRIDE;
   virtual bool AddDescriptor(
       device::BluetoothGattDescriptor* descriptor) OVERRIDE;
   virtual bool UpdateValue(const std::vector<uint8>& value) OVERRIDE;
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
index c2a5dbf..f2ec861 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
@@ -10,6 +10,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "device/bluetooth/bluetooth_device_chromeos.h"
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h"
+#include "device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.h"
 
 namespace chromeos {
 
@@ -164,6 +165,32 @@ void BluetoothRemoteGattServiceChromeOS::NotifyServiceChanged() {
                     GattServiceChanged(this));
 }
 
+void BluetoothRemoteGattServiceChromeOS::NotifyDescriptorAddedOrRemoved(
+    BluetoothRemoteGattCharacteristicChromeOS* characteristic,
+    BluetoothRemoteGattDescriptorChromeOS* descriptor,
+    bool added) {
+  DCHECK(characteristic->GetService() == this);
+  DCHECK(descriptor->GetCharacteristic() == characteristic);
+  if (added) {
+    FOR_EACH_OBSERVER(device::BluetoothGattService::Observer, observers_,
+                      GattDescriptorAdded(characteristic, descriptor));
+    return;
+  }
+  FOR_EACH_OBSERVER(device::BluetoothGattService::Observer, observers_,
+                    GattDescriptorRemoved(characteristic, descriptor));
+}
+
+void BluetoothRemoteGattServiceChromeOS::NotifyDescriptorValueChanged(
+    BluetoothRemoteGattCharacteristicChromeOS* characteristic,
+    BluetoothRemoteGattDescriptorChromeOS* descriptor,
+    const std::vector<uint8>& value) {
+  DCHECK(characteristic->GetService() == this);
+  DCHECK(descriptor->GetCharacteristic() == characteristic);
+  FOR_EACH_OBSERVER(
+      device::BluetoothGattService::Observer, observers_,
+      GattDescriptorValueChanged(characteristic, descriptor, value));
+}
+
 void BluetoothRemoteGattServiceChromeOS::GattServicePropertyChanged(
     const dbus::ObjectPath& object_path,
     const std::string& property_name){
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
index b781ff2a..f86c2ac 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
+++ b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
@@ -27,6 +27,7 @@ namespace chromeos {
 
 class BluetoothDeviceChromeOS;
 class BluetoothRemoteGattCharacteristicChromeOS;
+class BluetoothRemoteGattDescriptorChromeOS;
 
 // The BluetoothRemoteGattServiceChromeOS class implements BluetootGattService
 // for remote GATT services on the the Chrome OS platform.
@@ -68,6 +69,25 @@ class BluetoothRemoteGattServiceChromeOS
   // service observers when characteristic descriptors get added and removed.
   void NotifyServiceChanged();
 
+  // 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
+  // observers when characteristic descriptors get added and removed. If |added|
+  // is true, an "Added" event will be sent. Otherwise, a "Removed" event will
+  // be sent.
+  void NotifyDescriptorAddedOrRemoved(
+      BluetoothRemoteGattCharacteristicChromeOS* characteristic,
+      BluetoothRemoteGattDescriptorChromeOS* descriptor,
+      bool added);
+
+  // Notifies its observers that the value of a descriptor has changed. Called
+  // by BluetoothRemoteGattCharacteristicChromeOS instances to notify service
+  // observers when the value of one of their descriptors gets updated.
+  void NotifyDescriptorValueChanged(
+      BluetoothRemoteGattCharacteristicChromeOS* characteristic,
+      BluetoothRemoteGattDescriptorChromeOS* descriptor,
+      const std::vector<uint8>& value);
+
  private:
   friend class BluetoothDeviceChromeOS;
 
diff --git a/device/bluetooth/test/mock_bluetooth_gatt_characteristic.h b/device/bluetooth/test/mock_bluetooth_gatt_characteristic.h
index 6619543..85323f4 100644
--- a/device/bluetooth/test/mock_bluetooth_gatt_characteristic.h
+++ b/device/bluetooth/test/mock_bluetooth_gatt_characteristic.h
@@ -6,6 +6,7 @@
 #define DEVICE_BLUETOOTH_TEST_MOCK_BLUETOOTH_GATT_CHARACTERISTIC_H_
 
 #include <string>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
@@ -37,6 +38,8 @@ class MockBluetoothGattCharacteristic : public BluetoothGattCharacteristic {
   MOCK_CONST_METHOD0(GetProperties, Properties());
   MOCK_CONST_METHOD0(GetPermissions, Permissions());
   MOCK_CONST_METHOD0(GetDescriptors, std::vector<BluetoothGattDescriptor*>());
+  MOCK_CONST_METHOD1(GetDescriptor,
+                     BluetoothGattDescriptor*(const std::string&));
   MOCK_METHOD1(AddDescriptor, bool(BluetoothGattDescriptor*));
   MOCK_METHOD1(UpdateValue, bool(const std::vector<uint8>&));
   MOCK_METHOD2(ReadRemoteCharacteristic,