bluetooth: Refactor GetDescriptorForUUID to GetDescriptorsForUUID.

Follow up work from "bluetooth: android: Confirm the notify session after
the descriptor has been written." https://crrev.com/1712593002.

Support multiple descriptors with the same UUID.

BUG=584369, 576900

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

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

8 files changed, 102 insertions, 65 deletions

diff --git a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java
index 6d19c1f..7d6c50f 100644
--- a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java
+++ b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothRemoteGattCharacteristic.java
@@ -148,10 +148,15 @@ final class ChromeBluetoothRemoteGattCharacteristic {
     private void createDescriptors() {
         List<Wrappers.BluetoothGattDescriptorWrapper> descriptors =
                 mCharacteristic.getDescriptors();
+        // descriptorInstanceId ensures duplicate UUIDs have unique instance
+        // IDs. BluetoothGattDescriptor does not offer getInstanceId the way
+        // BluetoothGattCharacteristic does.
+        //
+        // TODO(crbug.com/576906) Do not reuse IDs upon onServicesDiscovered.
+        int instanceIdCounter = 0;
         for (Wrappers.BluetoothGattDescriptorWrapper descriptor : descriptors) {
-            // Create an adapter unique descriptor ID.
-            // TODO(crbug.com/576900) Unique descriptorInstanceId duplicate UUID values.
-            String descriptorInstanceId = mInstanceId + "/" + descriptor.getUuid().toString();
+            String descriptorInstanceId =
+                    mInstanceId + "/" + descriptor.getUuid().toString() + ";" + instanceIdCounter++;
             nativeCreateGattRemoteDescriptor(mNativeBluetoothRemoteGattCharacteristicAndroid,
                     descriptorInstanceId, descriptor, mChromeDevice);
         }
diff --git a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/Wrappers.java b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/Wrappers.java
index b61c7cd..2cc705d 100644
--- a/device/bluetooth/android/java/src/org/chromium/device/bluetooth/Wrappers.java
+++ b/device/bluetooth/android/java/src/org/chromium/device/bluetooth/Wrappers.java
@@ -517,22 +517,6 @@ class Wrappers {
             mDeviceWrapper = deviceWrapper;
         }
 
-        public BluetoothGattDescriptorWrapper getDescriptor(UUID uuid) {
-            BluetoothGattDescriptor descriptor = mCharacteristic.getDescriptor(uuid);
-            if (descriptor == null) {
-                return null;
-            }
-
-            BluetoothGattDescriptorWrapper descriptorWrapper =
-                    mDeviceWrapper.mDescriptorsToWrappers.get(descriptor);
-
-            if (descriptorWrapper == null) {
-                descriptorWrapper = new BluetoothGattDescriptorWrapper(descriptor, mDeviceWrapper);
-                mDeviceWrapper.mDescriptorsToWrappers.put(descriptor, descriptorWrapper);
-            }
-            return descriptorWrapper;
-        }
-
         public List<BluetoothGattDescriptorWrapper> getDescriptors() {
             List<BluetoothGattDescriptor> descriptors = mCharacteristic.getDescriptors();
 
diff --git a/device/bluetooth/bluetooth_gatt_characteristic.cc b/device/bluetooth/bluetooth_gatt_characteristic.cc
index 66c39f6..d4b3427 100644
--- a/device/bluetooth/bluetooth_gatt_characteristic.cc
+++ b/device/bluetooth/bluetooth_gatt_characteristic.cc
@@ -25,14 +25,15 @@ BluetoothGattCharacteristic* BluetoothGattCharacteristic::Create(
   return NULL;
 }
 
-BluetoothGattDescriptor* BluetoothGattCharacteristic::GetDescriptorForUUID(
-    const BluetoothUUID& uuid) {
+std::vector<BluetoothGattDescriptor*>
+BluetoothGattCharacteristic::GetDescriptorsByUUID(const BluetoothUUID& uuid) {
+  std::vector<BluetoothGattDescriptor*> descriptors;
   for (BluetoothGattDescriptor* descriptor : GetDescriptors()) {
     if (descriptor->GetUUID() == uuid) {
-      return descriptor;
+      descriptors.push_back(descriptor);
     }
   }
-  return NULL;
+  return descriptors;
 }
 
 }  // namespace device
diff --git a/device/bluetooth/bluetooth_gatt_characteristic.h b/device/bluetooth/bluetooth_gatt_characteristic.h
index 2eeeeff..c578d90 100644
--- a/device/bluetooth/bluetooth_gatt_characteristic.h
+++ b/device/bluetooth/bluetooth_gatt_characteristic.h
@@ -158,8 +158,10 @@ class DEVICE_BLUETOOTH_EXPORT BluetoothGattCharacteristic {
   virtual BluetoothGattDescriptor* GetDescriptor(
       const std::string& identifier) const = 0;
 
-  // Returns the GATT characteristic descriptor that matches |uuid|.
-  virtual BluetoothGattDescriptor* GetDescriptorForUUID(
+  // Returns the GATT characteristic descriptors that match |uuid|. There may be
+  // multiple, as illustrated by Core Bluetooth Specification [V4.2 Vol 3 Part G
+  // 3.3.3.5 Characteristic Presentation Format].
+  std::vector<BluetoothGattDescriptor*> GetDescriptorsByUUID(
       const BluetoothUUID& uuid);
 
   // Adds a characteristic descriptor to the locally hosted characteristic
diff --git a/device/bluetooth/bluetooth_gatt_characteristic_unittest.cc b/device/bluetooth/bluetooth_gatt_characteristic_unittest.cc
index 93a0c9e..3e4db9b 100644
--- a/device/bluetooth/bluetooth_gatt_characteristic_unittest.cc
+++ b/device/bluetooth/bluetooth_gatt_characteristic_unittest.cc
@@ -52,6 +52,7 @@ class BluetoothGattCharacteristicTest : public BluetoothTest {
   enum class StartNotifySetupError {
     CHARACTERISTIC_PROPERTIES,
     CONFIG_DESCRIPTOR_MISSING,
+    CONFIG_DESCRIPTOR_DUPLICATE,
     SET_NOTIFY,
     WRITE_DESCRIPTOR,
     NONE
@@ -69,13 +70,23 @@ class BluetoothGattCharacteristicTest : public BluetoothTest {
     }
     ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate(properties));
 
+    size_t expected_descriptors_count = 0;
     if (error != StartNotifySetupError::CONFIG_DESCRIPTOR_MISSING) {
       SimulateGattDescriptor(
           characteristic1_,
-          /* Client Characteristic Configuration descriptor's standard UUID: */
-          "00002902-0000-1000-8000-00805F9B34FB");
-      ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
+          BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()
+              .canonical_value());
+      expected_descriptors_count++;
     }
+    if (error == StartNotifySetupError::CONFIG_DESCRIPTOR_DUPLICATE) {
+      SimulateGattDescriptor(
+          characteristic1_,
+          BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()
+              .canonical_value());
+      expected_descriptors_count++;
+    }
+    ASSERT_EQ(expected_descriptors_count,
+              characteristic1_->GetDescriptors().size());
 
     if (error == StartNotifySetupError::SET_NOTIFY) {
       SimulateGattCharacteristicSetNotifyWillFailSynchronouslyOnce(
@@ -776,6 +787,28 @@ TEST_F(BluetoothGattCharacteristicTest, StartNotifySession_NoConfigDescriptor) {
   EXPECT_EQ(0, error_callback_count_);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(1, error_callback_count_);
+  EXPECT_EQ(BluetoothGattService::GATT_ERROR_NOT_SUPPORTED,
+            last_gatt_error_code_);
+}
+#endif  // defined(OS_ANDROID)
+
+#if defined(OS_ANDROID)
+// StartNotifySession fails if the characteristic has multiple Client
+// Characteristic Configuration descriptors.
+TEST_F(BluetoothGattCharacteristicTest,
+       StartNotifySession_MultipleConfigDescriptor) {
+  ASSERT_NO_FATAL_FAILURE(StartNotifyBoilerplate(
+      /* properties: NOTIFY */ 0x10,
+      /* expected_config_descriptor_value: NOTIFY */ 1,
+      StartNotifySetupError::CONFIG_DESCRIPTOR_DUPLICATE));
+
+  EXPECT_EQ(0, gatt_notify_characteristic_attempts_);
+
+  // The expected error callback is asynchronous:
+  EXPECT_EQ(0, error_callback_count_);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, error_callback_count_);
+  EXPECT_EQ(BluetoothGattService::GATT_ERROR_FAILED, last_gatt_error_code_);
 }
 #endif  // defined(OS_ANDROID)
 
@@ -855,8 +888,8 @@ TEST_F(BluetoothGattCharacteristicTest, StartNotifySession_Multiple) {
       FakeCharacteristicBoilerplate(/* properties: NOTIFY */ 0x10));
   SimulateGattDescriptor(
       characteristic1_,
-      /* Client Characteristic Configuration descriptor's standard UUID: */
-      "00002902-0000-1000-8000-00805F9B34FB");
+      BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid()
+          .canonical_value());
   ASSERT_EQ(1u, characteristic1_->GetDescriptors().size());
 
   characteristic1_->StartNotifySession(
@@ -977,4 +1010,34 @@ TEST_F(BluetoothGattCharacteristicTest, GetDescriptors_and_GetDescriptor) {
 }
 #endif  // defined(OS_ANDROID) || defined(OS_WIN)
 
+#if defined(OS_ANDROID) || defined(OS_WIN)
+TEST_F(BluetoothGattCharacteristicTest, GetDescriptorsByUUID) {
+  ASSERT_NO_FATAL_FAILURE(FakeCharacteristicBoilerplate());
+
+  // Add several Descriptors:
+  BluetoothUUID id1("11111111-0000-1000-8000-00805f9b34fb");
+  BluetoothUUID id2("22222222-0000-1000-8000-00805f9b34fb");
+  BluetoothUUID id3("33333333-0000-1000-8000-00805f9b34fb");
+  SimulateGattDescriptor(characteristic1_, id1.canonical_value());
+  SimulateGattDescriptor(characteristic1_, id2.canonical_value());
+  SimulateGattDescriptor(characteristic2_, id3.canonical_value());
+  SimulateGattDescriptor(characteristic2_, id3.canonical_value());
+
+  EXPECT_NE(characteristic2_->GetDescriptorsByUUID(id3).at(0)->GetIdentifier(),
+            characteristic2_->GetDescriptorsByUUID(id3).at(1)->GetIdentifier());
+
+  EXPECT_EQ(id1, characteristic1_->GetDescriptorsByUUID(id1).at(0)->GetUUID());
+  EXPECT_EQ(id2, characteristic1_->GetDescriptorsByUUID(id2).at(0)->GetUUID());
+  EXPECT_EQ(id3, characteristic2_->GetDescriptorsByUUID(id3).at(0)->GetUUID());
+  EXPECT_EQ(id3, characteristic2_->GetDescriptorsByUUID(id3).at(1)->GetUUID());
+  EXPECT_EQ(1u, characteristic1_->GetDescriptorsByUUID(id1).size());
+  EXPECT_EQ(1u, characteristic1_->GetDescriptorsByUUID(id2).size());
+  EXPECT_EQ(2u, characteristic2_->GetDescriptorsByUUID(id3).size());
+
+  EXPECT_EQ(0u, characteristic2_->GetDescriptorsByUUID(id1).size());
+  EXPECT_EQ(0u, characteristic2_->GetDescriptorsByUUID(id2).size());
+  EXPECT_EQ(0u, characteristic1_->GetDescriptorsByUUID(id3).size());
+}
+#endif  // defined(OS_ANDROID) || defined(OS_WIN)
+
 }  // namespace device
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc
index bb7e669..c4a73ff 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_android.cc
@@ -150,21 +150,22 @@ void BluetoothRemoteGattCharacteristicAndroid::StartNotifySession(
     LOG(ERROR) << "Characteristic needs NOTIFY or INDICATE";
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
-        base::Bind(error_callback,
-                   BluetoothRemoteGattServiceAndroid::GATT_ERROR_FAILED));
+        base::Bind(error_callback, BluetoothGattService::GATT_ERROR_FAILED));
     return;
   }
 
-  BluetoothGattDescriptor* descriptor = GetDescriptorForUUID(
+  std::vector<BluetoothGattDescriptor*> ccc_descriptor = GetDescriptorsByUUID(
       BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid());
 
-  if (!descriptor) {
-    LOG(ERROR)
-        << "Could not find client characteristic configuration descriptor";
+  if (ccc_descriptor.size() != 1u) {
+    LOG(ERROR) << "Found " << ccc_descriptor.size()
+               << " client characteristic configuration descriptors.";
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(error_callback,
-                   BluetoothRemoteGattServiceAndroid::GATT_ERROR_FAILED));
+                   (ccc_descriptor.size() == 0)
+                       ? BluetoothGattService::GATT_ERROR_NOT_SUPPORTED
+                       : BluetoothGattService::GATT_ERROR_FAILED));
     return;
   }
 
@@ -173,8 +174,7 @@ void BluetoothRemoteGattCharacteristicAndroid::StartNotifySession(
     LOG(ERROR) << "Error enabling characteristic notification";
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
-        base::Bind(error_callback,
-                   BluetoothRemoteGattServiceAndroid::GATT_ERROR_FAILED));
+        base::Bind(error_callback, BluetoothGattService::GATT_ERROR_FAILED));
     return;
   }
 
