Worker: Separate classic script evaluation from worker thread initialization

Before this CL, classic script evaluation is tied with worker thread
initialization. This is cumbersome for module scripts because module scripts are
fetched after a global scope is created and not ready during the initialization.
As a quickfix, it's bypassed for WorkletGlobalScope that is only the client of
module scripts on the worker thread for now (See the end of
WorkerThread::InitializeOnWorkerThread()).

For cleanup, this CL separates classic script evaluation from worker thread
initialization. If you want to evaluate classic scripts, you need to explictly
post a task after the worker thread initialization. This makes it easier to
switch classic script loading and module script loading in following CLs: if
you want to evaluate module scripts, you can just post a task for module script
evaluation.

Rough design doc of module scripts for Dedicated Worker:
https://docs.google.com/a/chromium.org/document/d/1IMGWAK7Wq37mLehwkbysNRBBnhQBo3z2MbYyMkViEnY/edit?usp=sharing

Bug: 680046
Change-Id: I7bf32051a8e423de65bb6b0d76164cb1540d3aa4
Reviewed-on: https://chromium-review.googlesource.com/822320Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524646}

third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp

@@ -340,11 +340,14 @@ void WebSharedWorkerImpl::OnScriptLoaderFinished() {
 
   GetWorkerThread()->Start(
       std::move(global_scope_creation_params), thread_startup_data,
-      std::make_unique<GlobalScopeInspectorCreationParams>(
-          worker_inspector_proxy_->ShouldPauseOnWorkerStart(document)),
-      task_runners, source_code);
+      worker_inspector_proxy_->ShouldPauseOnWorkerStart(document),
+      task_runners);
   worker_inspector_proxy_->WorkerThreadCreated(document, GetWorkerThread(),
                                                url_);
+  // TODO(nhiroki): Support module workers (https://crbug.com/680046).
+  GetWorkerThread()->EvaluateClassicScript(url_, source_code,
+                                           nullptr /* cached_meta_data */,
+                                           v8_inspector::V8StackTraceId());
   client_->WorkerScriptLoaded();
 }
 

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

