bluetooth: Disconnect if the page becomes hidden or is closed.

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

Index: third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.cpp
diff --git a/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.cpp b/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.cpp
index 862d903186b11a82d6c2120023a66aad0d3e2c91..6fc6b271fe56ad4caab73ec841e8055ae042137e 100644
--- a/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.cpp
+++ b/third_party/WebKit/Source/modules/bluetooth/BluetoothGATTRemoteServer.cpp
@@ -8,6 +8,7 @@
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMException.h"
+#include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "modules/bluetooth/BluetoothError.h"
 #include "modules/bluetooth/BluetoothGATTService.h"
@@ -17,16 +18,64 @@
 #include "wtf/OwnPtr.h"
 
 namespace blink {
+namespace {
 
-BluetoothGATTRemoteServer::BluetoothGATTRemoteServer(PassOwnPtr<WebBluetoothGATTRemoteServer> webGATT)
-    : m_webGATT(webGATT)
+bool isPageVisible(Page* page)

[haraken, 2016/01/23 02:02:02]

Can we add this helper function to Page? (i.e., Page::isVisible())

[ortuno, 2016/01/25 23:55:07]

done.

 {
+    return page->visibilityState() == PageVisibilityStateVisible;
 }
 
-BluetoothGATTRemoteServer* BluetoothGATTRemoteServer::take(ScriptPromiseResolver*, PassOwnPtr<WebBluetoothGATTRemoteServer> webGATT)
+} // namespace
+BluetoothGATTRemoteServer::BluetoothGATTRemoteServer(ExecutionContext* context, PassOwnPtr<WebBluetoothGATTRemoteServer> webGATT)
+    : ActiveDOMObject(context)
+    , PageLifecycleObserver(toDocument(context)->page())
+    , m_webGATT(webGATT)
+{
+    // See example in Source/platform/heap/ThreadState.h
+    ThreadState::current()->registerPreFinalizer(this);
+}
+
+BluetoothGATTRemoteServer* BluetoothGATTRemoteServer::take(ScriptPromiseResolver* resolver, PassOwnPtr<WebBluetoothGATTRemoteServer> webGATT)
 {
     ASSERT(webGATT);
-    return new BluetoothGATTRemoteServer(webGATT);
+    BluetoothGATTRemoteServer* server = new BluetoothGATTRemoteServer(resolver->executionContext(), webGATT);
+    if (!isPageVisible(server->page())) {
+        server->disconnectIfConnected();
+    }
+    server->suspendIfNeeded();
+    return server;
+}
+
+void BluetoothGATTRemoteServer::dispose()
+{
+    disconnectIfConnected();
+}
+
+void BluetoothGATTRemoteServer::stop()
+{
+    disconnectIfConnected();
+}
+
+void BluetoothGATTRemoteServer::pageVisibilityChanged()
+{
+    if (!isPageVisible(page())) {
+        disconnectIfConnected();
+    }
+}
+
+void BluetoothGATTRemoteServer::disconnectIfConnected()
+{
+    if (m_webGATT->connected) {
+        m_webGATT->connected = false;
+        WebBluetooth* webbluetooth = BluetoothSupplement::fromExecutionContext(executionContext());

[haraken, 2016/01/23 02:02:02]

I think we've discussed this before, but it's a bit nasty to have
BluetoothSupplement::fromExecutionContext and
BluetoothSupplement::fromScriptState. Given that BluetoothSupplement is a
supplement of a Frame, it should provide only
BluetoothSupplement::from(LocalFrame*).

The caller side (e.g., BluetoothGATTRemoteServer) should hold a Member<Frame>
and use BluetoothSupplement::from(m_frame) to retrieve the supplement.

(In the case of BluetoothGATTRemoteServer, it doesn't need to hold m_frame
because it can retrieve the frame from executionContext(). It can just provide
BluetoothGATTRemoteServer::frame() which returns the frame associated with
executionContext(). Then it can call BluetoothSupplement::from(frame())).

(You don't need to address this in this CL.)

[Jeffrey Yasskin, 2016/01/23 02:33:36]

FWIW, I disagree. There are uses of BluetoothSupplement in 5 different files,
none of which have a frame immediately available. They all have either a
ScriptState or an ExecutionContext, and accepting these in methods on
BluetoothSupplement allows us to centralize the non-trivial knowledge of how to
get from the object we have to the Frame. I don't want to duplicate this code.

+        webbluetooth->disconnect(m_webGATT->deviceId);
+    }
+}
+
+DEFINE_TRACE(BluetoothGATTRemoteServer)
+{
+    ActiveDOMObject::trace(visitor);
+    PageLifecycleObserver::trace(visitor);
 }
 
 void BluetoothGATTRemoteServer::disconnect(ScriptState* scriptState)