bluetooth: Move GetCharacteristic(s) over to Mojo

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

Index: third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTService.cpp
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTService.cpp b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTService.cpp
index 721686281ebb400495580c620e3e57acee9f1781..b94c2bf45d5703e3ddff8f9fb249f598e98ebeb5 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTService.cpp
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTService.cpp
@@ -30,25 +30,51 @@ BluetoothRemoteGATTService* BluetoothRemoteGATTService::take(ScriptPromiseResolv
     return new BluetoothRemoteGATTService(webService);
 }
 
-ScriptPromise BluetoothRemoteGATTService::getCharacteristic(ScriptState* scriptState,
-    const StringOrUnsignedLong& characteristic, ExceptionState& exceptionState)
-{
-    WebBluetooth* webbluetooth = BluetoothSupplement::fromScriptState(scriptState);
+// 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) {}
 
-    String characteristicUUID = BluetoothUUID::getCharacteristic(characteristic, exceptionState);
-    if (exceptionState.hadException())
-        return exceptionState.reject(scriptState);
+    void onSuccess(const WebVector<WebBluetoothRemoteGATTCharacteristicInit*>& webCharacteristics) override
+    {
+        if (!m_resolver->getExecutionContext() || m_resolver->getExecutionContext()->activeDOMObjectsAreStopped())
+            return;
 
-    ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
-    ScriptPromise promise = resolver->promise();
-    webbluetooth->getCharacteristic(m_webService->serviceInstanceID, characteristicUUID, new CallbackPromiseAdapter<BluetoothRemoteGATTCharacteristic, BluetoothError>(resolver));
+        if (m_singleCharacteristic) {
+            DCHECK_EQ(1u, webCharacteristics.size());
+            m_resolver->resolve(BluetoothRemoteGATTCharacteristic::take(m_resolver, adoptPtr(webCharacteristics[0])));
+            return;
+        }
 
-    return promise;
-}
+        HeapVector<Member<BluetoothRemoteGATTCharacteristic>> characteristics;
+        characteristics.reserveInitialCapacity(webCharacteristics.size());
+        for (WebBluetoothRemoteGATTCharacteristicInit* webCharacteristic : webCharacteristics) {
+            characteristics.append(BluetoothRemoteGATTCharacteristic::take(m_resolver, adoptPtr(webCharacteristic)));
+        }
+        m_resolver->resolve(characteristics);
+    }
 
-ScriptPromise BluetoothRemoteGATTService::getCharacteristics(ScriptState* scriptState, ExceptionState&)
+    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)
 {
-    return getCharacteristicsImpl(scriptState, String());
+    String characteristicUUID = BluetoothUUID::getCharacteristic(characteristic, exceptionState);
+    if (exceptionState.hadException())
+        return exceptionState.reject(scriptState);
+
+    return getCharacteristicsImpl(scriptState, true /* single_characteristic */, characteristicUUID);
 }
 
 ScriptPromise BluetoothRemoteGATTService::getCharacteristics(ScriptState* scriptState, const StringOrUnsignedLong& characteristic, ExceptionState& exceptionState)
@@ -57,33 +83,21 @@ ScriptPromise BluetoothRemoteGATTService::getCharacteristics(ScriptState* script
     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;
 }