Worker: Attempt to gracefully terminate WorkerThread as much as possible

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

Index: third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp
diff --git a/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp b/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp
index 8fb299c9d7693bf4a41fbf1664d1646ac85b57d8..9915e5f3786db7ea8a8887c0ea37159cf0196133 100644
--- a/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp
+++ b/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp
@@ -5,6 +5,7 @@
 #include "core/workers/WorkerThread.h"
 
 #include "core/workers/WorkerThreadTestHelper.h"
+#include "platform/testing/UnitTestHelpers.h"
 #include "public/platform/WebScheduler.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -17,7 +18,7 @@ using testing::Mock;
 
 namespace blink {
 
-class WorkerThreadTest : public testing::Test {
+class WorkerThreadTest : public ::testing::Test {
 public:
     void SetUp() override
     {
@@ -49,6 +50,16 @@ public:
         m_workerThread->waitForInit();
     }
 
+    void waitForShutdown()
+    {
+        m_workerThread->m_shutdownEvent->wait();
+    }
+
+    void setForceTerminationDelayInMs(long long forceTerminationDelayInMs)
+    {
+        m_workerThread->setForceTerminationDelayInMs(forceTerminationDelayInMs);
+    }
+
 protected:
     void expectWorkerLifetimeReportingCalls()
     {
@@ -70,6 +81,7 @@ TEST_F(WorkerThreadTest, StartAndStop)
     start();
     waitForInit();
     m_workerThread->terminateAndWait();
+    EXPECT_EQ(WorkerThread::ExitCode::SyncForciblyTerminated, m_workerThread->exitCode());
 }
 
 TEST_F(WorkerThreadTest, StartAndStopImmediately)
@@ -83,10 +95,27 @@ TEST_F(WorkerThreadTest, StartAndStopImmediately)
     EXPECT_CALL(*m_mockWorkerReportingProxy, willDestroyWorkerGlobalScope())
         .Times(AtMost(1));
     start();
+
+    // terminateAndWait() does not attempt to terminate the isolate because it
+    // has not started yet at this point.
     m_workerThread->terminateAndWait();
+    EXPECT_EQ(WorkerThread::ExitCode::GracefullyTerminated, m_workerThread->exitCode());
+}
+
+TEST_F(WorkerThreadTest, StartAndStopOnScriptLoaded_GracefullyTerminate)
+{
+    expectWorkerLifetimeReportingCalls();
+    start();
+    waitForInit();
+
+    // The worker thread is not being blocked, so the worker thread should be
+    // gracefully shut down without any forcible isolate termination.
+    m_workerThread->terminate();
+    waitForShutdown();
+    EXPECT_EQ(WorkerThread::ExitCode::GracefullyTerminated, m_workerThread->exitCode());
 }
 
-TEST_F(WorkerThreadTest, StartAndStopOnScriptLoaded)
+TEST_F(WorkerThreadTest, StartAndStopOnScriptLoaded_SyncForciblyTerminate)
 {
     // Use a JavaScript source code that makes an infinite loop so that we can
     // catch some kind of issues as a timeout.
@@ -102,7 +131,40 @@ TEST_F(WorkerThreadTest, StartAndStopOnScriptLoaded)
         .Times(AtMost(1));
     startWithSourceCode(source);
     m_workerThread->waitUntilScriptLoaded();
+
+    // terminateAndWait() synchronously terminates the isolate.
     m_workerThread->terminateAndWait();
+    EXPECT_EQ(WorkerThread::ExitCode::SyncForciblyTerminated, m_workerThread->exitCode());
+}
+
+TEST_F(WorkerThreadTest, StartAndStopOnScriptLoaded_AsyncForciblyTerminate)

[yhirano, 2016/05/27 11:04:43]

Can you add a test case for the case that a thread is terminated forcibly while
waiting for ForceTerminationTask to run?

[nhiroki, 2016/05/30 06:18:47]

Good point. The patchset 1 couldn't handle this case as follows:

1) terminateInternal() called from terminateAndWait() immediately returns
because |m_terminated| was already set by the previous terminate() call.
2) terminateAndWait() blocks the main thread and it prevents the scheduled task
from running.
3) DEAD LOCK.

The patchset 2 made terminateAndWait() overtake the scheduled termination task.

+{
+    const long long kForceTerminationDelayInMs = 10;
+    setForceTerminationDelayInMs(kForceTerminationDelayInMs);
+
+    // Use a JavaScript source code that makes an infinite loop so that we can
+    // catch some kind of issues as a timeout.
+    const String source("while(true) {}");
+
+    EXPECT_CALL(*m_mockWorkerReportingProxy, workerGlobalScopeStarted(_))
+        .Times(AtMost(1));
+    EXPECT_CALL(*m_mockWorkerReportingProxy, didEvaluateWorkerScript(_))
+        .Times(AtMost(1));
+    EXPECT_CALL(*m_mockWorkerReportingProxy, workerThreadTerminated())
+        .Times(AtMost(1));
+    EXPECT_CALL(*m_mockWorkerReportingProxy, willDestroyWorkerGlobalScope())
+        .Times(AtMost(1));
+    startWithSourceCode(source);
+    m_workerThread->waitUntilScriptLoaded();
+
+    // terminate() schedules a force termination task.
+    m_workerThread->terminate();
+    EXPECT_EQ(WorkerThread::ExitCode::NotTerminated, m_workerThread->exitCode());
+
+    // Wait until the force termination task runs.
+    testing::runDelayedTasks(kForceTerminationDelayInMs);
+    waitForShutdown();
+    EXPECT_EQ(WorkerThread::ExitCode::AsyncForciblyTerminated, m_workerThread->exitCode());
 }
 
 } // namespace blink