Remove the concept of a cleanup task

Source/core/workers/WorkerGlobalScope.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
  * Copyright (C) 2009, 2011 Google Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 50 matching lines @@
 #include "core/workers/WorkerReportingProxy.h"
 #include "core/workers/WorkerScriptLoader.h"
 #include "core/workers/WorkerThread.h"
 #include "platform/network/ContentSecurityPolicyParsers.h"
 #include "platform/weborigin/KURL.h"
 #include "platform/weborigin/SecurityOrigin.h"
 #include "public/platform/WebURLRequest.h"
 
 namespace blink {
 
-class CloseWorkerGlobalScopeTask : public ExecutionContextTask {
-public:
-    static PassOwnPtr<CloseWorkerGlobalScopeTask> create()
-    {
-        return adoptPtr(new CloseWorkerGlobalScopeTask);
-    }
-
-    virtual void performTask(ExecutionContext *context)
-    {
-        WorkerGlobalScope* workerGlobalScope = toWorkerGlobalScope(context);
-        // Notify parent that this context is closed. Parent is responsible for calling WorkerThread::stop().
-        workerGlobalScope->thread()->workerReportingProxy().workerGlobalScopeClosed();
-    }
-
-    virtual bool isCleanupTask() const { return true; }
-};
-
 WorkerGlobalScope::WorkerGlobalScope(const KURL& url, const String& userAgent, WorkerThread* thread, double timeOrigin, const SecurityOrigin* starterOrigin, PassOwnPtrWillBeRawPtr<WorkerClients> workerClients)
     : m_url(url)
     , m_userAgent(userAgent)
     , m_v8CacheOptions(V8CacheOptionsDefault)
     , m_script(adoptPtr(new WorkerScriptController(*this, thread->isolate())))
     , m_thread(thread)
     , m_workerInspectorController(adoptRefWillBeNoop(new WorkerInspectorController(this)))
     , m_closing(false)
     , m_eventQueue(WorkerEventQueue::create(this))
     , m_workerClients(workerClients)
@@ skipping 69 matching lines @@
     return &m_timers;
 }
 
 WorkerLocation* WorkerGlobalScope::location() const
 {
     if (!m_location)
         m_location = WorkerLocation::create(m_url);
     return m_location.get();
 }
 
 void WorkerGlobalScope::close()

[haraken, 2015/05/09 15:54:36]

I'm just curious if we can add ASSERT(!m_closing) here. Can the close() be
called more than once?

[Sami, 2015/05/11 10:35:32]

Correct me if I'm wrong but I think a worker script can still call close twice
in the same task without yielding in the middle:

  self.close();
  self.close();

For that reason I don't think we can assert here.

 {
-    if (m_closing)
-        return;
-
-    // Let current script run to completion but prevent future script evaluations.
-    // After m_closing is set, all the tasks in the queue continue to be fetched but only
-    // tasks with isCleanupTask()==true will be executed.
+    // Let current script run to completion, but tell the worker thread to shut down after the script exits.

[haraken, 2015/05/09 15:54:36]

This comment looks out-dated. We no longer tell the worker thread to shut down
here.

[Sami, 2015/05/11 10:35:32]

Right, I've now tried to make it a little more accurate. Setting m_closing =
true will make trigger the worker micro task runner to cleanup the thread after
the current task.

     m_closing = true;
-    postTask(FROM_HERE, CloseWorkerGlobalScopeTask::create());
 }
 
 WorkerConsole* WorkerGlobalScope::console()
 {
     if (!m_console)
         m_console = WorkerConsole::create(this);
     return m_console.get();
 }
 
 WorkerNavigator* WorkerGlobalScope::navigator() const
@@ skipping 12 matching lines @@
 {
     ASSERT(m_workerInspectorController);
     thread()->setWorkerInspectorController(nullptr);
     m_workerInspectorController->dispose();
     m_workerInspectorController.clear();
 }
 
 void WorkerGlobalScope::dispose()
 {
     ASSERT(thread()->isCurrentThread());
+    stopActiveDOMObjects();
+
+    // Event listeners would keep DOMWrapperWorld objects alive for too long. Also, they have references to JS objects,
+    // which become dangling once Heap is destroyed.
+    removeAllEventListeners();
 
     clearScript();
     clearInspector();
     m_eventQueue->close();
 
     // We do not clear the thread field of the
     // WorkerGlobalScope. Other objects keep the worker global scope
     // alive because they need its thread field to check that work is
     // being carried out on the right thread. We therefore cannot clear
     // the thread field before all references to the worker global
@@ skipping 181 matching lines @@
     visitor->trace(m_timers);
     visitor->trace(m_messageStorage);
     visitor->trace(m_pendingMessages);
     HeapSupplementable<WorkerGlobalScope>::trace(visitor);
 #endif
     ExecutionContext::trace(visitor);
     EventTargetWithInlineData::trace(visitor);
 }
 
 } // namespace blink