bluetooth: Move GetCharacteristic(s) over to Mojo

third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTService.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/BluetoothRemoteGATTService.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMException.h"
@@ skipping 12 matching lines @@
 }
 
 BluetoothRemoteGATTService* BluetoothRemoteGATTService::take(ScriptPromiseResolver*, PassOwnPtr<WebBluetoothRemoteGATTService> webService)
 {
     if (!webService) {
         return nullptr;
     }
     return new BluetoothRemoteGATTService(webService);
 }
 
-ScriptPromise BluetoothRemoteGATTService::getCharacteristic(ScriptState* scriptState,
-    const StringOrUnsignedLong& characteristic, ExceptionState& exceptionState)
+// Class that allows us to resolve the promise with a single Characteristic or
+// with a vector owning the characteristics.
+class GetCharacteristicsCallback : public WebBluetoothGetCharacteristicsCallbacks {

[Jeffrey Yasskin, 2016/04/22 01:16:38]

Are you sure the extra complexity in this class is worth being able to unify
OnGetCharacteristicsComplete()? It seems like it would be clearer and maybe
shorter to keep separate single- vs multiple-result paths on the renderer side.

[ortuno, 2016/04/25 15:29:45]

Thinking this through:

Right now the reason we use this class is so that we can use a single
WebCallbacks type when we call webBluetooth->getCharacteristics(). If we decide
to have separate paths, we could go back to using CallbackPromiseAdapter and get
rid of this class but we would need two functions in WebBluetoothImpl, one for
each WebCallbacks type. Each function will also have to have its own callback
with its own logic.

This seems fine for now but I'm scared we will go back to this in the long run:

We will need to implement a similar class as this for setting the parent of the
object (See ConnectCallback in BluetoothRemoteGATTServer). I think we would want
to implement only one of these classes and that class would look a lot like the
class in this CL.

Another thing to keep in mind is that the content/renderer layer is going away
soon so any decisions we take regarding it won't matter for too long.

So in conclusion I think we will eventually need this class and it already
allows us to remove some repeated code.

[Jeffrey Yasskin, 2016/04/25 17:26:10]

Ok, sounds good to keep it. Thanks for explaining the tradeoffs!

+public:
+    GetCharacteristicsCallback(bool singleCharacteristic, ScriptPromiseResolver* resolver)
+        : m_resolver(resolver)
+        , m_singleCharacteristic(singleCharacteristic) {}
+
+    void onSuccess(const WebVector<WebBluetoothRemoteGATTCharacteristicInit*>& webCharacteristics) override
+    {
+        if (!m_resolver->getExecutionContext() || m_resolver->getExecutionContext()->activeDOMObjectsAreStopped())
+            return;
+
+        if (m_singleCharacteristic) {
+            DCHECK_EQ(1u, webCharacteristics.size());
+            m_resolver->resolve(BluetoothRemoteGATTCharacteristic::take(m_resolver, adoptPtr(webCharacteristics[0])));
+            return;
+        }
+
+        HeapVector<Member<BluetoothRemoteGATTCharacteristic>> characteristics;
+        characteristics.reserveInitialCapacity(webCharacteristics.size());
+        for (WebBluetoothRemoteGATTCharacteristicInit* webCharacteristic : webCharacteristics) {
+            characteristics.append(BluetoothRemoteGATTCharacteristic::take(m_resolver, adoptPtr(webCharacteristic)));
+        }
+        m_resolver->resolve(characteristics);
+    }
+
+    void onError(const WebBluetoothError& e) override
+    {
+        if (!m_resolver->getExecutionContext() || m_resolver->getExecutionContext()->activeDOMObjectsAreStopped())
+            return;
+        m_resolver->reject(BluetoothError::take(m_resolver, e));
+    }
+private:
+    Persistent<ScriptPromiseResolver> m_resolver;
+    bool m_singleCharacteristic;
+};
+
+ScriptPromise BluetoothRemoteGATTService::getCharacteristic(ScriptState* scriptState, const StringOrUnsignedLong& characteristic, ExceptionState& exceptionState)
 {
-    WebBluetooth* webbluetooth = BluetoothSupplement::fromScriptState(scriptState);
-
     String characteristicUUID = BluetoothUUID::getCharacteristic(characteristic, exceptionState);
     if (exceptionState.hadException())
         return exceptionState.reject(scriptState);
 
-    ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
-    ScriptPromise promise = resolver->promise();
-    webbluetooth->getCharacteristic(m_webService->serviceInstanceID, characteristicUUID, new CallbackPromiseAdapter<BluetoothRemoteGATTCharacteristic, BluetoothError>(resolver));
-
-    return promise;
-}
-
-ScriptPromise BluetoothRemoteGATTService::getCharacteristics(ScriptState* scriptState, ExceptionState&)
-{
-    return getCharacteristicsImpl(scriptState, String());
+    return getCharacteristicsImpl(scriptState, true /* single_characteristic */, characteristicUUID);
 }
 
 ScriptPromise BluetoothRemoteGATTService::getCharacteristics(ScriptState* scriptState, const StringOrUnsignedLong& characteristic, ExceptionState& exceptionState)
 {
     String characteristicUUID = BluetoothUUID::getCharacteristic(characteristic, exceptionState);
     if (exceptionState.hadException())
         return exceptionState.reject(scriptState);
 
-    return getCharacteristicsImpl(scriptState, characteristicUUID);
+    return getCharacteristicsImpl(scriptState, false /* single_characteristic */, characteristicUUID);
 }
 
-// Class that allows us to use CallbackPromiseAdapter to resolve a promise with a
-// vector owning BluetoothRemoteGATTCharacteristics.
-class RemoteCharacteristicArray {
-    STATIC_ONLY(RemoteCharacteristicArray);
-public:
-    using WebType = OwnPtr<WebVector<WebBluetoothRemoteGATTCharacteristicInit*>>;
-    static HeapVector<Member<BluetoothRemoteGATTCharacteristic>> take(ScriptPromiseResolver* resolver, PassOwnPtr<WebVector<WebBluetoothRemoteGATTCharacteristicInit*>> webCharacteristics)
-    {
-        HeapVector<Member<BluetoothRemoteGATTCharacteristic>> characteristics;
-        characteristics.reserveInitialCapacity(webCharacteristics->size());
-        for (WebBluetoothRemoteGATTCharacteristicInit* webCharacteristic : *webCharacteristics) {
-            characteristics.append(BluetoothRemoteGATTCharacteristic::take(resolver, adoptPtr(webCharacteristic)));
-        }
-        return characteristics;
-    }
-};
+ScriptPromise BluetoothRemoteGATTService::getCharacteristics(ScriptState* scriptState, ExceptionState&)
+{
+    return getCharacteristicsImpl(scriptState, false /* single_characteristic */);
+}
 
-ScriptPromise BluetoothRemoteGATTService::getCharacteristicsImpl(ScriptState* scriptState, String characteristicsUUID)
+ScriptPromise BluetoothRemoteGATTService::getCharacteristicsImpl(ScriptState* scriptState, bool singleCharacteristic, String characteristicsUUID)
 {
     ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
 
     WebBluetooth* webbluetooth = BluetoothSupplement::fromScriptState(scriptState);
-    webbluetooth->getCharacteristics(m_webService->serviceInstanceID, characteristicsUUID, new CallbackPromiseAdapter<RemoteCharacteristicArray, BluetoothError>(resolver));
+    webbluetooth->getCharacteristics(m_webService->serviceInstanceID, singleCharacteristic, characteristicsUUID, new GetCharacteristicsCallback(singleCharacteristic, resolver));
 
     return promise;
 }
 
 } // namespace blink