worker: Move pausing top-level classic script execution logic to WorkerGlobalScope

This CL moves logic to top-level script execution from
ServiceWorkerGlobalScope to WorkerGlobalScope so that we can use the same logic
among other WorkerGlobalScopes. We will use this functionality to avoid a race
condition which could happen when off-the-main-thread shared worker script fetch
is enabled.

Change-Id: Ib34373fe7864189da5ba4c00d19db2c027ce36cc
Bug: 945673
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1616863
Auto-Submit: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662039}

third_party/blink/renderer/core/workers/dedicated_worker_global_scope.cc

@@ -98,6 +98,8 @@ DedicatedWorkerGlobalScope::DedicatedWorkerGlobalScope(
     base::TimeTicks time_origin,
     std::unique_ptr<Vector<String>> outside_origin_trial_tokens)
     : WorkerGlobalScope(std::move(creation_params), thread, time_origin) {
+  // Dedicated workers don't need to pause after script fetch.
+  ReadyToRunClassicScript();
   // Inherit the outside's origin trial tokens.
   OriginTrialContext::AddTokens(this, outside_origin_trial_tokens.get());
 }

third_party/blink/renderer/core/workers/experimental/thread_pool_thread.cc

@@ -26,7 +26,9 @@ class ThreadPoolWorkerGlobalScope final : public WorkerGlobalScope {
       WorkerThread* thread)
       : WorkerGlobalScope(std::move(creation_params),
                           thread,
-                          CurrentTimeTicks()) {}
+                          CurrentTimeTicks()) {
+    ReadyToRunClassicScript();
+  }
 
   ~ThreadPoolWorkerGlobalScope() override = default;
 

third_party/blink/renderer/core/workers/shared_worker_global_scope.cc

@@ -90,7 +90,10 @@ SharedWorkerGlobalScope::SharedWorkerGlobalScope(
     std::unique_ptr<GlobalScopeCreationParams> creation_params,
     SharedWorkerThread* thread,
     base::TimeTicks time_origin)
-    : WorkerGlobalScope(std::move(creation_params), thread, time_origin) {}
+    : WorkerGlobalScope(std::move(creation_params), thread, time_origin) {
+  // TODO(bashi): Call this after appcache host is set.
+  ReadyToRunClassicScript();
+}
 
 SharedWorkerGlobalScope::~SharedWorkerGlobalScope() = default;
 

third_party/blink/renderer/core/workers/worker_global_scope.cc

@@ -390,11 +390,28 @@ void WorkerGlobalScope::ReceiveMessage(BlinkTransferableMessage message) {
     debugger->ExternalAsyncTaskFinished(message.sender_stack_trace_id);
 }
 
+void WorkerGlobalScope::ReadyToRunClassicScript() {
+  DCHECK_EQ(ScriptEvalState::kPauseAfterFetch, script_eval_state_);
+  script_eval_state_ = ScriptEvalState::kReadyToEvaluate;
+  if (evaluate_script_)
+    std::move(evaluate_script_).Run();
+}
+
 void WorkerGlobalScope::EvaluateClassicScriptInternal(
     const KURL& script_url,
     String source_code,
     std::unique_ptr<Vector<uint8_t>> cached_meta_data) {
   DCHECK(IsContextThread());
+  DCHECK_NE(ScriptEvalState::kEvaluated, script_eval_state_);
+
+  if (script_eval_state_ == ScriptEvalState::kPauseAfterFetch) {
+    evaluate_script_ =
+        WTF::Bind(&WorkerGlobalScope::EvaluateClassicScriptInternal,
+                  WrapWeakPersistent(this), script_url, std::move(source_code),
+                  std::move(cached_meta_data));
+    return;
+  }
+
   SingleCachedMetadataHandler* handler =
       CreateWorkerScriptCachedMetadataHandler(script_url,
                                               std::move(cached_meta_data));
@@ -408,6 +425,7 @@ void WorkerGlobalScope::EvaluateClassicScriptInternal(
       SanitizeScriptErrors::kDoNotSanitize, nullptr /* error_event */,
       GetV8CacheOptions());
   ReportingProxy().DidEvaluateClassicScript(success);
+  script_eval_state_ = ScriptEvalState::kEvaluated;
 }
 
 WorkerGlobalScope::WorkerGlobalScope(
@@ -435,7 +453,8 @@ WorkerGlobalScope::WorkerGlobalScope(
               creation_params->begin_frame_provider_params)),
       agent_cluster_id_(creation_params->agent_cluster_id.is_empty()
                             ? base::UnguessableToken::Create()
-                            : creation_params->agent_cluster_id) {
+                            : creation_params->agent_cluster_id),
+      script_eval_state_(ScriptEvalState::kPauseAfterFetch) {
   InstanceCounters::IncrementCounter(
       InstanceCounters::kWorkerGlobalScopeCounter);
   scoped_refptr<SecurityOrigin> security_origin =

third_party/blink/renderer/core/workers/worker_global_scope.h

@@ -209,8 +209,13 @@ class CORE_EXPORT WorkerGlobalScope
   void ExceptionThrown(ErrorEvent*) override;
   void RemoveURLFromMemoryCache(const KURL&) final;
 
+  // Notifies that the top-level classic script is ready to evaluate.
+  // Worker top-level script is evaluated after it is fetched and
+  // ReadyToRunClassicScript() is called.
+  void ReadyToRunClassicScript();
+
   // Evaluates the given top-level classic script.
-  virtual void EvaluateClassicScriptInternal(
+  void EvaluateClassicScriptInternal(
       const KURL& script_url,
       String source_code,
       std::unique_ptr<Vector<uint8_t>> cached_meta_data);
@@ -260,6 +265,21 @@ class CORE_EXPORT WorkerGlobalScope
 
   const base::UnguessableToken agent_cluster_id_;
 
+  // State transition about worker-toplevel script evaluation.
+  enum class ScriptEvalState {
+    // Initial state: ReadyToRunClassicScript() is not yet called.
+    // Worker top-level script fetch might or might not completed, and even when
+    // the fetch completes in this state, script evaluation will be deferred to
+    // when ReadyToRunClassicScript() is called later.
+    kPauseAfterFetch,
+    // ReadyToRunClassicScript() is already called.
+    kReadyToEvaluate,
+    // The worker top-level script is evaluated.
+    kEvaluated,
+  };
+  ScriptEvalState script_eval_state_;
+  base::OnceClosure evaluate_script_;
+
   HttpsState https_state_;
 };
 

third_party/blink/renderer/core/workers/worker_thread_test_helper.h

@@ -49,7 +49,9 @@ class FakeWorkerGlobalScope : public WorkerGlobalScope {
       WorkerThread* thread)
       : WorkerGlobalScope(std::move(creation_params),
                           thread,
-                          CurrentTimeTicks()) {}
+                          CurrentTimeTicks()) {
+    ReadyToRunClassicScript();
+  }
 
   ~FakeWorkerGlobalScope() override = default;
 

