workers: Change how debugger-tasks are posted to the thread.

Source/core/workers/WorkerInspectorProxy.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 
 #include "core/workers/WorkerInspectorProxy.h"
 
 #include "core/dom/CrossThreadTask.h"
 #include "core/inspector/InspectorInstrumentation.h"
 #include "core/inspector/InspectorTraceEvents.h"
 #include "core/inspector/WorkerInspectorController.h"
 #include "core/workers/WorkerThread.h"
+#include "platform/Task.h"
 #include "platform/TraceEvent.h"
 #include "platform/weborigin/KURL.h"
+#include "public/platform/WebTraceLocation.h"
 
 namespace blink {
 
 WorkerInspectorProxy::WorkerInspectorProxy()
     : m_workerThread(nullptr)
     , m_executionContext(nullptr)
     , m_pageInspector(nullptr)
     , m_workerGlobalScopeProxy(nullptr)
 {
 }
@@ skipping 15 matching lines @@
 }
 
 void WorkerInspectorProxy::workerThreadTerminated()
 {
     if (m_workerThread)
         InspectorInstrumentation::workerTerminated(m_executionContext, this);
     m_workerThread = nullptr;
     m_pageInspector = nullptr;
 }
 
-static void connectToWorkerGlobalScopeInspectorTask(ExecutionContext* context)
+static void connectToWorkerGlobalScopeInspectorTask(WorkerThread* workerThread)
 {
-    toWorkerGlobalScope(context)->workerInspectorController()->connectFrontend();
+    workerThread->workerGlobalScope()->workerInspectorController()->connectFrontend();
 }
 
 void WorkerInspectorProxy::connectToInspector(WorkerInspectorProxy::PageInspector* pageInspector)
 {
     if (!m_workerThread)
         return;
     ASSERT(!m_pageInspector);
     m_pageInspector = pageInspector;
-    m_workerThread->postDebuggerTask(FROM_HERE, createCrossThreadTask(connectToWorkerGlobalScopeInspectorTask));
+    addDebuggerTaskForWorker(FROM_HERE, adoptPtr(new Task(threadSafeBind(connectToWorkerGlobalScopeInspectorTask, AllowCrossThreadAccess(m_workerThread)))));
 }
 
-static void disconnectFromWorkerGlobalScopeInspectorTask(ExecutionContext* context)
+static void disconnectFromWorkerGlobalScopeInspectorTask(WorkerThread* workerThread)
 {
-    toWorkerGlobalScope(context)->workerInspectorController()->disconnectFrontend();
+    workerThread->workerGlobalScope()->workerInspectorController()->disconnectFrontend();
 }
 
 void WorkerInspectorProxy::disconnectFromInspector()
 {
     m_pageInspector = nullptr;
     if (!m_workerThread)
         return;
-    m_workerThread->postDebuggerTask(FROM_HERE, createCrossThreadTask(disconnectFromWorkerGlobalScopeInspectorTask));
+    addDebuggerTaskForWorker(FROM_HERE, adoptPtr(new Task(threadSafeBind(disconnectFromWorkerGlobalScopeInspectorTask, AllowCrossThreadAccess(m_workerThread)))));
 }
 
-static void dispatchOnInspectorBackendTask(const String& message, ExecutionContext* context)
+static void dispatchOnInspectorBackendTask(const String& message, WorkerThread* workerThread)
 {
-    toWorkerGlobalScope(context)->workerInspectorController()->dispatchMessageFromFrontend(message);
+    workerThread->workerGlobalScope()->workerInspectorController()->dispatchMessageFromFrontend(message);

[yurys, 2015/05/15 23:41:41]

Why is it safe to access workerGlobalScope here? It seems to me that the task
might get executed on the platform thread when the WorkerGlobalScope is already
disposed.

[sadrul, 2015/05/16 00:50:53]

The WorkerGlobalScope gets destroyed at the same time as the thread
(WorkerThread::shutdown()). So if the task is executing, the global-scope is
also still alive.

 }
 
 void WorkerInspectorProxy::sendMessageToInspector(const String& message)
 {
     if (!m_workerThread)
         return;
-    m_workerThread->postDebuggerTask(FROM_HERE, createCrossThreadTask(dispatchOnInspectorBackendTask, message));
+    addDebuggerTaskForWorker(FROM_HERE, adoptPtr(new Task(threadSafeBind(dispatchOnInspectorBackendTask, message, AllowCrossThreadAccess(m_workerThread)))));
     m_workerThread->interruptAndDispatchInspectorCommands();
 }
 
 void WorkerInspectorProxy::writeTimelineStartedEvent(const String& sessionId, const String& workerId)
 {
     if (!m_workerThread)
         return;
     TRACE_EVENT_INSTANT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "TracingSessionIdForWorker", TRACE_EVENT_SCOPE_THREAD, "data", InspectorTracingSessionIdForWorkerEvent::data(sessionId, workerId, m_workerThread));
 }
 
+static void runDebuggerTaskForWorker(WorkerThread* workerThread)
+{
+    workerThread->runDebuggerTask(WorkerThread::DontWaitForMessage);
+}
+
+void WorkerInspectorProxy::addDebuggerTaskForWorker(const WebTraceLocation& location, PassOwnPtr<WebThread::Task> task)
+{
+    m_workerThread->appendDebuggerTask(task);
+    m_workerThread->backingThread().postTask(location, new Task(threadSafeBind(&runDebuggerTaskForWorker, AllowCrossThreadAccess(m_workerThread))));

[kinuko, 2015/05/18 15:19:49]

Could this be just threadSafeBind(&WorkerThread::runDebuggerTask,
AllowCrossThreadAccess(m_workerThread))

[sadrul, 2015/05/19 03:50:47]

backingThread() returns a WebThreadSupportingGC, which doesn't have a postTask()
version that takes a Function.

I am tempted to expose a WorkerThread::backingPlatformThread() (or just
WorkerThread::platformThread()?) that returns the WebThread, instead of
WebThreadSupportingGC. With that change, we won't need the Task to be explicitly
created here. WDYT?

[sadrul, 2015/05/19 16:23:06]

I have made this change (moved WorkerThread::backingThread() to private,
introduced a public WorkerThread::platformThread(), which uses the result from
backingThread(), and removed WorkerThread::platformThreadId() in favour of the
new function). PTAL

[kinuko, 2015/05/20 08:17:46]

Sorry, I should have made it clearer.  I meant to ask if we could remove the
indirection method, i.e. runDebuggerTaskForWorker, like following:

new Task(threadSafeBind(&WorkerThread::runDebuggerTask,
AllowCrossThreadAccess(m_workerThread), WorkerThread::DontWaitForMessage))

(...but then now I realized that runDebuggerTask returns non-void value which
makes compiling the above code fail)

+}
+
 } // namespace blink