Worker: Check forcible termination by WorkerThread::isForciblyTerminated()

Before this CL, WorkerThread checks whether forcible termination happened by
WorkerOrWorkletScriptController::isExecutionTerminating(). After this CL,
WorkerThread checks it by WorkerThread::isForciblyTerminated() instead. This
should be clearer than the previous way and enables to remove an access to
WorkerThread::m_globalScope from the main thread.

BUG=638877

Review-Url: https://codereview.chromium.org/2324693003
Cr-Commit-Position: refs/heads/master@{#418571}

third_party/WebKit/Source/bindings/core/v8/WorkerOrWorkletScriptController.cpp

@@ -34,22 +34,17 @@
 #include "bindings/core/v8/ScriptSourceCode.h"
 #include "bindings/core/v8/ScriptValue.h"
 #include "bindings/core/v8/SourceLocation.h"
-#include "bindings/core/v8/V8DedicatedWorkerGlobalScope.h"
+#include "bindings/core/v8/V8DOMWrapper.h"
 #include "bindings/core/v8/V8ErrorHandler.h"
 #include "bindings/core/v8/V8Initializer.h"
 #include "bindings/core/v8/V8ObjectConstructor.h"
 #include "bindings/core/v8/V8ScriptRunner.h"
-#include "bindings/core/v8/V8SharedWorkerGlobalScope.h"
-#include "bindings/core/v8/V8WorkerGlobalScope.h"
 #include "bindings/core/v8/WrapperTypeInfo.h"
 #include "core/events/ErrorEvent.h"
-#include "core/frame/DOMTimer.h"
 #include "core/inspector/InspectorTraceEvents.h"
-#include "core/inspector/MainThreadDebugger.h"
 #include "core/inspector/WorkerThreadDebugger.h"
-#include "core/workers/MainThreadWorkletGlobalScope.h"
+#include "core/workers/WorkerGlobalScope.h"
 #include "core/workers/WorkerOrWorkletGlobalScope.h"
-#include "core/workers/WorkerThread.h"
 #include "platform/heap/ThreadState.h"
 #include "public/platform/Platform.h"
 #include <memory>
@@ -109,7 +104,6 @@ WorkerOrWorkletScriptController::WorkerOrWorkletScriptController(WorkerOrWorklet
     : m_globalScope(globalScope)
     , m_isolate(isolate)
     , m_executionForbidden(false)
-    , m_executionScheduledToTerminate(false)
     , m_rejectedPromises(RejectedPromises::create())
     , m_executionState(0)
 {
@@ -280,22 +274,6 @@ bool WorkerOrWorkletScriptController::evaluate(const ScriptSourceCode& sourceCod
     return true;
 }
 
-void WorkerOrWorkletScriptController::willScheduleExecutionTermination()
-{
-    // The mutex provides a memory barrier to ensure that once
-    // termination is scheduled, isExecutionTerminating will
-    // accurately reflect that state when called from another thread.
-    MutexLocker locker(m_scheduledTerminationMutex);
-    m_executionScheduledToTerminate = true;
-}
-
-bool WorkerOrWorkletScriptController::isExecutionTerminating() const
-{
-    // See comments in willScheduleExecutionTermination regarding mutex usage.
-    MutexLocker locker(m_scheduledTerminationMutex);
-    return m_executionScheduledToTerminate;
-}
-
 void WorkerOrWorkletScriptController::forbidExecution()
 {
     ASSERT(m_globalScope->isContextThread());

third_party/WebKit/Source/bindings/core/v8/WorkerOrWorkletScriptController.h

@@ -37,7 +37,6 @@
 #include "bindings/core/v8/V8CacheOptions.h"
 #include "core/CoreExport.h"
 #include "wtf/Allocator.h"
-#include "wtf/ThreadingPrimitives.h"
 #include "wtf/text/TextPosition.h"
 #include <v8.h>
 
@@ -57,23 +56,15 @@ public:
     void dispose();
 
     bool isExecutionForbidden() const;
-    bool isExecutionTerminating() const;
 
     // Returns true if the evaluation completed with no uncaught exception.
     bool evaluate(const ScriptSourceCode&, ErrorEvent** = nullptr, CachedMetadataHandler* = nullptr, V8CacheOptions = V8CacheOptionsDefault);
 
-    // Prevents future JavaScript execution. See
-    // willScheduleExecutionTermination, isExecutionForbidden.
+    // Prevents future JavaScript execution.
     void forbidExecution();
 
     // Used by WorkerThread:
     bool initializeContextIfNeeded();
-    // Async request to terminate future JavaScript execution on the worker
-    // thread. JavaScript evaluation exits with a non-continuable exception and
-    // WorkerOrWorkletScriptController calls forbidExecution to prevent further
-    // JavaScript execution. Use forbidExecution()/isExecutionForbidden() to
-    // guard against reentry into JavaScript.
-    void willScheduleExecutionTermination();
 
     // Used by WorkerGlobalScope:
     void rethrowExceptionFromImportedScript(ErrorEvent*, ExceptionState&);
@@ -110,8 +101,6 @@ private:
     RefPtr<DOMWrapperWorld> m_world;
     String m_disableEvalPending;
     bool m_executionForbidden;
-    bool m_executionScheduledToTerminate;
-    mutable Mutex m_scheduledTerminationMutex;
 
     RefPtr<RejectedPromises> m_rejectedPromises;
 

third_party/WebKit/Source/core/workers/WorkerThread.cpp

@@ -104,8 +104,7 @@ private:
             return;
         }
 
-        m_workerThread->forciblyTerminateExecution();
-        m_workerThread->setExitCode(lock, ExitCode::AsyncForciblyTerminated);
+        m_workerThread->forciblyTerminateExecution(lock, ExitCode::AsyncForciblyTerminated);
     }
 
     WorkerThread* m_workerThread;
@@ -138,7 +137,7 @@ public:
 
                 // Stop further worker tasks to run after this point.
                 m_workerThread->prepareForShutdownOnWorkerThread();
-            } else if (scriptController && scriptController->isExecutionTerminating()) {
+            } else if (m_workerThread->isForciblyTerminated()) {
                 // The script has been terminated forcibly, which means we need
                 // to ask objects in the thread to stop working as soon as
                 // possible.
@@ -397,9 +396,7 @@ void WorkerThread::terminateInternal(TerminationMode mode)
             // main thread and the scheduled termination task never runs.
             if (mode == TerminationMode::Forcible && m_exitCode == ExitCode::NotTerminated) {
                 DCHECK(m_scheduledForceTerminationTask);
-                m_scheduledForceTerminationTask.reset();
-                forciblyTerminateExecution();
-                setExitCode(lock, ExitCode::SyncForciblyTerminated);
+                forciblyTerminateExecution(lock, ExitCode::SyncForciblyTerminated);
             }
             return;
         }
@@ -413,8 +410,7 @@ void WorkerThread::terminateInternal(TerminationMode mode)
         } else if (shouldScheduleToTerminateExecution(lock)) {
             switch (mode) {
             case TerminationMode::Forcible:
-                forciblyTerminateExecution();
-                setExitCode(lock, ExitCode::SyncForciblyTerminated);
+                forciblyTerminateExecution(lock, ExitCode::SyncForciblyTerminated);
                 break;
             case TerminationMode::Graceful:
                 DCHECK(!m_scheduledForceTerminationTask);
@@ -458,12 +454,16 @@ bool WorkerThread::shouldScheduleToTerminateExecution(const MutexLocker& lock)
     return false;
 }
 
-void WorkerThread::forciblyTerminateExecution()
+void WorkerThread::forciblyTerminateExecution(const MutexLocker& lock, ExitCode exitCode)
 {
     DCHECK(isMainThread());
-    DCHECK(m_globalScope);
-    m_globalScope->scriptController()->willScheduleExecutionTermination();
+    DCHECK(isThreadStateMutexLocked(lock));
+
+    DCHECK(exitCode == ExitCode::SyncForciblyTerminated || exitCode == ExitCode::AsyncForciblyTerminated);
+    setExitCode(lock, exitCode);
+
     isolate()->TerminateExecution();
+    m_scheduledForceTerminationTask.reset();
 }
 
 bool WorkerThread::isInShutdown()

third_party/WebKit/Source/core/workers/WorkerThread.h

@@ -203,7 +203,9 @@ private:
     // acquired.
     bool shouldScheduleToTerminateExecution(const MutexLocker&);
 
-    void forciblyTerminateExecution();
+    // Forcibly terminates the worker execution. This must be called with
+    // |m_threadStateMutex| acquired.
+    void forciblyTerminateExecution(const MutexLocker&, ExitCode);
 
     // Returns true if termination or shutdown sequence has started. This is
     // thread safe.

third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp

@@ -348,6 +348,7 @@ TEST_F(WorkerThreadTest, StartAndTerminateOnScriptLoaded_TerminateWhileDebuggerT
     m_mockWorkerReportingProxy->waitUntilScriptLoaded();
 
     // Simulate that a debugger task is running.
+    // TODO(nhiroki): Run a real debugger task instead of simulation.
     m_workerThread->m_runningDebuggerTask = true;
 
     // terminate() should not schedule a force termination task because there is
@@ -370,7 +371,7 @@ TEST_F(WorkerThreadTest, StartAndTerminateOnScriptLoaded_TerminateWhileDebuggerT
 
     // Clean up in order to satisfy DCHECK in the dtor of WorkerThread.
     m_workerThread->m_runningDebuggerTask = false;
-    m_workerThread->forciblyTerminateExecution();
+    m_workerThread->isolate()->TerminateExecution();
     m_workerThread->waitForShutdownForTesting();
 }
 

third_party/WebKit/Source/core/workers/WorkletGlobalScope.cpp

@@ -29,7 +29,6 @@ void WorkletGlobalScope::dispose()
     stopActiveDOMObjects();
 
     DCHECK(m_scriptController);
-    m_scriptController->willScheduleExecutionTermination();
     m_scriptController->dispose();
     m_scriptController.clear();
 }