third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc

@@ -162,10 +162,7 @@ ServiceWorkerGlobalScope::ServiceWorkerGlobalScope(
 ServiceWorkerGlobalScope::~ServiceWorkerGlobalScope() = default;
 
 void ServiceWorkerGlobalScope::ReadyToEvaluateScript() {
-  DCHECK(!evaluate_script_ready_);
-  evaluate_script_ready_ = true;
-  if (evaluate_script_)
-    std::move(evaluate_script_).Run();
+  ReadyToRunClassicScript();
 }
 
 bool ServiceWorkerGlobalScope::ShouldInstallV8Extensions() const {
@@ -584,26 +581,6 @@ bool ServiceWorkerGlobalScope::AddEventListenerInternal(
                                                      options);
 }
 
-void ServiceWorkerGlobalScope::EvaluateClassicScriptInternal(
-    const KURL& script_url,
-    String source_code,
-    std::unique_ptr<Vector<uint8_t>> cached_meta_data) {
-  DCHECK(IsContextThread());
-
-  // TODO(nhiroki): Move this mechanism to
-  // WorkerGlobalScope::EvaluateClassicScriptInternal().
-  if (!evaluate_script_ready_) {
-    evaluate_script_ =
-        WTF::Bind(&ServiceWorkerGlobalScope::EvaluateClassicScriptInternal,
-                  WrapWeakPersistent(this), script_url, std::move(source_code),
-                  std::move(cached_meta_data));
-    return;
-  }
-
-  WorkerGlobalScope::EvaluateClassicScriptInternal(script_url, source_code,
-                                                   std::move(cached_meta_data));
-}
-
 const AtomicString& ServiceWorkerGlobalScope::InterfaceName() const {
   return event_target_names::kServiceWorkerGlobalScope;
 }

third_party/blink/renderer/modules/service_worker/service_worker_global_scope.h

@@ -185,12 +185,6 @@ class MODULES_EXPORT ServiceWorkerGlobalScope final : public WorkerGlobalScope {
       EventListener*,
       const AddEventListenerOptionsResolved*) override;
 
-  // WorkerGlobalScope
-  void EvaluateClassicScriptInternal(
-      const KURL& script_url,
-      String source_code,
-      std::unique_ptr<Vector<uint8_t>> cached_meta_data) override;
-
  private:
   void importScripts(const HeapVector<StringOrTrustedScriptURL>& urls,
                      ExceptionState&) override;
@@ -236,9 +230,6 @@ class MODULES_EXPORT ServiceWorkerGlobalScope final : public WorkerGlobalScope {
   uint64_t cache_storage_installed_script_total_size_ = 0;
   uint64_t cache_storage_installed_script_metadata_total_size_ = 0;
 
-  bool evaluate_script_ready_ = false;
-  base::OnceClosure evaluate_script_;
-
   // May be provided in the constructor as an optimization so InterfaceProvider
   // doesn't need to be used. Taken at the initial call to
   // ServiceWorkerGlobalScope#caches.