Implement WebBluetooth getDescriptor[s]

content/browser/bluetooth/web_bluetooth_service_impl.cc

Index: content/browser/bluetooth/web_bluetooth_service_impl.cc
diff --git a/content/browser/bluetooth/web_bluetooth_service_impl.cc b/content/browser/bluetooth/web_bluetooth_service_impl.cc
index 0b70dec104fe52a3d47ae4b58a563db87f10a0f1..63105bdd4961214f12959cfe1ba157d835ec6ec5 100644
--- a/content/browser/bluetooth/web_bluetooth_service_impl.cc
+++ b/content/browser/bluetooth/web_bluetooth_service_impl.cc
@@ -2,12 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// ID Not In Map Note:
-// A service, characteristic, or descriptor ID not in the corresponding
-// WebBluetoothServiceImpl map [service_id_to_device_address_,
-// characteristic_id_to_service_id_, descriptor_to_characteristic_] implies a
-// hostile renderer because a renderer obtains the corresponding ID from this
-// class and it will be added to the map at that time.
+// ID Not In Map Note: A service, characteristic, or descriptor ID not in the
+// corresponding WebBluetoothServiceImpl map [service_id_to_device_address_,
+// characteristic_id_to_service_id_, descriptor_id_to_characteristic_id_]
+// implies a hostile renderer because a renderer obtains the corresponding ID
+// from this class and it will be added to the map at that time.
 
 #include "content/browser/bluetooth/web_bluetooth_service_impl.h"
 
@@ -27,6 +26,7 @@
 #include "content/public/browser/web_contents.h"
 #include "device/bluetooth/bluetooth_adapter_factory_wrapper.h"
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic.h"
+#include "device/bluetooth/bluetooth_remote_gatt_descriptor.h"
 
 using device::BluetoothAdapterFactoryWrapper;
 using device::BluetoothUUID;
@@ -129,6 +129,21 @@ blink::mojom::WebBluetoothResult TranslateGATTErrorAndRecord(
   NOTREACHED();
   return blink::mojom::WebBluetoothResult::GATT_UNTRANSLATED_ERROR_CODE;
 }
+std::vector<device::BluetoothRemoteGattDescriptor*> GetDescriptorsByUUID(
+    device::BluetoothRemoteGattCharacteristic* characteristic,
+    const BluetoothUUID& descriptor_uuid) {
+  std::vector<device::BluetoothRemoteGattDescriptor*> descriptors;
+  VLOG(1) << "Looking for descriptor: " << descriptor_uuid.canonical_value();

[dcheng, 2016/12/29 09:07:18]

DVLOG

[dougt, 2017/01/12 19:38:16]

Other logging messages in this file use VLOG().

[dcheng, 2017/01/13 09:28:42]

They should be changed to DVLOG unless there's a good reason to do otherwise.
These strings take up space in an official build, even though they're basically
never seen.

+  for (device::BluetoothRemoteGattDescriptor* descriptor :
+       characteristic->GetDescriptors()) {
+    VLOG(1) << "Descriptor in cache: "

[dcheng, 2016/12/29 09:07:18]

Ditto: DVLOG

[dougt, 2017/01/12 19:38:16]

Acknowledged.

[dougt, 2017/01/13 20:08:03]

I am removing the VLOG in this directory in:
    https://codereview.chromium.org/2631773002/

+            << descriptor->GetUUID().canonical_value();
+    if (descriptor->GetUUID() == descriptor_uuid) {
+      descriptors.push_back(descriptor);
+    }
+  }
+  return descriptors;
+}
 
 // TODO(ortuno): This should really be a BluetoothDevice method.
 // Replace when implemented. http://crbug.com/552022
@@ -540,6 +555,75 @@ void WebBluetoothServiceImpl::RemoteServiceGetCharacteristics(
                base::nullopt /* characteristics */);
 }
 
