Worker: Introduce an observation mechanism for WorkerThread termination

third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp

Index: third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp
diff --git a/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp b/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp
index 50bba63302bd43bdae2121cf5c017139ff0d082a..735544a764848784938fb3038d679a08f7dbc663 100644
--- a/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp
+++ b/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp
@@ -177,13 +177,14 @@ DEFINE_TRACE(WorkerWebSocketChannel)
     WebSocketChannel::trace(visitor);
 }
 
-Peer::Peer(Bridge* bridge, PassRefPtr<WorkerLoaderProxy> loaderProxy, WebSocketChannelSyncHelper* syncHelper)
-    : m_bridge(bridge)
+Peer::Peer(Bridge* bridge, PassRefPtr<WorkerLoaderProxy> loaderProxy, WebSocketChannelSyncHelper* syncHelper, WorkerThreadContext* workerThreadContext)
+    : WorkerThreadLifecycleObserver(workerThreadContext)
+    , m_bridge(bridge)
     , m_loaderProxy(loaderProxy)
     , m_mainWebSocketChannel(nullptr)
     , m_syncHelper(syncHelper)
 {
-    ASSERT(!isMainThread());
+    DCHECK(isMainThread());
 }
 
 Peer::~Peer()
@@ -194,8 +195,10 @@ Peer::~Peer()
 void Peer::initialize(PassOwnPtr<SourceLocation> location, ExecutionContext* context)
 {
     ASSERT(isMainThread());
-    Document* document = toDocument(context);
-    m_mainWebSocketChannel = DocumentWebSocketChannel::create(document, this, std::move(location));
+    if (!wasContextDestroyedBeforeObserverCreation()) {
+        Document* document = toDocument(context);
+        m_mainWebSocketChannel = DocumentWebSocketChannel::create(document, this, std::move(location));
+    }
     m_syncHelper->signalWorkerThread();
 }
 
@@ -357,12 +360,22 @@ void Peer::didError()
     m_loaderProxy->postTaskToWorkerGlobalScope(createCrossThreadTask(&workerGlobalScopeDidError, m_bridge));
 }
 
+void Peer::contextDestroyed()
+{
+    DCHECK(isMainThread());
+    if (m_mainWebSocketChannel) {
+        m_mainWebSocketChannel->disconnect();
+        m_mainWebSocketChannel = nullptr;
+    }

[yhirano, 2016/06/09 04:55:13]

m_bridge = nullptr;

[nhiroki, 2016/06/09 07:37:07]

Done.

+}
+
 DEFINE_TRACE(Peer)
 {
     visitor->trace(m_bridge);
     visitor->trace(m_mainWebSocketChannel);
     visitor->trace(m_syncHelper);
     WebSocketChannelClient::trace(visitor);
+    WorkerThreadLifecycleObserver::trace(visitor);
 }
 
 Bridge::Bridge(WebSocketChannelClient* client, WorkerGlobalScope& workerGlobalScope)
@@ -370,7 +383,6 @@ Bridge::Bridge(WebSocketChannelClient* client, WorkerGlobalScope& workerGlobalSc
     , m_workerGlobalScope(workerGlobalScope)
     , m_loaderProxy(m_workerGlobalScope->thread()->workerLoaderProxy())
     , m_syncHelper(WebSocketChannelSyncHelper::create(adoptPtr(new WaitableEvent())))
-    , m_peer(new Peer(this, m_loaderProxy, m_syncHelper))
 {
 }
 
@@ -379,9 +391,23 @@ Bridge::~Bridge()
     ASSERT(!m_peer);
 }
 
+void Bridge::createPeerOnMainThread(PassOwnPtr<SourceLocation> location, WorkerThreadContext* workerThreadContext, ExecutionContext* context)
+{
+    DCHECK(isMainThread());

[yhirano, 2016/06/09 04:55:13]

+DCHECK(!m_peer);

[nhiroki, 2016/06/09 07:37:07]

Done.

+    m_peer = new Peer(this, m_loaderProxy, m_syncHelper, workerThreadContext);
+    m_peer->initialize(std::move(location), context);

[yhirano, 2016/06/09 04:55:13]

[optional] Is it good to set |m_peer| only when necessary?

Peer* peer = new Peer(...);
if (peer->initialize(...))
  m_peer = peer;
m_syncHelper->signalWorkerThread();

[nhiroki, 2016/06/09 07:37:06]

Done.

+}
+
 void Bridge::initialize(PassOwnPtr<SourceLocation> location)
 {
-    if (!waitForMethodCompletion(createCrossThreadTask(&Peer::initialize, wrapCrossThreadPersistent(m_peer.get()), passed(std::move(location))))) {
+    // TODO(nhiroki): Stop waiting for completion of the task on the main
+    // thread and make this function async instead. This wait was necessary to
+    // prevent worker thread shutdown during initialization on the main thread.
+    // If it happens, the main thread may retain dangling pointers to objects on
+    // the worker thread. However, in the current implementation, the shutdown
+    // sequence always goes through the main thread and gives a chance to
+    // release pointers before they dangle. See Peer::contextDestroyed.
+    if (!waitForMethodCompletion(createCrossThreadTask(&Bridge::createPeerOnMainThread, wrapCrossThreadPersistent(this), passed(std::move(location)), wrapCrossThreadPersistent(m_workerGlobalScope->thread()->workerThreadContext())))) {
         // The worker thread has been signalled to shutdown before method completion.
         disconnect();
     }
@@ -392,6 +418,8 @@ bool Bridge::connect(const KURL& url, const String& protocol)
     if (!m_peer)
         return false;
 
+    // Wait for completion of the task on the main thread because the mixed
+    // content check must synchronously be conducted.
     if (!waitForMethodCompletion(createCrossThreadTask(&Peer::connect, wrapCrossThreadPersistent(m_peer.get()), url, protocol)))
         return false;
 
@@ -442,6 +470,9 @@ void Bridge::disconnect()
     if (!m_peer)
         return;
 
+    // TODO(nhiroki): Stop waiting for completion of the task on the main thread
+    // and make this function async instead. See a comment on Bridge::initialize
+    // for more details.
     waitForMethodCompletion(createCrossThreadTask(&Peer::disconnect, wrapCrossThreadPersistent(m_peer.get())));
     // Here |m_peer| is detached from the main thread and we can delete it.