Worker: Clarify thread affinity for improving code maintainability

To improve code maintainability, this CL clarifies thread affinity of
initialization and shutdown functions in worker classes. This does not change
any existing behavior.

Bug: 741227
Change-Id: I25fe7278130c90292b79cdd5607f10a0ef92f198
Reviewed-on: https://chromium-review.googlesource.com/567609
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486740}

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

@@ -53,7 +53,7 @@ Vector<RefPtr<DOMWrapperWorld>> CreateWorlds(v8::Isolate* isolate) {
 
 void WorkerThreadFunc(WorkerBackingThread* thread,
                       RefPtr<WebTaskRunner> main_thread_task_runner) {
-  thread->Initialize(WorkerV8Settings::Default());
+  thread->InitializeOnBackingThread(WorkerV8Settings::Default());
 
   // Worlds on the main thread should not be visible from the worker thread.
   Vector<RefPtr<DOMWrapperWorld>> retrieved_worlds;
@@ -78,7 +78,7 @@ void WorkerThreadFunc(WorkerBackingThread* thread,
   }
   worlds.clear();
 
-  thread->Shutdown();
+  thread->ShutdownOnBackingThread();
   main_thread_task_runner->PostTask(BLINK_FROM_HERE,
                                     CrossThreadBind(&testing::ExitRunLoop));
 }

third_party/WebKit/Source/core/css/threaded/MultiThreadedTestUtil.h

@@ -63,11 +63,12 @@ class MultiThreadedTest : public ::testing::Test {
       WebTaskRunner* task_runner =
           threads[i]->PlatformThread().GetWebTaskRunner();
 
-      task_runner->PostTask(
-          FROM_HERE,
-          CrossThreadBind(
-              [](WebThreadSupportingGC* thread) { thread->Initialize(); },
-              CrossThreadUnretained(threads[i].get())));
+      task_runner->PostTask(FROM_HERE,
+                            CrossThreadBind(
+                                [](WebThreadSupportingGC* thread) {
+                                  thread->InitializeOnThread();
+                                },
+                                CrossThreadUnretained(threads[i].get())));
 
       for (int j = 0; j < callbacks_per_thread_; ++j) {
         task_runner->PostTask(FROM_HERE,
@@ -77,7 +78,7 @@ class MultiThreadedTest : public ::testing::Test {
       task_runner->PostTask(
           FROM_HERE, CrossThreadBind(
                          [](WebThreadSupportingGC* thread, WaitableEvent* w) {
-                           thread->Shutdown();
+                           thread->ShutdownOnThread();
                            w->Signal();
                          },
                          CrossThreadUnretained(threads[i].get()),

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

@@ -58,9 +58,12 @@ WorkerBackingThread::WorkerBackingThread(WebThread* thread,
 
 WorkerBackingThread::~WorkerBackingThread() {}
 
-void WorkerBackingThread::Initialize(const WorkerV8Settings& settings) {
+void WorkerBackingThread::InitializeOnBackingThread(
+    const WorkerV8Settings& settings) {
+  DCHECK(backing_thread_->IsCurrentThread());
+  backing_thread_->InitializeOnThread();
+
   DCHECK(!isolate_);
-  backing_thread_->Initialize();
   isolate_ = V8PerIsolateData::Initialize(
       backing_thread_->PlatformThread().GetWebTaskRunner());
   AddWorkerIsolate(isolate_);
@@ -91,7 +94,8 @@ void WorkerBackingThread::Initialize(const WorkerV8Settings& settings) {
                                 WorkerV8Settings::AtomicsWaitMode::kAllow);
 }
 
-void WorkerBackingThread::Shutdown() {
+void WorkerBackingThread::ShutdownOnBackingThread() {
+  DCHECK(backing_thread_->IsCurrentThread());
   if (is_owning_thread_)
     Platform::Current()->WillStopWorkerThread();
 
@@ -101,7 +105,7 @@ void WorkerBackingThread::Shutdown() {
     // This statement runs only in tests.
     V8GCController::CollectAllGarbageForTesting(isolate_);
   }
-  backing_thread_->Shutdown();
+  backing_thread_->ShutdownOnThread();
 
   RemoveWorkerIsolate(isolate_);
   V8PerIsolateData::Destroy(isolate_);

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

@@ -20,14 +20,10 @@ class WebThread;
 class WebThreadSupportingGC;
 struct WorkerV8Settings;
 
-// WorkerBackingThread represents a WebThread with Oilpan and V8 potentially
-// shared by multiple WebWorker scripts. A WebWorker needs to call initialize()
-// to using V8 and Oilpan functionalities, and call shutdown() when the script
-// no longer needs the thread.
-// A WorkerBackingThread represents a WebThread while a WorkerThread corresponds
-// to a web worker. There is one-to-one correspondence between WorkerThread and
-// WorkerGlobalScope, but multiple WorkerThreads (i.e., multiple
-// WorkerGlobalScopes) can share one WorkerBackingThread.
+// WorkerBackingThread represents a WebThread with Oilpan and V8. A client of
+// WorkerBackingThread (i.e., WorkerThread) needs to call
+// InitializeOnBackingThread() to use V8 and Oilpan functionalities, and call
+// ShutdownOnBackingThread() when it no longer needs the thread.
 class CORE_EXPORT WorkerBackingThread final {
  public:
   static std::unique_ptr<WorkerBackingThread> Create(const char* name) {
@@ -48,12 +44,12 @@ class CORE_EXPORT WorkerBackingThread final {
 
   ~WorkerBackingThread();
 
-  // initialize() and shutdown() attaches and detaches Oilpan and V8 to / from
-  // the caller worker script, respectively. A worker script must not call
-  // any function after calling shutdown().
-  // They should be called from |this| thread.
-  void Initialize(const WorkerV8Settings&);
-  void Shutdown();
+  // InitializeOnBackingThread() and ShutdownOnBackingThread() attaches and
+  // detaches Oilpan and V8 to / from the caller worker script, respectively.
+  // A worker script must not call any function after calling
+  // ShutdownOnBackingThread(). They should be called from |this| thread.
+  void InitializeOnBackingThread(const WorkerV8Settings&);
+  void ShutdownOnBackingThread();
 
   WebThreadSupportingGC& BackingThread() {
     DCHECK(backing_thread_);

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

@@ -409,7 +409,7 @@ void WorkerThread::InitializeOnWorkerThread(
     MutexLocker lock(thread_state_mutex_);
 
     if (IsOwningBackingThread())
-      GetWorkerBackingThread().Initialize(v8_settings);
+      GetWorkerBackingThread().InitializeOnBackingThread(v8_settings);
     GetWorkerBackingThread().BackingThread().AddTaskObserver(this);
 
     console_message_storage_ = new ConsoleMessageStorage();
@@ -518,7 +518,7 @@ void WorkerThread::PerformShutdownOnWorkerThread() {
   global_scope_ = nullptr;
 
   if (IsOwningBackingThread())
-    GetWorkerBackingThread().Shutdown();
+    GetWorkerBackingThread().ShutdownOnBackingThread();
   // We must not touch workerBackingThread() from now on.
 
   // Notify the proxy that the WorkerOrWorkletGlobalScope has been disposed

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

@@ -63,8 +63,8 @@ enum WorkerThreadStartMode {
 
 // WorkerThread is a kind of WorkerBackingThread client. Each worker mechanism
 // can access the lower thread infrastructure via an implementation of this
-// abstract class. Multiple WorkerThreads can share one WorkerBackingThread.
-// See WorkerBackingThread.h for more details.
+// abstract class. Multiple WorkerThreads may share one WorkerBackingThread for
+// worklets.
 //
 // WorkerThread start and termination must be initiated on the main thread and
 // an actual task is executed on the worker thread.

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

@@ -92,7 +92,7 @@ class WorkletThreadHolder {
   void InitializeOnWorkletThread() {
     MutexLocker locker(HolderInstanceMutex());
     DCHECK(!initialized_);
-    thread_->Initialize(WorkerV8Settings::Default());
+    thread_->InitializeOnBackingThread(WorkerV8Settings::Default());
     initialized_ = true;
   }
 
@@ -108,7 +108,7 @@ class WorkletThreadHolder {
   }
 
   void ShutdownOnWorlketThread(WaitableEvent* waitable_event) {
-    thread_->Shutdown();
+    thread_->ShutdownOnBackingThread();
     waitable_event->Signal();
   }
 

third_party/WebKit/Source/modules/fetch/DataConsumerHandleTestUtil.cpp

@@ -57,6 +57,7 @@ DataConsumerHandleTestUtil::Thread::~Thread() {
 }
 
 void DataConsumerHandleTestUtil::Thread::Initialize() {
+  DCHECK(thread_->IsCurrentThread());
   if (initialization_policy_ >= kScriptExecution) {
     isolate_holder_ =
         WTF::MakeUnique<gin::IsolateHolder>(Platform::Current()
@@ -66,7 +67,7 @@ void DataConsumerHandleTestUtil::Thread::Initialize() {
                                                 ->ToSingleThreadTaskRunner());
     GetIsolate()->Enter();
   }
-  thread_->Initialize();
+  thread_->InitializeOnThread();
   if (initialization_policy_ >= kScriptExecution) {
     v8::HandleScope handle_scope(GetIsolate());
     script_state_ = ScriptState::Create(
@@ -81,12 +82,13 @@ void DataConsumerHandleTestUtil::Thread::Initialize() {
 }
 
 void DataConsumerHandleTestUtil::Thread::Shutdown() {
+  DCHECK(thread_->IsCurrentThread());
   execution_context_ = nullptr;
   if (script_state_) {
     script_state_->DisposePerContextData();
   }
   script_state_ = nullptr;
-  thread_->Shutdown();
+  thread_->ShutdownOnThread();
   if (isolate_holder_) {
     GetIsolate()->Exit();
     GetIsolate()->RequestGarbageCollectionForTesting(

third_party/WebKit/Source/modules/webdatabase/DatabaseThread.cpp

@@ -67,7 +67,8 @@ void DatabaseThread::Start() {
 }
 
 void DatabaseThread::SetupDatabaseThread() {
-  thread_->Initialize();
+  DCHECK(thread_->IsCurrentThread());
+  thread_->InitializeOnThread();
   transaction_coordinator_ = new SQLTransactionCoordinator();
 }
 
@@ -122,7 +123,8 @@ void DatabaseThread::CleanupDatabaseThread() {
 }
 
 void DatabaseThread::CleanupDatabaseThreadCompleted() {
-  thread_->Shutdown();
+  DCHECK(thread_->IsCurrentThread());
+  thread_->ShutdownOnThread();
   if (cleanup_sync_)  // Someone wanted to know when we were done cleaning up.
     cleanup_sync_->Signal();
 }

third_party/WebKit/Source/platform/WebThreadSupportingGC.cpp

@@ -26,6 +26,7 @@ std::unique_ptr<WebThreadSupportingGC> WebThreadSupportingGC::CreateForThread(
 WebThreadSupportingGC::WebThreadSupportingGC(const char* name,
                                              WebThread* thread)
     : thread_(thread) {
+  DCHECK(IsMainThread());
   DCHECK(!name || !thread);
 #if DCHECK_IS_ON()
   // We call this regardless of whether an existing thread is given or not,
@@ -41,17 +42,20 @@ WebThreadSupportingGC::WebThreadSupportingGC(const char* name,
 }
 
 WebThreadSupportingGC::~WebThreadSupportingGC() {
+  DCHECK(IsMainThread());
   // WebThread's destructor blocks until all the tasks are processed.
   owning_thread_.reset();
   MemoryCoordinator::UnregisterThread(thread_);
 }
 
-void WebThreadSupportingGC::Initialize() {
+void WebThreadSupportingGC::InitializeOnThread() {
+  DCHECK(thread_->IsCurrentThread());
   ThreadState::AttachCurrentThread();
   gc_task_runner_ = WTF::MakeUnique<GCTaskRunner>(thread_);
 }
 
-void WebThreadSupportingGC::Shutdown() {
+void WebThreadSupportingGC::ShutdownOnThread() {
+  DCHECK(thread_->IsCurrentThread());
 #if defined(LEAK_SANITIZER)
   ThreadState::Current()->ReleaseStaticPersistentNodes();
 #endif

third_party/WebKit/Source/platform/WebThreadSupportingGC.h

@@ -16,11 +16,12 @@
 namespace blink {
 
 // WebThreadSupportingGC wraps a WebThread and adds support for attaching
-// to and detaching from the Blink GC infrastructure. The initialize method
-// must be called during initialization on the WebThread and before the
-// thread allocates any objects managed by the Blink GC. The shutdown
-// method must be called on the WebThread during shutdown when the thread
-// no longer needs to access objects managed by the Blink GC.
+// to and detaching from the Blink GC infrastructure.
+//
+// The initialize method must be called during initialization on the WebThread
+// and before the thread allocates any objects managed by the Blink GC. The
+// shutdown method must be called on the WebThread during shutdown when the
+// thread no longer needs to access objects managed by the Blink GC.
 //
 // WebThreadSupportingGC usually internally creates and owns WebThread unless
 // an existing WebThread is given via createForThread.
@@ -67,8 +68,9 @@ class PLATFORM_EXPORT WebThreadSupportingGC final {
     thread_->RemoveTaskObserver(observer);
   }
 
-  void Initialize();
-  void Shutdown();
+  // Must be called on the WebThread.
+  void InitializeOnThread();
+  void ShutdownOnThread();
 
   WebThread& PlatformThread() const {
     DCHECK(thread_);