Refactor WebBluetoothServiceClient in the web_bluetooth.mojom

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 b2f37308e39c06b5a4492be5d41eec2973b4e697..1c5da9e1ac41825cb51172486af1f39bdb586fa9 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
@@ -19,12 +19,25 @@
 
 namespace blink {
 
-BluetoothRemoteGATTServer::BluetoothRemoteGATTServer(BluetoothDevice* device)
-    : m_device(device), m_connected(false) {}
+BluetoothRemoteGATTServer::BluetoothRemoteGATTServer(ExecutionContext* context,
+                                                     BluetoothDevice* device)
+    : ContextLifecycleObserver(context),
+      m_clientBinding(this),
+      m_device(device),
+      m_connected(false) {}
 
 BluetoothRemoteGATTServer* BluetoothRemoteGATTServer::create(
+    ExecutionContext* context,
     BluetoothDevice* device) {
-  return new BluetoothRemoteGATTServer(device);
+  return new BluetoothRemoteGATTServer(context, device);
+}
+
+void BluetoothRemoteGATTServer::contextDestroyed(ExecutionContext*) {
+  disconnectIfConnected();
+}
+
+void BluetoothRemoteGATTServer::GATTServerDisconnected() {
+  dispatchDisconnected();
 }
 
 void BluetoothRemoteGATTServer::AddToActiveAlgorithms(
@@ -42,20 +55,55 @@ bool BluetoothRemoteGATTServer::RemoveFromActiveAlgorithms(
   return true;
 }
 
+void BluetoothRemoteGATTServer::disconnectIfConnected() {
+  if (connected()) {

[ortuno, 2017/03/09 23:16:38]

nit: We changed to m_connected so you might want to change connected().

[juncai, 2017/03/10 03:57:02]

Done.

+    setConnected(false);
+    ClearActiveAlgorithms();
+    mojom::blink::WebBluetoothService* service =
+        m_device->bluetooth()->service();
+    service->RemoteServerDisconnect(m_device->id());
+  }
+}
+
+void BluetoothRemoteGATTServer::cleanupDisconnectedDeviceAndFireEvent() {
+  DCHECK(connected());
+  setConnected(false);
+  ClearActiveAlgorithms();
+  m_device->clearAttributeInstanceMapAndFireEvent();
+}
+
+void BluetoothRemoteGATTServer::dispatchDisconnected() {
+  if (!connected()) {
+    return;
+  }
+  cleanupDisconnectedDeviceAndFireEvent();
+}
+
+void BluetoothRemoteGATTServer::dispose() {
+  disconnectIfConnected();
+  // The pipe to this object must be closed when is marked unreachable to
+  // prevent messages from being dispatched before lazy sweeping.
+  if (m_clientBinding.is_bound())
+    m_clientBinding.Close();
+}
+
 DEFINE_TRACE(BluetoothRemoteGATTServer) {
   visitor->trace(m_activeAlgorithms);
   visitor->trace(m_device);
+  ContextLifecycleObserver::trace(visitor);
 }
 
 void BluetoothRemoteGATTServer::ConnectCallback(
     ScriptPromiseResolver* resolver,
-    mojom::blink::WebBluetoothResult result) {
+    mojom::blink::WebBluetoothResult result,
+    mojom::blink::WebBluetoothServerClientAssociatedRequest request) {
   if (!resolver->getExecutionContext() ||
       resolver->getExecutionContext()->isContextDestroyed())
     return;
 
   if (result == mojom::blink::WebBluetoothResult::SUCCESS) {
-    m_device->bluetooth()->addToConnectedDevicesMap(m_device->id(), m_device);
+    if (request.is_pending())
+      m_clientBinding.Bind(std::move(request));
     setConnected(true);
     resolver->resolve(this);
   } else {
@@ -79,8 +127,7 @@ ScriptPromise BluetoothRemoteGATTServer::connect(ScriptState* scriptState) {
 void BluetoothRemoteGATTServer::disconnect(ScriptState* scriptState) {
   if (!m_connected)
     return;
-  m_device->cleanupDisconnectedDeviceAndFireEvent();
-  m_device->bluetooth()->removeFromConnectedDevicesMap(m_device->id());
+  cleanupDisconnectedDeviceAndFireEvent();

[ortuno, 2017/03/09 23:16:38]

Should we unbind the client as well? Otherwise we could receive two events:

gatt.connect()
  .then(() => get_request_disconnection(gatt))
  .then(requestDisconnection => {
    return Promise.all([
      requestDisconnection(), // One event for device disconnecting.
      gatt.disconnect() // one event for disconnect.
    ]);
  });

[juncai, 2017/03/10 03:57:02]

Done.

[ortuno, 2017/03/10 04:37:09]

Would it make sense to have a test for this?

[juncai, 2017/03/10 22:31:34]

Done.

   mojom::blink::WebBluetoothService* service = m_device->bluetooth()->service();
   service->RemoteServerDisconnect(m_device->id());
 }