Oilpan: move WorkerGlobalScope to oilpan's heap.

Source/core/workers/WorkerGlobalScope.cpp

Index: Source/core/workers/WorkerGlobalScope.cpp
diff --git a/Source/core/workers/WorkerGlobalScope.cpp b/Source/core/workers/WorkerGlobalScope.cpp
index d0f3064a5060c908f073f80a5992341dbbdd197d..e273691d3451995df34b6a969105be18446cd214 100644
--- a/Source/core/workers/WorkerGlobalScope.cpp
+++ b/Source/core/workers/WorkerGlobalScope.cpp
@@ -93,12 +93,9 @@ WorkerGlobalScope::WorkerGlobalScope(const KURL& url, const String& userAgent, W
 
 WorkerGlobalScope::~WorkerGlobalScope()
 {
-    ASSERT(thread()->isCurrentThread());
-
-    // Notify proxy that we are going away. This can free the WorkerThread object, so do not access it after this.
-    thread()->workerReportingProxy().workerGlobalScopeDestroyed();
-
-    setClient(0);
+#if !ENABLE(OILPAN)
+    dispose();
+#endif
 }
 
 void WorkerGlobalScope::applyContentSecurityPolicyFromString(const String& policy, ContentSecurityPolicyHeaderType contentSecurityPolicyType)
@@ -190,6 +187,20 @@ void WorkerGlobalScope::clearInspector()
     m_workerInspectorController.clear();
 }
 
+void WorkerGlobalScope::dispose()
+{
+    ASSERT(thread()->isCurrentThread());
+
+    // Notify proxy that we are going away. This can free the WorkerThread object, so do not access it after this.
+    thread()->workerReportingProxy().workerGlobalScopeDestroyed();
+
+    clearScript();
+    clearInspector();
+    setClient(0);
+    // NOTE: the thread reference isn't cleared, as the execution context's
+    // thread is accessed by other GCed objects being finalized/destructed.

[haraken, 2014/03/10 07:10:03]

Thanks sof for the clarification! Now I think this CL is almost ready to go.

Let me ask a final question: Why is it OK to not clear m_thread in
WorkerGlobalScope::dispose()? It looks strange that WorkerThread is disposing
WorkerGlobalScope but WorkerGlobalScope does not (intentionally) clear the raw
pointer to the WorkerThread. I do understand why you cannot clear m_thread, but
it looks strange that you cannot clear m_thread.

[Mads Ager (chromium), 2014/03/10 08:51:14]

Is the issue here that lifecycle observers will access this object until the
destructor has been called? If that is the only issue this is working as it
should and the comment should be updated to state that m_thread can be accessed
by context life cycle observers until the WorkerGlobalScope is destructed.

The other option, which might be clearer, would be to make it possible to notify
the observers that the context is gone at this point by explicitly clearing the
observers in the lifecycle notifier. Long term, the context observers should
directly use our weak pointers instead, but we need to deal with this manually
for now until we get Document into the heap as well.

I think it would be fine to land this with the comment updated if the lifecycle
observers are the only ones accessing m_thread.

+}
+
 void WorkerGlobalScope::importScripts(const Vector<String>& urls, ExceptionState& exceptionState)
 {
     ASSERT(contentSecurityPolicy());
@@ -281,4 +292,12 @@ WorkerEventQueue* WorkerGlobalScope::eventQueue() const
     return m_eventQueue.get();
 }
 
+void WorkerGlobalScope::trace(Visitor* visitor)
+{
+    visitor->trace(m_console);
+    visitor->trace(m_location);
+    visitor->trace(m_navigator);
+    WillBeHeapSupplementable<WorkerGlobalScope>::trace(visitor);
+}
+
 } // namespace WebCore