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

Unified Diff: Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp

Issue 368453003: [WebSocket] Task creation should be separated from task posting. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@crash
Patch Set: Created 6 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
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp
diff --git a/Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp b/Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp
index 75244e242f38aa5785fc5c3602260d98c36287fc..8bceaa9aba3087e64b32e91f254fc999e9d3b5d0 100644
--- a/Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp
+++ b/Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp
@@ -455,10 +455,16 @@ void WorkerThreadableWebSocketChannel::Bridge::initialize(const String& sourceUR
RefPtrWillBeRawPtr<Bridge> protect(this);
#if ENABLE(OILPAN)
- if (!waitForMethodCompletion(createCallbackTask(&Peer::initialize, &m_peer, AllowCrossThreadAccess(&m_loaderProxy), m_workerClientWrapper.get(), sourceURL, lineNumber, syncHelper.get()))) {
+ // In order to assure all temporally objects to be destroyed before
+ // posting the task, we should bind the task. In other words, it is
+ // dangerous to have a complicated expression as a waitForMethodCompletion
+ // argument.
+ OwnPtr<ExecutionContextTask> task = createCallbackTask(&Peer::initialize, &m_peer, AllowCrossThreadAccess(&m_loaderProxy), m_workerClientWrapper.get(), sourceURL, lineNumber, syncHelper.get());
#else
- if (!waitForMethodCompletion(createCallbackTask(&Peer::initialize, reference, AllowCrossThreadAccess(&m_loaderProxy), m_workerClientWrapper.get(), sourceURL, lineNumber, syncHelper.get()))) {
+ // See the above comment.
+ OwnPtr<ExecutionContextTask> task = createCallbackTask(&Peer::initialize, reference, AllowCrossThreadAccess(&m_loaderProxy), m_workerClientWrapper.get(), sourceURL, lineNumber, syncHelper.get());
#endif
+ if (!waitForMethodCompletion(task.release())) {
// The worker thread has been signalled to shutdown before method completion.
disconnect();
}
@@ -470,7 +476,10 @@ bool WorkerThreadableWebSocketChannel::Bridge::connect(const KURL& url, const St
return false;
RefPtrWillBeRawPtr<Bridge> protect(this);
- if (!waitForMethodCompletion(CallClosureTask::create(bind(&Peer::connect, m_peer, url.copy(), protocol.isolatedCopy()))))
+ // It is important to seprate the task creation from the
+ // waitForMethodCompletion call. See the above comment.
+ OwnPtr<ExecutionContextTask> task = CallClosureTask::create(bind(&Peer::connect, m_peer, url.copy(), protocol.isolatedCopy()));
+ if (!waitForMethodCompletion(task.release()))
return false;
return m_syncHelper->connectRequestResult();
@@ -482,7 +491,10 @@ WebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(cons
return WebSocketChannel::SendFail;
RefPtrWillBeRawPtr<Bridge> protect(this);
- if (!waitForMethodCompletion(CallClosureTask::create(bind(&Peer::send, m_peer, message.isolatedCopy()))))
+ // It is important to seprate the task creation from the
+ // waitForMethodCompletion call. See the above comment.
+ OwnPtr<ExecutionContextTask> task = CallClosureTask::create(bind(&Peer::send, m_peer, message.isolatedCopy()));
+ if (!waitForMethodCompletion(task.release()))
return WebSocketChannel::SendFail;
return m_syncHelper->sendRequestResult();
@@ -499,7 +511,10 @@ WebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(cons
memcpy(data->data(), static_cast<const char*>(binaryData.data()) + byteOffset, byteLength);
RefPtrWillBeRawPtr<Bridge> protect(this);
- if (!waitForMethodCompletion(CallClosureTask::create(bind(&Peer::sendArrayBuffer, m_peer, data.release()))))
+ // It is important to seprate the task creation from the
+ // waitForMethodCompletion call. See the above comment.
+ OwnPtr<ExecutionContextTask> task = CallClosureTask::create(bind(&Peer::sendArrayBuffer, m_peer, data.release()));
+ if (!waitForMethodCompletion(task.release()))
return WebSocketChannel::SendFail;
return m_syncHelper->sendRequestResult();
@@ -511,7 +526,10 @@ WebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(Pass
return WebSocketChannel::SendFail;
RefPtrWillBeRawPtr<Bridge> protect(this);
- if (!waitForMethodCompletion(CallClosureTask::create(bind(&Peer::sendBlob, m_peer, data))))
+ // It is important to seprate the task creation from the
+ // waitForMethodCompletion call. See the above comment.
+ OwnPtr<ExecutionContextTask> task = CallClosureTask::create(bind(&Peer::sendBlob, m_peer, data));
+ if (!waitForMethodCompletion(task.release()))
return WebSocketChannel::SendFail;
return m_syncHelper->sendRequestResult();
@@ -569,14 +587,18 @@ bool WorkerThreadableWebSocketChannel::Bridge::waitForMethodCompletion(PassOwnPt
void WorkerThreadableWebSocketChannel::Bridge::terminatePeer()
{
ASSERT(!hasTerminatedPeer());
+
+ // It is important to seprate the task creation from the
+ // waitForMethodCompletion call. See the above comment.
+ OwnPtr<ExecutionContextTask> task = CallClosureTask::create(bind(&Peer::destroy, m_peer));
#if ENABLE(OILPAN)
// The worker thread has to wait for the main thread to complete Peer::destroy,
// because the worker thread has to make sure that the main thread does not have any
// references to on-heap objects allocated in the thread heap of the worker thread
// before the worker thread shuts down.
- waitForMethodCompletion(CallClosureTask::create(bind(&Peer::destroy, m_peer)));
+ waitForMethodCompletion(task.release());
#else
- m_loaderProxy.postTaskToLoader(CallClosureTask::create(bind(&Peer::destroy, m_peer)));
+ m_loaderProxy.postTaskToLoader(task.release());
#endif
// Peer::destroy() deletes m_peer and then m_syncHelper will be released.
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698