[Onion Soup] Move WebBluetoothImpl from //content/renderer/bluetooth to Blink's bluetooth module

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

Index: third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
index 1fbd1f65bd533253d9e8bd7a020cfb29bbb72f3d..6f7c657e4569005a0e6275310e4fd643f2711640 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
@@ -13,9 +13,8 @@
 #include "modules/bluetooth/Bluetooth.h"
 #include "modules/bluetooth/BluetoothError.h"
 #include "modules/bluetooth/BluetoothRemoteGATTService.h"
-#include "modules/bluetooth/BluetoothSupplement.h"
 #include "modules/bluetooth/BluetoothUUID.h"
-#include "public/platform/modules/bluetooth/WebBluetooth.h"
+#include "modules/bluetooth/BluetoothUtils.h"
 
 namespace blink {
 
@@ -25,7 +24,8 @@ const char kGATTServerDisconnected[] =
     "GATT Server disconnected while retrieving services.";
 const char kGATTServerNotConnected[] =
     "GATT Server is disconnected. Cannot retrieve services.";
-}
+
+}  // namespace
 
 BluetoothRemoteGATTServer::BluetoothRemoteGATTServer(BluetoothDevice* device)
     : m_device(device), m_connected(false) {}
@@ -55,45 +55,36 @@ DEFINE_TRACE(BluetoothRemoteGATTServer) {
   visitor->trace(m_device);
 }
 
