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 8f522942f58ceff3f3dd501ebb8d4c515c280870..9005f8076a79e31ae55a305d3b4926956aa2323d 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp
@@ -28,13 +28,17 @@ const char kGATTServerNotConnected[] =
 }  // namespace
 
 BluetoothRemoteGATTServer::BluetoothRemoteGATTServer(BluetoothDevice* device)
-    : m_device(device), m_connected(false) {}
+    : m_clientBinding(this), m_device(device), m_connected(false) {}
 
 BluetoothRemoteGATTServer* BluetoothRemoteGATTServer::create(
     BluetoothDevice* device) {
   return new BluetoothRemoteGATTServer(device);
 }
 
+void BluetoothRemoteGATTServer::GattServerDisconnected() {
+  m_device->dispatchGattServerDisconnected();
+}
+
 void BluetoothRemoteGATTServer::AddToActiveAlgorithms(
     ScriptPromiseResolver* resolver) {
   auto result = m_activeAlgorithms.insert(resolver);
@@ -50,6 +54,13 @@ bool BluetoothRemoteGATTServer::RemoveFromActiveAlgorithms(
   return true;
 }
 
+void BluetoothRemoteGATTServer::dispose() {

[ortuno, 2017/02/24 03:28:52]

Similar to BluetoothRemoteGATTCharacteristic, we want the device to disconnect
as soon as you navigate away. dispose only gets called when the object gets
garbage collected which could happen many seconds after we've navigated away.
For that reason we should also implement contextDestroyed.

[juncai, 2017/03/01 02:04:12]

Done.

+  // 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);
@@ -57,13 +68,15 @@ DEFINE_TRACE(BluetoothRemoteGATTServer) {
 
 void BluetoothRemoteGATTServer::ConnectCallback(
     ScriptPromiseResolver* resolver,
-    mojom::blink::WebBluetoothResult result) {
+    mojom::blink::WebBluetoothResult result,
+    mojom::blink::WebBluetoothServerClientAssociatedRequest request) {
   if (!resolver->getExecutionContext() ||
       resolver->getExecutionContext()->isContextDestroyed())
     return;
 
+  m_clientBinding.Bind(std::move(request));

[ortuno, 2017/02/24 03:28:52]

Hmm seems we are missing tests. We need a test to make sure that the following
works:

device.gatt.connect().then(gatt => gatt.connect());


Right now that would result in the second callback getting called with a null
request. And the client would no longer receive a disconnection event. I would
first write the test and then implement the fix that way you know that the test
is working correctly.

[juncai, 2017/03/01 02:04:12]

Done.

+
   if (result == mojom::blink::WebBluetoothResult::SUCCESS) {
-    m_device->bluetooth()->addToConnectedDevicesMap(device()->id(), device());
     setConnected(true);
     resolver->resolve(this);
   } else {
@@ -88,7 +101,6 @@ void BluetoothRemoteGATTServer::disconnect(ScriptState* scriptState) {
   if (!m_connected)
     return;
   device()->cleanupDisconnectedDeviceAndFireEvent();
-  m_device->bluetooth()->removeFromConnectedDevicesMap(device()->id());
   mojom::blink::WebBluetoothService* service = m_device->bluetooth()->service();
   service->RemoteServerDisconnect(device()->id());
 }