Implement WebBluetooth getDescriptor[s]

third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/bluetooth/BluetoothRemoteGATTCharacteristic.h"
 
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMDataView.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/events/Event.h"
 #include "core/inspector/ConsoleMessage.h"
 #include "modules/bluetooth/BluetoothCharacteristicProperties.h"
 #include "modules/bluetooth/BluetoothError.h"
+#include "modules/bluetooth/BluetoothRemoteGATTDescriptor.h"
 #include "modules/bluetooth/BluetoothRemoteGATTService.h"
 #include "modules/bluetooth/BluetoothSupplement.h"
+#include "modules/bluetooth/BluetoothUUID.h"
 #include "public/platform/modules/bluetooth/WebBluetooth.h"
 #include <memory>
 
 namespace blink {
 
 namespace {
 
 const char kGATTServerDisconnected[] =
     "GATT Server disconnected while performing a GATT operation.";
 const char kGATTServerNotConnected[] =
@@ skipping 85 matching lines @@
   // We will also need to unregister a characteristic once all the event
   // listeners have been removed. See http://crbug.com/541390
   if (eventType == EventTypeNames::characteristicvaluechanged) {
     WebBluetooth* webbluetooth =
         BluetoothSupplement::fromExecutionContext(getExecutionContext());
     webbluetooth->registerCharacteristicObject(
         m_webCharacteristic->characteristicInstanceID, this);
   }
 }
 
-class ReadValueCallback : public WebBluetoothReadValueCallbacks {
+class GetDescriptorsCallback : public WebBluetoothGetDescriptorsCallbacks {
  public:
-  ReadValueCallback(BluetoothRemoteGATTCharacteristic* characteristic,
-                    ScriptPromiseResolver* resolver)
+  GetDescriptorsCallback(BluetoothRemoteGATTCharacteristic* characteristic,
+                         mojom::blink::WebBluetoothGATTQueryQuantity quantity,
+                         ScriptPromiseResolver* resolver)
+      : m_characteristic(characteristic),
+        m_quantity(quantity),
+        m_resolver(resolver) {
+    // We always check that the device is connected before constructing this
+    // object.
+    CHECK(m_characteristic->gatt()->connected());

[dcheng, 2016/12/09 08:17:57]

Why is this a CHECK? Either we should handle the error case properly, or this is
attempting to enforce some invariant (so it should be a DCHECK)

[dougt, 2016/12/09 19:20:07]

Hrm.  I think this should be handled properly and we should fail and pass up
that error when the callback object is created.

Also see:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/blueto...

ortuno, what do you think?

[scheib, 2016/12/10 01:12:59]

DCHECK is right here. (dougt, see
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
if you haven't yet). The original patch used CHECK and this wasn't debated, but
the context remains the same. https://codereview.chromium.org/2387293008

+    m_characteristic->gatt()->AddToActiveAlgorithms(m_resolver.get());
+  }
+
+  void onSuccess(const WebVector<WebBluetoothRemoteGATTDescriptorInit*>&
+                     webDescriptors) override {
+    if (!m_resolver->getExecutionContext() ||
+        m_resolver->getExecutionContext()->isContextDestroyed())
+      return;
+
+    // If the resolver is not in the set of ActiveAlgorithms then the frame
+    // disconnected so we reject.
+    if (!m_characteristic->gatt()->RemoveFromActiveAlgorithms(
+            m_resolver.get())) {
+      m_resolver->reject(
+          DOMException::create(NetworkError, kGATTServerDisconnected));
+      return;
+    }
+
+    if (m_quantity == mojom::blink::WebBluetoothGATTQueryQuantity::SINGLE) {
+      DCHECK_EQ(1u, webDescriptors.size());
+      m_resolver->resolve(
+          m_characteristic->gatt()
+              ->device()
+              ->getOrCreateBluetoothRemoteGATTDescriptor(
+                  wrapUnique(webDescriptors[0]), m_characteristic));
+      return;
+    }
+
+    HeapVector<Member<BluetoothRemoteGATTDescriptor>> descriptors;
+    descriptors.reserveInitialCapacity(webDescriptors.size());
+    for (WebBluetoothRemoteGATTDescriptorInit* webDescriptor : webDescriptors) {
+      descriptors.append(m_characteristic->gatt()
+                             ->device()
+                             ->getOrCreateBluetoothRemoteGATTDescriptor(
+                                 wrapUnique(webDescriptor), m_characteristic));

[dcheng, 2016/12/09 08:17:57]

It seems slightly surprising to me that we're transferring (?) ownership out of
a const WebVector<T*>&. If there's ownership transfer involved here, can we just
make the WebVector own unique ptrs and move here?

[dougt, 2016/12/09 19:20:07]

Another question for ortuno or vince -- not sure why we copy them, but most of
the web bluetooth callbacks do just that.

[scheib, 2016/12/10 01:12:59]

Related to the comment in content/renderer/bluetooth/web_bluetooth_impl.cc:
  // TODO(dcheng): This WebVector should use smart pointers.

This will be cleaned up as we go through onion soup and no longer rely on
WebBluetoothGetCharacteristicsCallbacks. This pattern was introduced (from
another confusing one of a unique ptr to a web vector of pointers) in
https://codereview.chromium.org/1861013005. dcheng you're not on that as a
reviewer -- so I'm presuming ortuno added your TODO after discussing with you in
another context.

[ortuno, 2016/12/11 22:12:21]

I first introduced the raw pointers pattern in http://crrev.com/1723723002
because WebPassOwnPtr didn't have a move operator. Then the pattern and TODO
were introduced by dcheng when we got rid of WebPassOwnPtr
http://crrev.com/1865913005. I just copied the code when moving from
bluetooth_dispatcher to web_bluetooth_impl.

If WebVector supports unique_ptrs then I think we could just use unique_ptr and
stop adding this TODO.

+    }
+    m_resolver->resolve(descriptors);
+  }
+
+  void onError(
+      int32_t
+          error /* Corresponds to WebBluetoothResult in web_bluetooth.mojom */)
+      override {
+    if (!m_resolver->getExecutionContext() ||
+        m_resolver->getExecutionContext()->isContextDestroyed())
+      return;
+
+    // If the resolver is not in the set of ActiveAlgorithms then the frame
+    // disconnected so we reject.
+    if (!m_characteristic->gatt()->RemoveFromActiveAlgorithms(
+            m_resolver.get())) {
+      m_resolver->reject(
+          DOMException::create(NetworkError, kGATTServerDisconnected));
+      return;
+    }
+
+    m_resolver->reject(BluetoothError::take(m_resolver, error));
+  }
+
+ private:
+  Persistent<BluetoothRemoteGATTCharacteristic> m_characteristic;
+  mojom::blink::WebBluetoothGATTQueryQuantity m_quantity;
+  const Persistent<ScriptPromiseResolver> m_resolver;
+};
+
+ScriptPromise BluetoothRemoteGATTCharacteristic::getDescriptor(
+    ScriptState* scriptState,
+    const StringOrUnsignedLong& descriptorUUID,
+    ExceptionState& exceptionState) {
+  String descriptor =
+      BluetoothUUID::getDescriptor(descriptorUUID, exceptionState);
+  if (exceptionState.hadException())
+    return exceptionState.reject(scriptState);
+
+  return getDescriptorsImpl(scriptState,
+                            mojom::blink::WebBluetoothGATTQueryQuantity::SINGLE,
+                            descriptor);
+}
+
+ScriptPromise BluetoothRemoteGATTCharacteristic::getDescriptors(
+    ScriptState* scriptState,
+    ExceptionState&) {
+  return getDescriptorsImpl(
+      scriptState, mojom::blink::WebBluetoothGATTQueryQuantity::MULTIPLE);
+}
+
+ScriptPromise BluetoothRemoteGATTCharacteristic::getDescriptors(
+    ScriptState* scriptState,
+    const StringOrUnsignedLong& descriptorUUID,
+    ExceptionState& exceptionState) {
+  String descriptor =
+      BluetoothUUID::getDescriptor(descriptorUUID, exceptionState);
+  if (exceptionState.hadException())
+    return exceptionState.reject(scriptState);
+
+  return getDescriptorsImpl(
+      scriptState, mojom::blink::WebBluetoothGATTQueryQuantity::MULTIPLE,
+      descriptor);
+}
+
+class CharacteristicReadValueCallback : public WebBluetoothReadValueCallbacks {
+ public:
+  CharacteristicReadValueCallback(
+      BluetoothRemoteGATTCharacteristic* characteristic,
+      ScriptPromiseResolver* resolver)
       : m_characteristic(characteristic), m_resolver(resolver) {
     // We always check that the device is connected before constructing this
     // object.
     CHECK(m_characteristic->gatt()->connected());
     m_characteristic->gatt()->AddToActiveAlgorithms(m_resolver.get());
   }
 
   void onSuccess(const WebVector<uint8_t>& value) override {
     if (!m_resolver->getExecutionContext() ||
         m_resolver->getExecutionContext()->isContextDestroyed())
@@ skipping 49 matching lines @@
     return ScriptPromise::rejectWithDOMException(
         scriptState,
         DOMException::create(InvalidStateError, kInvalidCharacteristic));
   }
 
   WebBluetooth* webbluetooth =
       BluetoothSupplement::fromScriptState(scriptState);
 
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
-  webbluetooth->readValue(m_webCharacteristic->characteristicInstanceID,
-                          new ReadValueCallback(this, resolver));
+  webbluetooth->characteristicReadValue(
+      m_webCharacteristic->characteristicInstanceID,
+      new CharacteristicReadValueCallback(this, resolver));
 
   return promise;
 }
 
-class WriteValueCallback : public WebBluetoothWriteValueCallbacks {
+class CharacteristicWriteValueCallback
+    : public WebBluetoothWriteValueCallbacks {
  public:
-  WriteValueCallback(BluetoothRemoteGATTCharacteristic* characteristic,
-                     ScriptPromiseResolver* resolver)
+  CharacteristicWriteValueCallback(
+      BluetoothRemoteGATTCharacteristic* characteristic,
+      ScriptPromiseResolver* resolver)
       : m_characteristic(characteristic), m_resolver(resolver) {
     // We always check that the device is connected before constructing this
     // object.
     CHECK(m_characteristic->gatt()->connected());
     m_characteristic->gatt()->AddToActiveAlgorithms(m_resolver.get());
   }
 
   void onSuccess(const WebVector<uint8_t>& value) override {
     if (!m_resolver->getExecutionContext() ||
         m_resolver->getExecutionContext()->isContextDestroyed())
@@ skipping 12 matching lines @@
     m_resolver->resolve();
   }
 
   void onError(
       int32_t
           error /* Corresponds to WebBluetoothResult in web_bluetooth.mojom */)
       override {
     if (!m_resolver->getExecutionContext() ||
         m_resolver->getExecutionContext()->isContextDestroyed())
       return;
-
     if (!m_characteristic->gatt()->RemoveFromActiveAlgorithms(
             m_resolver.get())) {
       m_resolver->reject(
           DOMException::create(NetworkError, kGATTServerDisconnected));
       return;
     }
 
     m_resolver->reject(BluetoothError::take(m_resolver, error));
   }
 
@@ skipping 30 matching lines @@
     return ScriptPromise::rejectWithDOMException(
         scriptState, DOMException::create(InvalidModificationError,
                                           "Value can't exceed 512 bytes."));
 
   // Let valueVector be a copy of the bytes held by value.
   WebVector<uint8_t> valueVector(value.bytes(), value.byteLength());
 
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
 
   ScriptPromise promise = resolver->promise();
-  webbluetooth->writeValue(m_webCharacteristic->characteristicInstanceID,
-                           valueVector, new WriteValueCallback(this, resolver));
+  webbluetooth->characteristicWriteValue(
+      m_webCharacteristic->characteristicInstanceID, valueVector,
+      new CharacteristicWriteValueCallback(this, resolver));
 
   return promise;
 }
 
 class NotificationsCallback : public WebBluetoothNotificationsCallbacks {
  public:
   NotificationsCallback(BluetoothRemoteGATTCharacteristic* characteristic,
                         ScriptPromiseResolver* resolver)
       : m_characteristic(characteristic), m_resolver(resolver) {
     // We always check that the device is connected before constructing this
@@ skipping 90 matching lines @@
 
   WebBluetooth* webbluetooth =
       BluetoothSupplement::fromScriptState(scriptState);
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
   webbluetooth->stopNotifications(m_webCharacteristic->characteristicInstanceID,
                                   new NotificationsCallback(this, resolver));
   return promise;
 }
 
+ScriptPromise BluetoothRemoteGATTCharacteristic::getDescriptorsImpl(
+    ScriptState* scriptState,
+    mojom::blink::WebBluetoothGATTQueryQuantity quantity,
+    const String& descriptor) {
+#if OS(MACOSX)
+  // TODO(crbug.com/624017): Remove when descriptors are implemented on MacOS.
+  return ScriptPromise::rejectWithDOMException(
+      scriptState, DOMException::create(NotSupportedError,
+                                        "getDescriptor[s] is not implemented "
+                                        "yet. See https://goo.gl/J6ASzs"));
+#endif  // OS(MACOSX)
+  WebBluetooth* webbluetooth =
+      BluetoothSupplement::fromScriptState(scriptState);
+
+  if (!gatt()->connected()) {
+    return ScriptPromise::rejectWithDOMException(
+        scriptState,
+        DOMException::create(NetworkError, kGATTServerNotConnected));
+  }
+
+  if (!gatt()->device()->isValidCharacteristic(
+          m_webCharacteristic->characteristicInstanceID)) {
+    return ScriptPromise::rejectWithDOMException(
+        scriptState,
+        DOMException::create(InvalidStateError, kInvalidCharacteristic));
+  }
+
+  ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
+  ScriptPromise promise = resolver->promise();
+
+  webbluetooth->getDescriptors(
+      m_webCharacteristic->characteristicInstanceID,
+      static_cast<int32_t>(quantity), descriptor,
+      new GetDescriptorsCallback(this, quantity, resolver));
+
+  return promise;
+}
+
 DEFINE_TRACE(BluetoothRemoteGATTCharacteristic) {
   visitor->trace(m_service);
   visitor->trace(m_properties);
   visitor->trace(m_value);
   EventTargetWithInlineData::trace(visitor);
   ActiveDOMObject::trace(visitor);
 }
 
 }  // namespace blink