-class ConnectCallback : public WebBluetoothRemoteGATTServerConnectCallbacks {
- public:
-  ConnectCallback(BluetoothDevice* device, ScriptPromiseResolver* resolver)
-      : m_device(device), m_resolver(resolver) {}
+void BluetoothRemoteGATTServer::ConnectCallback(
+    ScriptPromiseResolver* resolver,
+    mojom::blink::WebBluetoothResult result) {
+  if (!resolver->getExecutionContext() ||
+      resolver->getExecutionContext()->isContextDestroyed())
+    return;

[Reilly Grant (use Gerrit), 2016/12/16 01:41:29]

This check is unnecessary. It is performed by
ScriptPromiseResolver::resolve/reject.

[haraken, 2016/12/16 11:24:05]

Is it okay to call m_device->gatt()->setConnected(true) when the context is
detached?

[juncai, 2016/12/17 01:18:52]

I think when context is detached, the m_device->gatt() may be garbage collected.
So it is necessary to have the above check.

 
-  void onSuccess() override {
-    if (!m_resolver->getExecutionContext() ||
-        m_resolver->getExecutionContext()->isContextDestroyed())
-      return;
+  if (result == mojom::blink::WebBluetoothResult::SUCCESS) {
     m_device->gatt()->setConnected(true);
-    m_resolver->resolve(m_device->gatt());
-  }
-
-  void onError(
-      int32_t
-          error /* Corresponds to WebBluetoothResult in web_bluetooth.mojom */)
-      override {
-    if (!m_resolver->getExecutionContext() ||
-        m_resolver->getExecutionContext()->isContextDestroyed())
-      return;
-    m_resolver->reject(BluetoothError::take(m_resolver, error));
+    resolver->resolve(m_device->gatt());
+  } else {
+    resolver->reject(BluetoothError::take(resolver, static_cast<int>(result)));
   }
-
- private:
-  Persistent<BluetoothDevice> m_device;
-  Persistent<ScriptPromiseResolver> m_resolver;
-};
+}
 
 ScriptPromise BluetoothRemoteGATTServer::connect(ScriptState* scriptState) {
-  WebBluetooth* webbluetooth =
-      BluetoothSupplement::fromScriptState(scriptState);
-  if (!webbluetooth)
-    return ScriptPromise::rejectWithDOMException(
-        scriptState, DOMException::create(NotSupportedError));
+  m_device->bluetooth()->addDevice(device()->id(), device());
 
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
-  webbluetooth->connect(device()->id(), device(),
-                        new ConnectCallback(device(), resolver));
+
+  mojom::blink::WebBluetoothService* service = m_device->bluetooth()->service();
+  mojom::blink::WebBluetoothDeviceIdPtr deviceIdPtr =
+      createWebBluetoothDeviceIdPtr(device()->id());

[Reilly Grant (use Gerrit), 2016/12/16 01:41:29]

Don't put "Ptr" in the variable name.

[juncai, 2016/12/17 01:18:52]

Done.

+  service->RemoteServerConnect(
+      std::move(deviceIdPtr),
+      convertToBaseCallback(
+          WTF::bind(&BluetoothRemoteGATTServer::ConnectCallback,
+                    wrapPersistent(this), wrapPersistent(resolver))));
+
   return promise;
 }
 
@@ -101,79 +92,61 @@ void BluetoothRemoteGATTServer::disconnect(ScriptState* scriptState) {
   if (!m_connected)
     return;
   device()->cleanupDisconnectedDeviceAndFireEvent();
-  WebBluetooth* webbluetooth =
-      BluetoothSupplement::fromScriptState(scriptState);
-  webbluetooth->disconnect(device()->id());
+  m_device->bluetooth()->removeDevice(device()->id());
+  mojom::blink::WebBluetoothService* service = m_device->bluetooth()->service();
+  mojom::blink::WebBluetoothDeviceIdPtr deviceIdPtr =
+      createWebBluetoothDeviceIdPtr(device()->id());
+  service->RemoteServerDisconnect(std::move(deviceIdPtr));
 }
 
-// Class that allows us to resolve the promise with a single service or
+// Callback that allows us to resolve the promise with a single service or
 // with a vector owning the services.
-class GetPrimaryServicesCallback
-    : public WebBluetoothGetPrimaryServicesCallbacks {
- public:
-  GetPrimaryServicesCallback(
-      BluetoothDevice* device,
-      mojom::blink::WebBluetoothGATTQueryQuantity quantity,
-      ScriptPromiseResolver* resolver)
-      : m_device(device), m_quantity(quantity), m_resolver(resolver) {
-    // We always check that the device is connected before constructing this
-    // object.
-    CHECK(m_device->gatt()->connected());
-    m_device->gatt()->AddToActiveAlgorithms(m_resolver.get());
-  }
-
-  void onSuccess(
-      const WebVector<WebBluetoothRemoteGATTService*>& webServices) override {
-    if (!m_resolver->getExecutionContext() ||
-        m_resolver->getExecutionContext()->isContextDestroyed())
-      return;
+void BluetoothRemoteGATTServer::GetPrimaryServicesCallback(
+    mojom::blink::WebBluetoothGATTQueryQuantity quantity,
+    ScriptPromiseResolver* resolver,
+    mojom::blink::WebBluetoothResult result,
+    Optional<Vector<mojom::blink::WebBluetoothRemoteGATTServicePtr>> services) {
+  if (!resolver->getExecutionContext() ||
+      resolver->getExecutionContext()->isContextDestroyed())
+    return;

[Reilly Grant (use Gerrit), 2016/12/16 01:41:29]

This check is unnecessary. It is performed by
ScriptPromiseResolver::resolve/reject.

[haraken, 2016/12/16 11:24:05]

Ditto. In general, it would be better to check this at the beginning of any
callback.

[juncai, 2016/12/17 01:18:52]

For callbacks that do more than ScriptPromiseResolver::resolve/reject, keep this
check at the beginning of the callback.

 
+  if (result == mojom::blink::WebBluetoothResult::SUCCESS) {
+    DCHECK(services);
     // If the resolver is not in the set of ActiveAlgorithms then the frame
     // disconnected so we reject.
-    if (!m_device->gatt()->RemoveFromActiveAlgorithms(m_resolver.get())) {
-      m_resolver->reject(
+    if (!m_device->gatt()->RemoveFromActiveAlgorithms(resolver)) {
+      resolver->reject(
           DOMException::create(NetworkError, kGATTServerDisconnected));
       return;
     }
 
-    if (m_quantity == mojom::blink::WebBluetoothGATTQueryQuantity::SINGLE) {
-      DCHECK_EQ(1u, webServices.size());
-      m_resolver->resolve(m_device->getOrCreateBluetoothRemoteGATTService(
-          WTF::wrapUnique(webServices[0])));
+    if (quantity == mojom::blink::WebBluetoothGATTQueryQuantity::SINGLE) {
+      DCHECK_EQ(1u, services->size());
+      resolver->resolve(m_device->getOrCreateBluetoothRemoteGATTService(
+          services.value()[0]->instance_id, services.value()[0]->uuid,
+          true /* isPrimary */, device()->id()));
       return;
     }
 
-    HeapVector<Member<BluetoothRemoteGATTService>> services;
-    services.reserveInitialCapacity(webServices.size());
-    for (WebBluetoothRemoteGATTService* webService : webServices) {
-      services.append(m_device->getOrCreateBluetoothRemoteGATTService(
-          WTF::wrapUnique(webService)));
-    }
-    m_resolver->resolve(services);
-  }
-
-  void onError(
-      int32_t
-          error /* Corresponds to WebBluetoothResult in web_bluetooth.mojom */)
-      override {
-    if (!m_resolver->getExecutionContext() ||
-        m_resolver->getExecutionContext()->isContextDestroyed())
-      return;
+    HeapVector<Member<BluetoothRemoteGATTService>> gattServices;
+    gattServices.reserveInitialCapacity(services->size());
 
-    if (!m_device->gatt()->RemoveFromActiveAlgorithms(m_resolver.get())) {
-      m_resolver->reject(
+    for (const auto& service : services.value()) {
+      gattServices.append(m_device->getOrCreateBluetoothRemoteGATTService(
+          service->instance_id, service->uuid, true /* isPrimary */,
+          device()->id()));
+    }
+    resolver->resolve(gattServices);
+  } else {
+    if (!m_device->gatt()->RemoveFromActiveAlgorithms(resolver)) {
+      resolver->reject(
           DOMException::create(NetworkError, kGATTServerDisconnected));
       return;
     }
 
-    m_resolver->reject(BluetoothError::take(m_resolver, error));
+    resolver->reject(BluetoothError::take(resolver, static_cast<int>(result)));
   }
-
- private:
-  Persistent<BluetoothDevice> m_device;
-  mojom::blink::WebBluetoothGATTQueryQuantity m_quantity;
-  const Persistent<ScriptPromiseResolver> m_resolver;
-};
+}
 
 ScriptPromise BluetoothRemoteGATTServer::getPrimaryService(
     ScriptState* scriptState,
@@ -221,12 +194,21 @@ ScriptPromise BluetoothRemoteGATTServer::getPrimaryServicesImpl(
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
 
-  WebBluetooth* webbluetooth =
-      BluetoothSupplement::fromScriptState(scriptState);
-  webbluetooth->getPrimaryServices(
-      device()->id(), static_cast<int32_t>(quantity), servicesUUID,
-      new GetPrimaryServicesCallback(device(), quantity, resolver));
-
+  // We always check that the device is connected.
+  CHECK(m_device->gatt()->connected());

[dougt, 2016/12/16 00:11:44]

Another crash if we're not connected. What do you think?

[juncai, 2016/12/17 01:18:52]

I think this line can just be removed since this case is already handled on line
188.

+  m_device->gatt()->AddToActiveAlgorithms(resolver);
+
+  mojom::blink::WebBluetoothService* service = m_device->bluetooth()->service();
+  mojom::blink::WebBluetoothDeviceIdPtr deviceIdPtr =
+      createWebBluetoothDeviceIdPtr(device()->id());
+  bluetooth::mojom::blink::UUIDPtr servicesUUIDPtr = nullptr;
+  if (!servicesUUID.isEmpty())
+    servicesUUIDPtr = createUUIDPtr(servicesUUID);
+  service->RemoteServerGetPrimaryServices(
+      std::move(deviceIdPtr), quantity, std::move(servicesUUIDPtr),
+      convertToBaseCallback(
+          WTF::bind(&BluetoothRemoteGATTServer::GetPrimaryServicesCallback,
+                    wrapPersistent(this), quantity, wrapPersistent(resolver))));
   return promise;
 }