DOMDataStore should be cleared when shutting down worker threads

Source/bindings/v8/WorkerScriptController.cpp

Index: Source/bindings/v8/WorkerScriptController.cpp
diff --git a/Source/bindings/v8/WorkerScriptController.cpp b/Source/bindings/v8/WorkerScriptController.cpp
index acf1941814d3d46b53a38e4756b46556320e5805..ff5b233faf0d6bf1f96fef529a456bc0932c5cb6 100644
--- a/Source/bindings/v8/WorkerScriptController.cpp
+++ b/Source/bindings/v8/WorkerScriptController.cpp
@@ -100,10 +100,6 @@ WorkerScriptController::~WorkerScriptController()
 {
     ThreadState::current()->removeInterruptor(m_interruptor.get());
 
-    RELEASE_ASSERT(m_world->hasOneRef());
-    // ~DOMWrapperWorld() must be called before disposing the isolate.
-    m_world = nullptr;
-
     // The corresponding call to didStartWorkerRunLoop is in
     // WorkerThread::workerThread().
     // See http://webkit.org/b/83104#c14 for why this is here.
@@ -111,6 +107,8 @@ WorkerScriptController::~WorkerScriptController()
 
     disposeContext();
 
+    m_world->dispose();

[kouhei (in TOK), 2014/03/10 04:49:26]

Can't we just delete the world instead? Why do we need dispose?

[haraken, 2014/03/10 04:58:59]

Because other objects can retain RefPtr<DOMWrapperWorld>. For example,
V8MutationCallback, V8PerContextDataHolder etc retains RefPtr<DOMWrapperWorld>.
Rather than manually clearing these retainers, it's better to let RefPtr decide
when the DOMWrapperWorld is destructed.

+
     ThreadState::current()->addCleanupTask(IsolateCleanupTask::create(m_isolate));
 }
 
@@ -128,7 +126,7 @@ bool WorkerScriptController::initializeContextIfNeeded()
     if (context.IsEmpty())
         return false;
 
-    m_perContextData = V8PerContextData::create(context, m_world.get());
+    m_perContextData = V8PerContextData::create(context, m_world);
 
     v8::Context::Scope scope(context);
 
@@ -213,11 +211,11 @@ void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, RefPtr
     if (state.hadException) {
         if (errorEvent) {
             *errorEvent = m_workerGlobalScope.shouldSanitizeScriptError(state.sourceURL, NotSharableCrossOrigin) ?
-                ErrorEvent::createSanitizedError(nullptr) : ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, nullptr);
+                ErrorEvent::createSanitizedError(0) : ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, 0);
             V8ErrorHandler::storeExceptionOnErrorEventWrapper(errorEvent->get(), state.exception.v8Value(), m_isolate);
         } else {
             ASSERT(!m_workerGlobalScope.shouldSanitizeScriptError(state.sourceURL, NotSharableCrossOrigin));
-            RefPtr<ErrorEvent> event = m_errorEventFromImportedScript ? m_errorEventFromImportedScript.release() : ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, nullptr);
+            RefPtr<ErrorEvent> event = m_errorEventFromImportedScript ? m_errorEventFromImportedScript.release() : ErrorEvent::create(state.errorMessage, state.sourceURL, state.lineNumber, state.columnNumber, 0);
             m_workerGlobalScope.reportException(event, nullptr, NotSharableCrossOrigin);
         }
     }