@@ -58,12 +58,14 @@ void DedicatedWorkerMessagingProxy::StartWorkerGlobalScope(
   // TODO(nhiroki): Move ReleaseWorkerClients() to DedicatedWorker for cleanup.
   creation_params->worker_clients = ReleaseWorkerClients();
 
-  // TODO(nhiroki): Support module scripts (https://crbug.com/680046).
-  DCHECK_EQ("classic", options.type());
   InitializeWorkerThread(
       std::move(creation_params),
-      CreateBackingThreadStartupData(ToIsolate(GetExecutionContext())),
-      script_url, stack_id, source_code);
+      CreateBackingThreadStartupData(ToIsolate(GetExecutionContext())));
+
+  // TODO(nhiroki): Support module scripts (https://crbug.com/680046).
+  DCHECK_EQ("classic", options.type());
+  GetWorkerThread()->EvaluateClassicScript(
+      script_url, source_code, nullptr /* cached_meta_data */, stack_id);
 
   // Post all queued tasks to the worker.
   for (auto& queued_task : queued_early_tasks_) {

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

@@ -137,8 +137,10 @@ class DedicatedWorkerMessagingProxyForTest
             kV8CacheOptionsDefault),
         WorkerBackingThreadStartupData(
             WorkerBackingThreadStartupData::HeapLimitMode::kDefault,
-            WorkerBackingThreadStartupData::AtomicsWaitMode::kAllow),
-        script_url, v8_inspector::V8StackTraceId(), source);
+            WorkerBackingThreadStartupData::AtomicsWaitMode::kAllow));
+    GetWorkerThread()->EvaluateClassicScript(script_url, source,
+                                             nullptr /* cached_meta_data */,
+                                             v8_inspector::V8StackTraceId());
   }
 
   DedicatedWorkerThreadForTest* GetDedicatedWorkerThread() {

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

@@ -74,21 +74,17 @@ void ThreadedMessagingProxyBase::Trace(blink::Visitor* visitor) {
 
 void ThreadedMessagingProxyBase::InitializeWorkerThread(
     std::unique_ptr<GlobalScopeCreationParams> global_scope_creation_params,
-    const WTF::Optional<WorkerBackingThreadStartupData>& thread_startup_data,
-    const KURL& script_url,
-    const v8_inspector::V8StackTraceId& stack_id,
-    const String& source_code) {
+    const WTF::Optional<WorkerBackingThreadStartupData>& thread_startup_data) {
   DCHECK(IsParentContextThread());
 
   Document* document = ToDocument(GetExecutionContext());
+  KURL script_url = global_scope_creation_params->script_url.Copy();
 
   worker_thread_ = CreateWorkerThread();
   worker_thread_->Start(
       std::move(global_scope_creation_params), thread_startup_data,
-      std::make_unique<GlobalScopeInspectorCreationParams>(
       GetWorkerInspectorProxy()->ShouldPauseOnWorkerStart(document),
-          stack_id),
-      GetParentFrameTaskRunners(), source_code);
+      GetParentFrameTaskRunners());
   GetWorkerInspectorProxy()->WorkerThreadCreated(document, GetWorkerThread(),
                                                  script_url);
 }

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

@@ -16,10 +16,6 @@
 #include "platform/wtf/Forward.h"
 #include "platform/wtf/Optional.h"
 
-namespace v8_inspector {
-struct V8StackTraceId;
-}  // namespace v8_inspector
-
 namespace blink {
 
 class ExecutionContext;
@@ -74,10 +70,7 @@ class CORE_EXPORT ThreadedMessagingProxyBase
 
   void InitializeWorkerThread(
       std::unique_ptr<GlobalScopeCreationParams>,
-      const WTF::Optional<WorkerBackingThreadStartupData>&,
-      const KURL& script_url,
-      const v8_inspector::V8StackTraceId&,
-      const String& source_code = String());
+      const WTF::Optional<WorkerBackingThreadStartupData>&);
 
   ThreadableLoadingContext* CreateThreadableLoadingContext() const;
 

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

@@ -49,8 +49,7 @@ void ThreadedWorkletMessagingProxy::Initialize() {
 
   // Worklets share the pre-initialized backing thread so that we don't have to
   // specify the backing thread startup data.
-  InitializeWorkerThread(std::move(global_scope_creation_params), WTF::nullopt,
-                         document->Url(), v8_inspector::V8StackTraceId());
+  InitializeWorkerThread(std::move(global_scope_creation_params), WTF::nullopt);
 }
 
 void ThreadedWorkletMessagingProxy::Trace(blink::Visitor* visitor) {

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

@@ -181,7 +181,7 @@ class ThreadedWorkletMessagingProxyForTest
             worker_clients, document->AddressSpace(),
             OriginTrialContext::GetTokens(document).get(),
             std::move(worker_settings), kV8CacheOptionsDefault),
-        WTF::nullopt, document->Url(), v8_inspector::V8StackTraceId());
+        WTF::nullopt);
   }
 
  private:

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

@@ -179,13 +179,4 @@ void WorkerInspectorProxy::Trace(blink::Visitor* visitor) {
   visitor->Trace(execution_context_);
 }
 
-GlobalScopeInspectorCreationParams::GlobalScopeInspectorCreationParams(
-    WorkerInspectorProxy::PauseOnWorkerStart pause_on_start,
-    const v8_inspector::V8StackTraceId& stack_id)
-    : pause_on_start(pause_on_start), stack_id(stack_id) {}
-
-GlobalScopeInspectorCreationParams::GlobalScopeInspectorCreationParams(
-    WorkerInspectorProxy::PauseOnWorkerStart pause_on_start)
-    : pause_on_start(pause_on_start) {}
-
 }  // namespace blink

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

@@ -75,24 +75,6 @@ class CORE_EXPORT WorkerInspectorProxy final
   String inspector_id_;
 };
 
-struct CORE_EXPORT GlobalScopeInspectorCreationParams final {
-  USING_FAST_MALLOC(GlobalScopeInspectorCreationParams);
-
- public:
-  explicit GlobalScopeInspectorCreationParams(
-      WorkerInspectorProxy::PauseOnWorkerStart pause_on_start);
-  GlobalScopeInspectorCreationParams(
-      WorkerInspectorProxy::PauseOnWorkerStart pause_on_start,
-      const v8_inspector::V8StackTraceId&);
-  ~GlobalScopeInspectorCreationParams() = default;
-
-  WorkerInspectorProxy::PauseOnWorkerStart pause_on_start;
-  v8_inspector::V8StackTraceId stack_id;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(GlobalScopeInspectorCreationParams);
-};
-
 }  // namespace blink
 
 #endif  // WorkerInspectorProxy_h

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