@@ -183,7 +183,7 @@ void BluetoothRemoteGattCharacteristicAndroid::StartNotifySession(
   value.push_back(0);
 
   pending_start_notify_calls_.push(std::make_pair(callback, error_callback));
-  descriptor->WriteRemoteDescriptor(
+  ccc_descriptor[0]->WriteRemoteDescriptor(
       value, base::Bind(&BluetoothRemoteGattCharacteristicAndroid::
                             OnStartNotifySessionSuccess,
                         base::Unretained(this)),
@@ -206,8 +206,7 @@ void BluetoothRemoteGattCharacteristicAndroid::ReadRemoteCharacteristic(
           AttachCurrentThread(), j_characteristic_.obj())) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
-        base::Bind(error_callback,
-                   BluetoothRemoteGattServiceAndroid::GATT_ERROR_FAILED));
+        base::Bind(error_callback, BluetoothGattService::GATT_ERROR_FAILED));
     return;
   }
 
@@ -233,8 +232,7 @@ void BluetoothRemoteGattCharacteristicAndroid::WriteRemoteCharacteristic(
           base::android::ToJavaByteArray(env, new_value).obj())) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
-        base::Bind(error_callback,
-                   BluetoothRemoteGattServiceAndroid::GATT_ERROR_FAILED));
+        base::Bind(error_callback, BluetoothGattService::GATT_ERROR_FAILED));
     return;
   }
 
