Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(994)

Unified Diff: third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp

Issue 2025783002: Worker: Introduce an observation mechanism for WorkerThread termination (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@delayed_task
Patch Set: update implementation comments Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..d950c25b07e3ab3f32869fac8f948a5af174aa0f 100644
--- a/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp
+++ b/third_party/WebKit/Source/modules/websockets/WorkerWebSocketChannel.cpp
@@ -177,26 +177,29 @@ 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()
{
- ASSERT(!isMainThread());
+ DCHECK(isMainThread());
}
-void Peer::initialize(PassOwnPtr<SourceLocation> location, ExecutionContext* context)
+bool Peer::initialize(PassOwnPtr<SourceLocation> location, ExecutionContext* context)
{
ASSERT(isMainThread());
+ if (wasContextDestroyedBeforeObserverCreation())
+ return false;
Document* document = toDocument(context);
m_mainWebSocketChannel = DocumentWebSocketChannel::create(document, this, std::move(location));
- m_syncHelper->signalWorkerThread();
+ return true;
}
void Peer::connect(const KURL& url, const String& protocol)
@@ -357,12 +360,23 @@ void Peer::didError()
m_loaderProxy->postTaskToWorkerGlobalScope(createCrossThreadTask(&workerGlobalScopeDidError, m_bridge));
}
+void Peer::contextDestroyed()
+{
+ DCHECK(isMainThread());
+ if (m_mainWebSocketChannel) {
+ m_mainWebSocketChannel->disconnect();
+ m_mainWebSocketChannel = nullptr;
+ }
+ m_bridge = nullptr;
+}
+
DEFINE_TRACE(Peer)
{
visitor->trace(m_bridge);
yhirano 2016/06/09 11:36:20 Peer::m_bridge is not Member. Should we keep this?
nhiroki 2016/06/09 13:23:51 Removed.
visitor->trace(m_mainWebSocketChannel);
visitor->trace(m_syncHelper);
WebSocketChannelClient::trace(visitor);
+ WorkerThreadLifecycleObserver::trace(visitor);
}
Bridge::Bridge(WebSocketChannelClient* client, WorkerGlobalScope& workerGlobalScope)
@@ -370,7 +384,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 +392,21 @@ Bridge::~Bridge()
ASSERT(!m_peer);
}
+void Bridge::createPeerOnMainThread(PassOwnPtr<SourceLocation> location, WorkerThreadContext* workerThreadContext, ExecutionContext* context)
+{
+ DCHECK(isMainThread());
+ DCHECK(!m_peer);
+ Peer* peer = new Peer(this, m_loaderProxy, m_syncHelper, workerThreadContext);
+ if (peer->initialize(std::move(location), context))
+ m_peer = peer;
+ m_syncHelper->signalWorkerThread();
+}
+
void Bridge::initialize(PassOwnPtr<SourceLocation> location)
{
- if (!waitForMethodCompletion(createCrossThreadTask(&Peer::initialize, wrapCrossThreadPersistent(m_peer.get()), passed(std::move(location))))) {
+ // Wait for completion of the task on the main thread because the connection
+ // must synchronously be established (see Bridge::connect).
+ 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 +417,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,8 +469,9 @@ void Bridge::disconnect()
if (!m_peer)
return;
+ // Wait for completion of the task on the main thread to ensure that
+ // |m_peer| does not touch this Bridge object after this point.
waitForMethodCompletion(createCrossThreadTask(&Peer::disconnect, wrapCrossThreadPersistent(m_peer.get())));
- // Here |m_peer| is detached from the main thread and we can delete it.
m_client = nullptr;
m_peer = nullptr;

Powered by Google App Engine
This is Rietveld 408576698