@@ -98,11 +98,8 @@ WorkerThread::~WorkerThread() {
 void WorkerThread::Start(
     std::unique_ptr<GlobalScopeCreationParams> global_scope_creation_params,
     const WTF::Optional<WorkerBackingThreadStartupData>& thread_startup_data,
-    std::unique_ptr<GlobalScopeInspectorCreationParams>
-        global_scope_inspector_creation_params,
-    ParentFrameTaskRunners* parent_frame_task_runners,
-    const String& source_code,
-    std::unique_ptr<Vector<char>> cached_meta_data) {
+    WorkerInspectorProxy::PauseOnWorkerStart pause_on_start,
+    ParentFrameTaskRunners* parent_frame_task_runners) {
   DCHECK(IsMainThread());
   DCHECK(!parent_frame_task_runners_);
   parent_frame_task_runners_ = parent_frame_task_runners;
@@ -119,12 +116,23 @@ void WorkerThread::Start(
 
   GetWorkerBackingThread().BackingThread().PostTask(
       BLINK_FROM_HERE,
-      CrossThreadBind(
-          &WorkerThread::InitializeOnWorkerThread, CrossThreadUnretained(this),
+      CrossThreadBind(&WorkerThread::InitializeOnWorkerThread,
+                      CrossThreadUnretained(this),
                       WTF::Passed(std::move(global_scope_creation_params)),
-          thread_startup_data,
-          WTF::Passed(std::move(global_scope_inspector_creation_params)),
-          source_code, WTF::Passed(std::move(cached_meta_data))));
+                      thread_startup_data, pause_on_start));
+}
+
+void WorkerThread::EvaluateClassicScript(
+    const KURL& script_url,
+    const String& source_code,
+    std::unique_ptr<Vector<char>> cached_meta_data,
+    const v8_inspector::V8StackTraceId& stack_id) {
+  GetTaskRunner(TaskType::kUnthrottled)
+      ->PostTask(
+          BLINK_FROM_HERE,
+          CrossThreadBind(&WorkerThread::EvaluateClassicScriptOnWorkerThread,
+                          CrossThreadUnretained(this), script_url, source_code,
+                          WTF::Passed(std::move(cached_meta_data)), stack_id));
 }
 
 void WorkerThread::Terminate() {
@@ -386,10 +394,7 @@ void WorkerThread::InitializeSchedulerOnWorkerThread(
 void WorkerThread::InitializeOnWorkerThread(
     std::unique_ptr<GlobalScopeCreationParams> global_scope_creation_params,
     const WTF::Optional<WorkerBackingThreadStartupData>& thread_startup_data,
-    std::unique_ptr<GlobalScopeInspectorCreationParams>
-        global_scope_inspector_creation_params,
-    String source_code,
-    std::unique_ptr<Vector<char>> cached_meta_data) {
+    WorkerInspectorProxy::PauseOnWorkerStart pause_on_start) {
   DCHECK(IsCurrentThread());
   DCHECK_EQ(ThreadState::kNotStarted, thread_state_);
 
@@ -425,10 +430,8 @@ void WorkerThread::InitializeOnWorkerThread(
     SetThreadState(lock, ThreadState::kRunning);
   }
 
-  if (global_scope_inspector_creation_params->pause_on_start ==
-      WorkerInspectorProxy::PauseOnWorkerStart::kPause) {
+  if (pause_on_start == WorkerInspectorProxy::PauseOnWorkerStart::kPause)
     StartRunningDebuggerTasksOnPauseOnWorkerThread();
-  }
 
   if (CheckRequestedToTerminateOnWorkerThread()) {
     // Stop further worker tasks from running after this point. WorkerThread
@@ -437,19 +440,19 @@ void WorkerThread::InitializeOnWorkerThread(
     PrepareForShutdownOnWorkerThread();
     return;
   }
+}
 
-  // Worklet will evaluate the script later via Worklet.addModule().
-  // TODO(nhiroki): Start module loading for workers here.
-  // (https://crbug.com/680046)
-  if (GlobalScope()->IsWorkletGlobalScope())
-    return;
+void WorkerThread::EvaluateClassicScriptOnWorkerThread(
+    const KURL& script_url,
+    String source_code,
+    std::unique_ptr<Vector<char>> cached_meta_data,
+    const v8_inspector::V8StackTraceId& stack_id) {
+  DCHECK(GlobalScope()->IsWorkerGlobalScope());
   WorkerThreadDebugger* debugger = WorkerThreadDebugger::From(GetIsolate());
-  debugger->ExternalAsyncTaskStarted(
-      global_scope_inspector_creation_params->stack_id);
+  debugger->ExternalAsyncTaskStarted(stack_id);
   GlobalScope()->EvaluateClassicScript(script_url, std::move(source_code),
                                        std::move(cached_meta_data));
-  debugger->ExternalAsyncTaskFinished(
-      global_scope_inspector_creation_params->stack_id);
+  debugger->ExternalAsyncTaskFinished(stack_id);
 }
 
 void WorkerThread::PrepareForShutdownOnWorkerThread() {

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

@@ -57,7 +57,6 @@ class WorkerInspectorController;
 class WorkerOrWorkletGlobalScope;
 class WorkerReportingProxy;
 struct GlobalScopeCreationParams;
-struct GlobalScopeInspectorCreationParams;
 
 // WorkerThread is a kind of WorkerBackingThread client. Each worker mechanism
 // can access the lower thread infrastructure via an implementation of this
@@ -89,19 +88,25 @@ class CORE_EXPORT WorkerThread : public WebThread::TaskObserver {
   // Starts the underlying thread and creates the global scope. Called on the
   // main thread.
   // Startup data for WorkerBackingThread is WTF::nullopt if |this| doesn't own
-  // the underlying WorkerBackingThread. |source_code| is empty for module
-  // scripts or installed scripts. |cached_meta_data| is nullptr if the global
-  // scope to be created doesn't use V8 code caching.
+  // the underlying WorkerBackingThread.
   // TODO(nhiroki): We could separate WorkerBackingThread initialization from
   // GlobalScope initialization sequence, that is, InitializeOnWorkerThread().
   // After that, we could remove this startup data for WorkerBackingThread.
   // (https://crbug.com/710364)
   void Start(std::unique_ptr<GlobalScopeCreationParams>,
              const WTF::Optional<WorkerBackingThreadStartupData>&,
-             std::unique_ptr<GlobalScopeInspectorCreationParams>,
-             ParentFrameTaskRunners*,
-             const String& source_code = String(),
-             std::unique_ptr<Vector<char>> cached_meta_data = nullptr);
+             WorkerInspectorProxy::PauseOnWorkerStart,
+             ParentFrameTaskRunners*);
+
+  // Posts a task to evaluate a top-level classic script on the worker thread.
+  // Called on the main thread after Start().
+  void EvaluateClassicScript(const KURL& script_url,
+                             const String& source_code,
+                             std::unique_ptr<Vector<char>> cached_meta_data,
+                             const v8_inspector::V8StackTraceId& stack_id);
+
+  // TODO(nhiroki): Implement ImportModuleScript() for module workers.
+  // (https://crbug.com/680046)
 
   // Closes the global scope and terminates the underlying thread. Called on the
   // main thread.
@@ -241,9 +246,13 @@ class CORE_EXPORT WorkerThread : public WebThread::TaskObserver {
   void InitializeOnWorkerThread(
       std::unique_ptr<GlobalScopeCreationParams>,
       const WTF::Optional<WorkerBackingThreadStartupData>&,
-      std::unique_ptr<GlobalScopeInspectorCreationParams>,
+      WorkerInspectorProxy::PauseOnWorkerStart);
+
+  void EvaluateClassicScriptOnWorkerThread(
+      const KURL& script_url,
       String source_code,
-      std::unique_ptr<Vector<char>> cached_meta_data);
+      std::unique_ptr<Vector<char>> cached_meta_data,
+      const v8_inspector::V8StackTraceId& stack_id);
 
   // These are called in this order during worker thread termination.
   void PrepareForShutdownOnWorkerThread();

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

@@ -309,8 +309,7 @@ TEST_F(WorkerThreadTest, Terminate_WhileDebuggerTaskIsRunningOnInitialization) {
   // on initialization to run debugger tasks.
   worker_thread_->Start(std::move(global_scope_creation_params),
                         WorkerBackingThreadStartupData::CreateDefault(),
-                        std::make_unique<GlobalScopeInspectorCreationParams>(
-                            WorkerInspectorProxy::PauseOnWorkerStart::kPause),
+                        WorkerInspectorProxy::PauseOnWorkerStart::kPause,
                         ParentFrameTaskRunners::Create());
 
   // Used to wait for worker thread termination in a debugger task on the

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

@@ -108,9 +108,10 @@ class WorkerThreadForTest : public WorkerThread {
 
     Start(std::move(creation_params),
           WorkerBackingThreadStartupData::CreateDefault(),
-          std::make_unique<GlobalScopeInspectorCreationParams>(
-              WorkerInspectorProxy::PauseOnWorkerStart::kDontPause),
-          parent_frame_task_runners, source);
+          WorkerInspectorProxy::PauseOnWorkerStart::kDontPause,
+          parent_frame_task_runners);
+    EvaluateClassicScript(script_url, source, nullptr /* cached_meta_data */,
+                          v8_inspector::V8StackTraceId());
   }
 
   void WaitForInit() {

third_party/WebKit/Source/modules/animationworklet/AnimationWorkletGlobalScopeTest.cpp

@@ -64,9 +64,7 @@ class AnimationWorkletGlobalScopeTest : public ::testing::Test {
             clients, document->AddressSpace(),
             OriginTrialContext::GetTokens(document).get(),
             nullptr /* worker_settings */, kV8CacheOptionsDefault),
-        WTF::nullopt,
-        std::make_unique<GlobalScopeInspectorCreationParams>(
-            WorkerInspectorProxy::PauseOnWorkerStart::kDontPause),
+        WTF::nullopt, WorkerInspectorProxy::PauseOnWorkerStart::kDontPause,
         ParentFrameTaskRunners::Create());
     return thread;
   }

third_party/WebKit/Source/modules/animationworklet/AnimationWorkletThreadTest.cpp

@@ -98,9 +98,7 @@ class AnimationWorkletThreadTest : public ::testing::Test {
             clients, document->AddressSpace(),
             OriginTrialContext::GetTokens(document).get(),
             nullptr /* worker_settings */, kV8CacheOptionsDefault),
-        WTF::nullopt,
-        std::make_unique<GlobalScopeInspectorCreationParams>(
-            WorkerInspectorProxy::PauseOnWorkerStart::kDontPause),
+        WTF::nullopt, WorkerInspectorProxy::PauseOnWorkerStart::kDontPause,
         ParentFrameTaskRunners::Create());
     return thread;
   }

third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp

@@ -431,13 +431,16 @@ void WebEmbeddedWorkerImpl::StartWorkerThread() {
   worker_thread_->Start(
       std::move(global_scope_creation_params),
       WorkerBackingThreadStartupData::CreateDefault(),
-      std::make_unique<GlobalScopeInspectorCreationParams>(
-          worker_inspector_proxy_->ShouldPauseOnWorkerStart(document)),
-      ParentFrameTaskRunners::Create(), source_code,
-      std::move(cached_meta_data));
+      worker_inspector_proxy_->ShouldPauseOnWorkerStart(document),
+      ParentFrameTaskRunners::Create());
 
   worker_inspector_proxy_->WorkerThreadCreated(document, worker_thread_.get(),
                                                worker_start_data_.script_url);
+
+  // TODO(nhiroki): Support module workers (https://crbug.com/680046).
+  worker_thread_->EvaluateClassicScript(
+      worker_start_data_.script_url, source_code, std::move(cached_meta_data),
+      v8_inspector::V8StackTraceId());
 }
 
 }  // namespace blink

