Remove the concept of a cleanup task

Source/core/workers/WorkerThread.cpp

Index: Source/core/workers/WorkerThread.cpp
diff --git a/Source/core/workers/WorkerThread.cpp b/Source/core/workers/WorkerThread.cpp
index f59a49182a66ff3dcd4b57653f081d209767d181..a1ccd11e58ddb807a2d62003bfd40c71010ce50d 100644
--- a/Source/core/workers/WorkerThread.cpp
+++ b/Source/core/workers/WorkerThread.cpp
@@ -46,6 +46,7 @@
 #include "platform/heap/ThreadState.h"
 #include "platform/weborigin/KURL.h"
 #include "public/platform/Platform.h"
+#include "public/platform/WebScheduler.h"
 #include "public/platform/WebThread.h"
 #include "public/platform/WebWaitableEvent.h"
 #include "wtf/Noncopyable.h"
@@ -72,6 +73,8 @@ public:
         if (WorkerGlobalScope* globalScope = m_workerThread->workerGlobalScope()) {
             if (WorkerScriptController* scriptController = globalScope->script())
                 scriptController->rejectedPromises()->processQueue();
+            if (globalScope->isClosing())
+                m_workerThread->workerReportingProxy().workerGlobalScopeClosed();
         }
     }
 
@@ -208,15 +211,11 @@ public:
     virtual void run() override
     {
         WorkerGlobalScope* workerGlobalScope = m_workerThread.workerGlobalScope();
-        // Tasks could be put on the message loop after the cleanup task,
-        // ensure none of those are ran.
-        if (!workerGlobalScope)
-            return;
+        ASSERT(workerGlobalScope);
 
         if (m_isInstrumented)
             InspectorInstrumentation::willPerformExecutionContextTask(workerGlobalScope, m_task.get());
-        if ((!workerGlobalScope->isClosing() && !m_workerThread.terminated()) || m_task->isCleanupTask())
-            m_task->performTask(workerGlobalScope);
+        m_task->performTask(workerGlobalScope);
         if (m_isInstrumented)
             InspectorInstrumentation::didPerformExecutionContextTask(workerGlobalScope);
     }
@@ -362,6 +361,9 @@ void WorkerThread::cleanup()
     // This should be called before we start the shutdown procedure.
     workerReportingProxy().willDestroyWorkerGlobalScope();
 
+    // Ensure no posted tasks will run from this point on.
+    m_thread->platformThread().scheduler()->shutdown();
+
     // The below assignment will destroy the context, which will in turn notify messaging proxy.
     // We cannot let any objects survive past thread exit, because no other thread will run GC or otherwise destroy them.
     // If Oilpan is enabled, we detach of the context/global scope, with the final heap cleanup below sweeping it out.
@@ -387,51 +389,6 @@ void WorkerThread::cleanup()
     PlatformThreadData::current().destroy();
 }
 
-class WorkerThreadShutdownFinishTask : public ExecutionContextTask {
-public:
-    static PassOwnPtr<WorkerThreadShutdownFinishTask> create()
-    {
-        return adoptPtr(new WorkerThreadShutdownFinishTask());
-    }
-
-    virtual void performTask(ExecutionContext *context)
-    {
-        WorkerGlobalScope* workerGlobalScope = toWorkerGlobalScope(context);
-        workerGlobalScope->dispose();
-
-        WorkerThread* workerThread = workerGlobalScope->thread();
-        workerThread->willDestroyIsolate();
-        workerThread->m_thread->postTask(FROM_HERE, new Task(WTF::bind(&WorkerThread::cleanup, workerThread)));
-    }
-
-    virtual bool isCleanupTask() const { return true; }
-};
-
-class WorkerThreadShutdownStartTask : public ExecutionContextTask {
-public:
-    static PassOwnPtr<WorkerThreadShutdownStartTask> create()
-    {
-        return adoptPtr(new WorkerThreadShutdownStartTask());
-    }
-
-    virtual void performTask(ExecutionContext *context)
-    {
-        WorkerGlobalScope* workerGlobalScope = toWorkerGlobalScope(context);
-        workerGlobalScope->stopActiveDOMObjects();
-        PlatformThreadData::current().threadTimers().setSharedTimer(nullptr);
-
-        // Event listeners would keep DOMWrapperWorld objects alive for too long. Also, they have references to JS objects,
-        // which become dangling once Heap is destroyed.
-        workerGlobalScope->removeAllEventListeners();
-
-        // Stick a shutdown command at the end of the queue, so that we deal
-        // with all the cleanup tasks the databases post first.
-        workerGlobalScope->postTask(FROM_HERE, WorkerThreadShutdownFinishTask::create());
-    }
-
-    virtual bool isCleanupTask() const { return true; }
-};
-
 void WorkerThread::stop()
 {
     // Prevent the deadlock between GC and an attempt to stop a thread.
@@ -478,7 +435,17 @@ void WorkerThread::stopInternal()
 
     InspectorInstrumentation::didKillAllExecutionContextTasks(m_workerGlobalScope.get());
     m_debuggerMessageQueue.kill();
-    postTask(FROM_HERE, WorkerThreadShutdownStartTask::create());
+
+    m_workerGlobalScope->stopActiveDOMObjects();
+    PlatformThreadData::current().threadTimers().setSharedTimer(nullptr);
+
+    // Event listeners would keep DOMWrapperWorld objects alive for too long. Also, they have references to JS objects,
+    // which become dangling once Heap is destroyed.
+    m_workerGlobalScope->removeAllEventListeners();
+    m_workerGlobalScope->dispose();
+
+    willDestroyIsolate();
+    cleanup();

[kinuko, 2015/04/30 04:03:44]

stopInternal() is called on the main thread while most of these need to run on
worker thread, we'll at least need to run one more task to do worker-thread side
cleanup.

[Sami, 2015/05/06 17:37:32]

Thanks for the hint! It took a while for me to work out the thread hop sequence
during shutdown. Could you check that the latest version looks consistent?

 }
 
 void WorkerThread::didStartRunLoop()