bluetooth: Disconnect when BluetoothDevice is garbage collected.

Instead of disconnecting when BluetoothGATTRemoteServer is garbage collected
we disconnect when BluetoothDevice is gc'ed.

The idea is to form a one way relationship between BluetoothDevice to
BluetoothGATTRemoteServer meaning, BluetoothDevice can control
BluetoothGATTRemoteServer but not the other way around. This way all events
and Server state changes will go from BluetoothDevice to
BluetoothGATTRemoteServer.

BUG=582537

Review URL: https://codereview.chromium.org/1656223004

Cr-Commit-Position: refs/heads/master@{#373697}

6 files changed, 75 insertions, 78 deletions

diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.cpp b/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.cpp
index a9fc5cd..9296fe0 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.cpp
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.cpp
@@ -19,21 +19,46 @@
 namespace blink {
 
 BluetoothDevice::BluetoothDevice(ExecutionContext* context, PassOwnPtr<WebBluetoothDevice> webDevice)
-    : m_webDevice(webDevice)
+    : ActiveDOMObject(context)
+    , PageLifecycleObserver(toDocument(context)->page())
+    , m_webDevice(webDevice)
     , m_adData(BluetoothAdvertisingData::create(m_webDevice->txPower,
         m_webDevice->rssi))
-    , m_gatt(BluetoothGATTRemoteServer::create(context, m_webDevice->id))
+    , m_gatt(BluetoothGATTRemoteServer::create(m_webDevice->id))
 {
+    // See example in Source/platform/heap/ThreadState.h
+    ThreadState::current()->registerPreFinalizer(this);
 }
 
 BluetoothDevice* BluetoothDevice::take(ScriptPromiseResolver* resolver, PassOwnPtr<WebBluetoothDevice> webDevice)
 {
     ASSERT(webDevice);
-    return new BluetoothDevice(resolver->executionContext(), webDevice);
+    BluetoothDevice* device = new BluetoothDevice(resolver->executionContext(), webDevice);
+    device->suspendIfNeeded();
+    return device;
+}
+
+void BluetoothDevice::dispose()
+{
+    m_gatt->disconnectIfConnected(executionContext());
+}
+
+void BluetoothDevice::stop()
+{
+    m_gatt->disconnectIfConnected(executionContext());
+}
+
+void BluetoothDevice::pageVisibilityChanged()
+{
+    if (!page()->isPageVisible()) {
+        m_gatt->disconnectIfConnected(executionContext());
+    }
 }
 
 DEFINE_TRACE(BluetoothDevice)
 {
+    ActiveDOMObject::trace(visitor);
+    PageLifecycleObserver::trace(visitor);
     visitor->trace(m_adData);
     visitor->trace(m_gatt);
 }
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.h b/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.h
index dcadf71..2ecac27 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.h
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.h
@@ -6,6 +6,8 @@
 #define BluetoothDevice_h
 
 #include "bindings/core/v8/ScriptWrappable.h"
+#include "core/dom/ActiveDOMObject.h"
+#include "core/page/PageLifecycleObserver.h"
 #include "modules/bluetooth/BluetoothAdvertisingData.h"
 #include "modules/bluetooth/BluetoothGATTRemoteServer.h"
 #include "platform/heap/Heap.h"
@@ -28,8 +30,12 @@ class ScriptState;
 // CallbackPromiseAdapter class comments.
 class BluetoothDevice final
     : public GarbageCollectedFinalized<BluetoothDevice>
+    , public ActiveDOMObject
+    , public PageLifecycleObserver
     , public ScriptWrappable {
+    USING_PRE_FINALIZER(BluetoothDevice, dispose);
     DEFINE_WRAPPERTYPEINFO();
+    WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(BluetoothDevice);
 public:
     BluetoothDevice(ExecutionContext*, PassOwnPtr<WebBluetoothDevice>);
 
@@ -37,6 +43,31 @@ public:
     using WebType = OwnPtr<WebBluetoothDevice>;
     static BluetoothDevice* take(ScriptPromiseResolver*, PassOwnPtr<WebBluetoothDevice>);
 
+    // We should disconnect from the device in all of the following cases:
+    // 1. When the object gets GarbageCollected e.g. it went out of scope.
+    // dispose() is called in this case.
+    // 2. When the parent document gets detached e.g. reloading a page.
+    // stop() is called in this case.
+    // 3. For now (https://crbug.com/579746), when the tab is no longer in the
+    // foreground e.g. change tabs.
+    // pageVisibilityChanged() is called in this case.
+    // TODO(ortuno): Users should be able to turn on notifications for
+    // events on navigator.bluetooth and still remain connected even if the
+    // BluetoothDevice object is garbage collected.
+    // TODO(ortuno): Allow connections when the tab is in the background.
+    // This is a short term solution instead of implementing a tab indicator
+    // for bluetooth connections.
+
+    // USING_PRE_FINALIZER interface.
+    // Called before the object gets garbage collected.
+    void dispose();
+
+    // ActiveDOMObject interface.
+    void stop() override;
+
+    // PageLifecycleObserver interface.
+    void pageVisibilityChanged() override;
+
     // Interface required by Garbage Collection:
     DECLARE_VIRTUAL_TRACE();
 
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.idl b/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.idl
index 00442dd..6c1118549 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.idl
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothDevice.idl
@@ -13,10 +13,11 @@ enum VendorIDSource {
 
 [
     GarbageCollected,
+    ActiveDOMObject,
     RuntimeEnabled=WebBluetooth,
 ] interface BluetoothDevice
 // Implement ServiceEventHandlers interface: http://crbug.com/421670
-// : ServiceEventHandlers 
+// : ServiceEventHandlers
 {
 // Implement BluetoothDevice interface: http://crbug.com/421668
     [DeprecateAs=BluetoothDeviceInstanceId, ImplementedAs=id] readonly attribute DOMString instanceID;
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.cpp b/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.cpp
index fa35ea7..64ce1f7 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.cpp
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.cpp
@@ -10,6 +10,7 @@
 #include "core/dom/DOMException.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
+#include "core/page/Page.h"
 #include "modules/bluetooth/BluetoothError.h"
 #include "modules/bluetooth/BluetoothGATTService.h"
 #include "modules/bluetooth/BluetoothSupplement.h"
@@ -19,55 +20,26 @@
 
 namespace blink {
 
-BluetoothGATTRemoteServer::BluetoothGATTRemoteServer(ExecutionContext* context, const String& deviceId)
-    : ActiveDOMObject(context)
-    , PageLifecycleObserver(toDocument(context)->page())
-    , m_deviceId(deviceId)
+BluetoothGATTRemoteServer::BluetoothGATTRemoteServer(const String& deviceId)
+    : m_deviceId(deviceId)
     , m_connected(false)
 {
-    // See example in Source/platform/heap/ThreadState.h
-    ThreadState::current()->registerPreFinalizer(this);
 }
 
-BluetoothGATTRemoteServer* BluetoothGATTRemoteServer::create(ExecutionContext* context, const String& deviceId)
+BluetoothGATTRemoteServer* BluetoothGATTRemoteServer::create(const String& deviceId)
 {
-    BluetoothGATTRemoteServer* server = new BluetoothGATTRemoteServer(context, deviceId);
-    server->suspendIfNeeded();
-    return server;
+    return new BluetoothGATTRemoteServer(deviceId);
 }
 
-void BluetoothGATTRemoteServer::dispose()
-{
-    disconnectIfConnected();
-}
-
-void BluetoothGATTRemoteServer::stop()
-{
-    disconnectIfConnected();
-}
-
-void BluetoothGATTRemoteServer::pageVisibilityChanged()
-{
-    if (!page()->isPageVisible()) {
-        disconnectIfConnected();
-    }
-}
-
-void BluetoothGATTRemoteServer::disconnectIfConnected()
+void BluetoothGATTRemoteServer::disconnectIfConnected(ExecutionContext* context)
 {
     if (m_connected) {
         m_connected = false;
-        WebBluetooth* webbluetooth = BluetoothSupplement::fromExecutionContext(executionContext());
+        WebBluetooth* webbluetooth = BluetoothSupplement::fromExecutionContext(context);
         webbluetooth->disconnect(m_deviceId);
     }
 }
 
-DEFINE_TRACE(BluetoothGATTRemoteServer)
-{
-    ActiveDOMObject::trace(visitor);
-    PageLifecycleObserver::trace(visitor);
-}
-
 class ConnectCallback : public WebBluetoothGATTServerConnectCallbacks {
 public:
     ConnectCallback(BluetoothGATTRemoteServer* gatt, ScriptPromiseResolver* resolver)
@@ -79,8 +51,8 @@ public:
         if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped())
             return;
         m_gatt->setConnected(true);
-        if (!m_gatt->page()->isPageVisible()) {
-            m_gatt->disconnectIfConnected();
+        if (!toDocument(m_resolver->executionContext())->page()->isPageVisible()) {
+            m_gatt->disconnectIfConnected(m_resolver->executionContext());
         }
         m_resolver->resolve(m_gatt);
     }
@@ -102,7 +74,7 @@ ScriptPromise BluetoothGATTRemoteServer::connect(ScriptState* scriptState)
     // This is a short term solution instead of implementing a tab indicator
     // for bluetooth connections.
     // https://crbug.com/579746
-    if (!page()->isPageVisible()) {
+    if (!toDocument(scriptState->executionContext())->page()->isPageVisible()) {
         return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(SecurityError, "Connection is only allowed while the page is visible. This is a temporary measure until we are able to effectively communicate to the user that a page is connected to a device."));
     }
 
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.h b/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.h
index fb7ccd4..08a7b8a 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.h
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.h
@@ -7,8 +7,6 @@
 
 #include "bindings/core/v8/ScriptWrappable.h"
 #include "bindings/modules/v8/UnionTypesModules.h"
-#include "core/dom/ActiveDOMObject.h"
-#include "core/page/PageLifecycleObserver.h"
 #include "platform/heap/Heap.h"
 #include "wtf/OwnPtr.h"
 #include "wtf/PassOwnPtr.h"
@@ -23,47 +21,18 @@ class ScriptState;
 // BluetoothGATTRemoteServer provides a way to interact with a connected bluetooth peripheral.
 class BluetoothGATTRemoteServer final
     : public GarbageCollectedFinalized<BluetoothGATTRemoteServer>
-    , public ActiveDOMObject
-    , public PageLifecycleObserver
     , public ScriptWrappable {
-    USING_PRE_FINALIZER(BluetoothGATTRemoteServer, dispose);
     DEFINE_WRAPPERTYPEINFO();
-    WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(BluetoothGATTRemoteServer);
 public:
-    BluetoothGATTRemoteServer(ExecutionContext*, const String& deviceId);
+    BluetoothGATTRemoteServer(const String& deviceId);
 
-    static BluetoothGATTRemoteServer* create(ExecutionContext*, const String& deviceId);
+    static BluetoothGATTRemoteServer* create(const String& deviceId);
 
-    // We should disconnect from the device in all of the following cases:
-    // 1. When the object gets GarbageCollected e.g. it went out of scope.
-    // dispose() is called in this case.
-    // 2. When the parent document gets detached e.g. reloading a page.
-    // stop() is called in this case.
-    // 3. For now (https://crbug.com/579746), when the tab is no longer in the
-    // foreground e.g. change tabs.
-    // pageVisibilityChanged() is called in this case.
-    // TODO(ortuno): Users should be able to turn on notifications listen for
-    // events on navigator.bluetooth and still remain connected even if the
-    // BluetoothGATTRemoteServer object is garbage collected.
-    // TODO(ortuno): Allow connections when the tab is in the background.
-    // This is a short term solution instead of implementing a tab indicator
-    // for bluetooth connections.
-
-    // USING_PRE_FINALIZER interface.
-    // Called before the object gets garbage collected.
-    void dispose();
-
-    // ActiveDOMObject interface.
-    void stop() override;
-
-    // PageLifecycleObserver interface.
-    void pageVisibilityChanged() override;
-
-    void disconnectIfConnected();
+    void disconnectIfConnected(ExecutionContext*);
     void setConnected(bool connected) { m_connected = connected; }
 
     // Interface required by Garbage Collectoin:
-    DECLARE_VIRTUAL_TRACE();
+    DEFINE_INLINE_TRACE() { }
 
     // IDL exposed interface:
     bool connected() { return m_connected; }
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.idl b/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.idl
index 206a3a8..1e523a5 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.idl
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.idl
@@ -8,7 +8,6 @@
 
 [
     GarbageCollected,
-    ActiveDOMObject,
     RuntimeEnabled=WebBluetooth,
 ] interface BluetoothGATTRemoteServer
 // Implement ServiceEventHandlers interface: http://crbug.com/421670