+void WebBluetoothServiceImpl::RemoteCharacteristicGetDescriptors(
+    const std::string& characteristic_instance_id,
+    blink::mojom::WebBluetoothGATTQueryQuantity quantity,
+    const base::Optional<BluetoothUUID>& descriptors_uuid,
+    const RemoteCharacteristicGetDescriptorsCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  if (descriptors_uuid &&
+      BluetoothBlocklist::Get().IsExcluded(descriptors_uuid.value())) {
+    callback.Run(blink::mojom::WebBluetoothResult::BLOCKLISTED_DESCRIPTOR_UUID,
+                 base::nullopt /* descriptor */);
+    return;
+  }
+
+  const CacheQueryResult query_result =
+      QueryCacheForCharacteristic(characteristic_instance_id);
+
+  if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
+    return;
+  }
+
+  if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
+    callback.Run(query_result.GetWebResult(), base::nullopt /* descriptor */);
+    return;
+  }
+
+  std::vector<device::BluetoothRemoteGattDescriptor*> descriptors =
+      descriptors_uuid ? GetDescriptorsByUUID(query_result.characteristic,
+                                              descriptors_uuid.value())
+                       : query_result.characteristic->GetDescriptors();
+
+  std::vector<blink::mojom::WebBluetoothRemoteGATTDescriptorPtr>
+      response_descriptors;
+  for (device::BluetoothRemoteGattDescriptor* descriptor : descriptors) {
+    if (BluetoothBlocklist::Get().IsExcluded(descriptor->GetUUID())) {
+      continue;
+    }
+    std::string descriptor_instance_id = descriptor->GetIdentifier();
+
+    auto insert_result = descriptor_id_to_characteristic_id_.insert(
+        {descriptor_instance_id, characteristic_instance_id});
+    // If value is already in map, DCHECK it's valid.
+    if (!insert_result.second)
+      DCHECK(insert_result.first->second == characteristic_instance_id);
+
+    blink::mojom::WebBluetoothRemoteGATTDescriptorPtr descriptor_ptr =
+        blink::mojom::WebBluetoothRemoteGATTDescriptor::New();
+
+    descriptor_ptr->instance_id = descriptor_instance_id;
+    descriptor_ptr->uuid = descriptor->GetUUID().canonical_value();

[dcheng, 2016/12/29 09:07:18]

Please use a typemap for this, rather than building the mojo struct by hand.

[dougt, 2017/01/12 19:38:16]

We'll do this in a follow up.

[dougt, 2017/01/12 19:38:16]

We will do this in a follow.  dcheng will send mail to chromium-mojo to discuss
approach.

+    response_descriptors.push_back(std::move(descriptor_ptr));
+
+    if (quantity == blink::mojom::WebBluetoothGATTQueryQuantity::SINGLE) {
+      break;
+    }
+  }
+
+  if (!response_descriptors.empty()) {
+    callback.Run(blink::mojom::WebBluetoothResult::SUCCESS,
+                 std::move(response_descriptors));
+    return;
+  }
+
+  callback.Run(descriptors_uuid
+                   ? blink::mojom::WebBluetoothResult::DESCRIPTOR_NOT_FOUND
+                   : blink::mojom::WebBluetoothResult::NO_DESCRIPTORS_FOUND,
+               base::nullopt /* descriptors */);
+}
+
 void WebBluetoothServiceImpl::RemoteCharacteristicReadValue(
     const std::string& characteristic_instance_id,
     const RemoteCharacteristicReadValueCallback& callback) {
@@ -569,9 +653,9 @@ void WebBluetoothServiceImpl::RemoteCharacteristicReadValue(
   }
 
   query_result.characteristic->ReadRemoteCharacteristic(
-      base::Bind(&WebBluetoothServiceImpl::OnReadValueSuccess,
+      base::Bind(&WebBluetoothServiceImpl::OnCharacteristicReadValueSuccess,
                  weak_ptr_factory_.GetWeakPtr(), callback),
-      base::Bind(&WebBluetoothServiceImpl::OnReadValueFailed,
+      base::Bind(&WebBluetoothServiceImpl::OnCharacteristicReadValueFailed,
                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
@@ -612,9 +696,10 @@ void WebBluetoothServiceImpl::RemoteCharacteristicWriteValue(
   }
 
   query_result.characteristic->WriteRemoteCharacteristic(
-      value, base::Bind(&WebBluetoothServiceImpl::OnWriteValueSuccess,
-                        weak_ptr_factory_.GetWeakPtr(), callback),
-      base::Bind(&WebBluetoothServiceImpl::OnWriteValueFailed,
+      value,
+      base::Bind(&WebBluetoothServiceImpl::OnCharacteristicWriteValueSuccess,
+                 weak_ptr_factory_.GetWeakPtr(), callback),
+      base::Bind(&WebBluetoothServiceImpl::OnCharacteristicWriteValueFailed,
                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
@@ -831,7 +916,7 @@ void WebBluetoothServiceImpl::OnCreateGATTConnectionFailed(
   callback.Run(TranslateConnectErrorAndRecord(error_code));
 }
 
-void WebBluetoothServiceImpl::OnReadValueSuccess(
+void WebBluetoothServiceImpl::OnCharacteristicReadValueSuccess(
     const RemoteCharacteristicReadValueCallback& callback,
     const std::vector<uint8_t>& value) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -839,7 +924,7 @@ void WebBluetoothServiceImpl::OnReadValueSuccess(
   callback.Run(blink::mojom::WebBluetoothResult::SUCCESS, value);
 }
 
-void WebBluetoothServiceImpl::OnReadValueFailed(
+void WebBluetoothServiceImpl::OnCharacteristicReadValueFailed(
     const RemoteCharacteristicReadValueCallback& callback,
     device::BluetoothRemoteGattService::GattErrorCode error_code) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -848,14 +933,14 @@ void WebBluetoothServiceImpl::OnReadValueFailed(
                base::nullopt /* value */);
 }
 
-void WebBluetoothServiceImpl::OnWriteValueSuccess(
+void WebBluetoothServiceImpl::OnCharacteristicWriteValueSuccess(
     const RemoteCharacteristicWriteValueCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordCharacteristicWriteValueOutcome(UMAGATTOperationOutcome::SUCCESS);
   callback.Run(blink::mojom::WebBluetoothResult::SUCCESS);
 }
 
-void WebBluetoothServiceImpl::OnWriteValueFailed(
+void WebBluetoothServiceImpl::OnCharacteristicWriteValueFailed(
     const RemoteCharacteristicWriteValueCallback& callback,
     device::BluetoothRemoteGattService::GattErrorCode error_code) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -995,6 +1080,7 @@ url::Origin WebBluetoothServiceImpl::GetOrigin() {
 void WebBluetoothServiceImpl::ClearState() {
   characteristic_id_to_notify_session_.clear();
   pending_primary_services_requests_.clear();
+  descriptor_id_to_characteristic_id_.clear();
   characteristic_id_to_service_id_.clear();
   service_id_to_device_address_.clear();
   connected_devices_.reset(