third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScopeTest.cpp

@@ -75,9 +75,7 @@ class AudioWorkletGlobalScopeTest : public ::testing::Test {
             nullptr /* worker_clients */, document->AddressSpace(),
             OriginTrialContext::GetTokens(document).get(),
             nullptr /* worker_settings */, kV8CacheOptionsDefault),
-        WTF::nullopt,
-        std::make_unique<GlobalScopeInspectorCreationParams>(
-            WorkerInspectorProxy::PauseOnWorkerStart::kDontPause),
+        WTF::nullopt, WorkerInspectorProxy::PauseOnWorkerStart::kDontPause,
         ParentFrameTaskRunners::Create());
     return thread;
   }

third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp

@@ -59,9 +59,7 @@ class AudioWorkletThreadTest : public ::testing::Test {
             nullptr /* worker_clients */, document->AddressSpace(),
             OriginTrialContext::GetTokens(document).get(),
             nullptr /* worker_settings */, kV8CacheOptionsDefault),
-        WTF::nullopt,
-        std::make_unique<GlobalScopeInspectorCreationParams>(
-            WorkerInspectorProxy::PauseOnWorkerStart::kDontPause),
+        WTF::nullopt, WorkerInspectorProxy::PauseOnWorkerStart::kDontPause,
         ParentFrameTaskRunners::Create());
     return thread;
   }