diff --git a/device/bluetooth/test/android/java/src/org/chromium/device/bluetooth/Fakes.java b/device/bluetooth/test/android/java/src/org/chromium/device/bluetooth/Fakes.java
index f3d98a7..b3677b0 100644
--- a/device/bluetooth/test/android/java/src/org/chromium/device/bluetooth/Fakes.java
+++ b/device/bluetooth/test/android/java/src/org/chromium/device/bluetooth/Fakes.java
@@ -632,15 +632,6 @@ class Fakes {
                     (FakeBluetoothGattCharacteristic) chromeCharacteristic.mCharacteristic;
             UUID uuid = UUID.fromString(uuidString);
 
-            // Check for duplicates
-            for (Wrappers.BluetoothGattDescriptorWrapper descriptor :
-                    fakeCharacteristic.mDescriptors) {
-                if (descriptor.getUuid().equals(uuid)) {
-                    throw new IllegalArgumentException(
-                            "FakeBluetoothGattCharacteristic addDescriptor called with uuid '"
-                            + uuidString + "' that has already been added to this characteristic.");
-                }
-            }
             fakeCharacteristic.mDescriptors.add(
                     new FakeBluetoothGattDescriptor(fakeCharacteristic, uuid));
         }
@@ -649,16 +640,6 @@ class Fakes {
         // Wrappers.BluetoothGattCharacteristicWrapper overrides:
 
         @Override
-        public Wrappers.BluetoothGattDescriptorWrapper getDescriptor(UUID uuid) {
-            for (Wrappers.BluetoothGattDescriptorWrapper descriptor : mDescriptors) {
-                if (descriptor.getUuid().equals(uuid)) {
-                    return descriptor;
-                }
-            }
-            return null;
-        }
-
-        @Override
         public List<Wrappers.BluetoothGattDescriptorWrapper> getDescriptors() {
             return mDescriptors;
         }
diff --git a/device/bluetooth/test/bluetooth_test_android.cc b/device/bluetooth/test/bluetooth_test_android.cc
index b1cd357..a3dfa4d 100644
--- a/device/bluetooth/test/bluetooth_test_android.cc
+++ b/device/bluetooth/test/bluetooth_test_android.cc
@@ -176,8 +176,11 @@ void BluetoothTestAndroid::RememberCharacteristicForSubsequentAction(
 
 void BluetoothTestAndroid::SimulateGattNotifySessionStarted(
     BluetoothGattCharacteristic* characteristic) {
-  BluetoothGattDescriptor* descriptor = characteristic->GetDescriptorForUUID(
-      BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid());
+  BluetoothGattDescriptor* descriptor =
+      characteristic
+          ->GetDescriptorsByUUID(
+              BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid())
+          .at(0);
   BluetoothRemoteGattDescriptorAndroid* descriptor_android =
       static_cast<BluetoothRemoteGattDescriptorAndroid*>(descriptor);
   Java_FakeBluetoothGattDescriptor_valueWrite(