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 d8f75cc105c1285c6f3df3d80327ff396782a07a..2517ff4e8357f869a538809ebced495bba3ee7bb 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"
@@ -58,20 +59,36 @@ namespace {
 const int64_t kShortIdleHandlerDelayMs = 1000;
 const int64_t kLongIdleHandlerDelayMs = 10*1000;
 
-class MicrotaskRunner : public WebThread::TaskObserver {
+} // namespace
+
+class WorkerMicrotaskRunner : public WebThread::TaskObserver {
 public:
-    explicit MicrotaskRunner(WorkerThread* workerThread)
+    explicit WorkerMicrotaskRunner(WorkerThread* workerThread)
         : m_workerThread(workerThread)
+        , m_wasClosed(false)
+    {
+    }
+
+    virtual void willProcessTask() override
     {
+        // No tasks should get executed after we have closed.
+        ASSERT(!m_wasClosed);
+        fprintf(stderr, "%ld: WorkerThread: will process task\n", Platform::current()->currentThread()->threadId());
     }
 
-    virtual void willProcessTask() override { }
     virtual void didProcessTask() override
     {
         Microtask::performCheckpoint();
         if (WorkerGlobalScope* globalScope = m_workerThread->workerGlobalScope()) {
             if (WorkerScriptController* scriptController = globalScope->script())
                 scriptController->rejectedPromises()->processQueue();
+            if (globalScope->isClosing()) {
+                m_wasClosed = true;
+                fprintf(stderr, "GLOBAL SCOPE CLOSING\n");
+                m_workerThread->workerReportingProxy().workerGlobalScopeClosed();
+                m_workerThread->cleanup();
+                fprintf(stderr, "SCOPE CLOSED\n");
+            }
         }
     }
 
@@ -79,9 +96,9 @@ private:
     // Thread owns the microtask runner; reference remains
     // valid for the lifetime of this object.
     WorkerThread* m_workerThread;
-};
 
-} // namespace
+    bool m_wasClosed;
+};
 
 static Mutex& threadSetMutex()
 {
@@ -208,15 +225,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);
     }
@@ -319,7 +332,7 @@ void WorkerThread::initialize()
             return;
         }
 
-        m_microtaskRunner = adoptPtr(new MicrotaskRunner(this));
+        m_microtaskRunner = adoptPtr(new WorkerMicrotaskRunner(this));
         backingThread().addTaskObserver(m_microtaskRunner.get());
         backingThread().attachGC();
 
@@ -354,6 +367,18 @@ void WorkerThread::initialize()
 
 void WorkerThread::cleanup()
 {
+    fprintf(stderr, "%ld: WorkerThread::cleanup\n", Platform::current()->currentThread()->threadId());
+    ASSERT(isCurrentThread());
+    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();
+    workerGlobalScope()->dispose();
+
+    willDestroyIsolate();
+
     // This should be called before we start the shutdown procedure.
     workerReportingProxy().willDestroyWorkerGlobalScope();
 
@@ -372,6 +397,9 @@ void WorkerThread::cleanup()
     backingThread().removeTaskObserver(m_microtaskRunner.get());
     m_microtaskRunner = nullptr;
 
+    // Ensure no posted tasks will run from this point on.
+    backingThread().platformThread().scheduler()->shutdown();
+
     // Notify the proxy that the WorkerGlobalScope has been disposed of.
     // This can free this thread object, hence it must not be touched afterwards.
     workerReportingProxy().workerThreadTerminated();
@@ -380,55 +408,13 @@ void WorkerThread::cleanup()
 
     // Clean up PlatformThreadData before WTF::WTFThreadData goes away!
     PlatformThreadData::current().destroy();
+    fprintf(stderr, "%ld: WorkerThread::cleanup done\n", Platform::current()->currentThread()->threadId());
 }
 
-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->backingThread().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; }

[alex clarke (OOO till 29th), 2015/05/05 17:45:05]

I wonder if you can now remove this from ExecutionContextTask too?

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

Did you mean isCleanupTask()? Yes, there's no need for it so I already took it
out.

-};
 
 void WorkerThread::stop()
 {
+    fprintf(stderr, "%ld: WorkerThread::stop\n", Platform::current()->currentThread()->threadId());
     // Prevent the deadlock between GC and an attempt to stop a thread.
     SafePointScope safePointScope(ThreadState::HeapPointersOnStack);
     stopInternal();
@@ -436,11 +422,13 @@ void WorkerThread::stop()
 
 void WorkerThread::stopInShutdownSequence()
 {
+    fprintf(stderr, "%ld: WorkerThread::stopInShutdownSequence\n", Platform::current()->currentThread()->threadId());
     stopInternal();
 }
 
 void WorkerThread::terminateAndWait()
 {
+    fprintf(stderr, "%ld: WorkerThread::terminateAndWait\n", Platform::current()->currentThread()->threadId());
     stop();
     m_terminationEvent->wait();
 }
@@ -453,6 +441,8 @@ bool WorkerThread::terminated()
 
 void WorkerThread::stopInternal()
 {
+    fprintf(stderr, "WorkerThread::stopInternal\n");
+
     // Protect against this method and initialize() racing each other.
     MutexLocker lock(m_threadCreationMutex);
 
@@ -473,7 +463,7 @@ void WorkerThread::stopInternal()
 
     InspectorInstrumentation::didKillAllExecutionContextTasks(m_workerGlobalScope.get());
     m_debuggerMessageQueue.kill();
-    postTask(FROM_HERE, WorkerThreadShutdownStartTask::create());
+    backingThread().postTask(FROM_HERE, new Task(threadSafeBind(&WorkerThread::cleanup, AllowCrossThreadAccess(this))));
 }
 
 void WorkerThread::didStartRunLoop()