Pausable Object Cleanup - Workers

Remove the paused_calls vector on WorkerGlobalScope because all
the calls executed occur on pausable task queues after
http://crrev.com/1eb2d22.

BUG=907125

Change-Id: I42ad827eb1131f643158f9c6e565d69886896ccc
Reviewed-on: https://chromium-review.googlesource.com/c/1385015
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618379}

third_party/blink/renderer/core/exported/web_shared_worker_impl.cc

@@ -189,7 +189,7 @@ void WebSharedWorkerImpl::ConnectTaskOnWorkerThread(
   // event.
   DCHECK(worker_thread_->IsCurrentThread());
   auto* scope = To<SharedWorkerGlobalScope>(worker_thread_->GlobalScope());
-  scope->ConnectPausable(std::move(channel));
+  scope->Connect(std::move(channel));
 }
 
 void WebSharedWorkerImpl::StartWorkerContext(

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

@@ -77,7 +77,7 @@ void DedicatedWorkerObjectProxy::ProcessMessageFromWorkerObject(
     BlinkTransferableMessage message,
     WorkerThread* worker_thread) {
   To<WorkerGlobalScope>(worker_thread->GlobalScope())
-      ->ReceiveMessagePausable(std::move(message));
+      ->ReceiveMessage(std::move(message));
 }
 
 void DedicatedWorkerObjectProxy::ProcessUnhandledException(

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

@@ -72,13 +72,8 @@ void SharedWorkerGlobalScope::ImportModuleScript(
   NOTREACHED();
 }
 
-void SharedWorkerGlobalScope::ConnectPausable(MessagePortChannel channel) {
-  if (IsContextPaused()) {
-    AddPausedCall(WTF::Bind(&SharedWorkerGlobalScope::ConnectPausable,
-                            WrapWeakPersistent(this), std::move(channel)));
-    return;
-  }
-
+void SharedWorkerGlobalScope::Connect(MessagePortChannel channel) {
+  DCHECK(!IsContextPaused());
   MessagePort* port = MessagePort::Create(*this);
   port->Entangle(std::move(channel));
   MessageEvent* event =

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

@@ -67,7 +67,7 @@ class CORE_EXPORT SharedWorkerGlobalScope final : public WorkerGlobalScope {
   DEFINE_ATTRIBUTE_EVENT_LISTENER(connect, kConnect);
   String name() const { return name_; }
 
-  void ConnectPausable(MessagePortChannel channel);
+  void Connect(MessagePortChannel channel);
 
   void Trace(blink::Visitor*) override;
 

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

@@ -106,7 +106,6 @@ KURL WorkerGlobalScope::CompleteURL(const String& url) const {
 void WorkerGlobalScope::Dispose() {
   DCHECK(IsContextThread());
   closing_ = true;
-  paused_calls_.clear();
   WorkerOrWorkletGlobalScope::Dispose();
 }
 
@@ -343,56 +342,28 @@ ExecutionContext* WorkerGlobalScope::GetExecutionContext() const {
   return const_cast<WorkerGlobalScope*>(this);
 }
 
-void WorkerGlobalScope::TasksWereUnpaused() {
-  WorkerOrWorkletGlobalScope::TasksWereUnpaused();
-  // We cannot run the paused tasks right away, as there might be some other
-  // code that still needs to be run synchronously.
-  GetTaskRunner(TaskType::kInternalWorker)
-      ->PostTask(FROM_HERE, WTF::Bind(&WorkerGlobalScope::MaybeRunPausedTasks,
-                                      WrapWeakPersistent(this)));
-}
-
-void WorkerGlobalScope::MaybeRunPausedTasks() {
-  if (IsContextPaused())
-    return;
-  Vector<base::OnceClosure> calls;
-  paused_calls_.swap(calls);
-  for (auto& call : calls)
-    std::move(call).Run();
-}
-
-void WorkerGlobalScope::EvaluateClassicScriptPausable(
+void WorkerGlobalScope::EvaluateClassicScript(
     const KURL& script_url,
     String source_code,
     std::unique_ptr<Vector<char>> cached_meta_data,
     const v8_inspector::V8StackTraceId& stack_id) {
-  if (IsContextPaused()) {
-    AddPausedCall(WTF::Bind(&WorkerGlobalScope::EvaluateClassicScriptPausable,
-                            WrapWeakPersistent(this), script_url, source_code,
-                            WTF::Passed(std::move(cached_meta_data)),
-                            stack_id));
-    return;
-  }
+  DCHECK(!IsContextPaused());
   ThreadDebugger* debugger = ThreadDebugger::From(GetThread()->GetIsolate());
   if (debugger)
     debugger->ExternalAsyncTaskStarted(stack_id);
-  EvaluateClassicScript(script_url, source_code, std::move(cached_meta_data));
+  EvaluateClassicScriptInternal(script_url, source_code,
+                                std::move(cached_meta_data));
   if (debugger)
     debugger->ExternalAsyncTaskFinished(stack_id);
 }
 
 // https://html.spec.whatwg.org/multipage/workers.html#worker-processing-model
-void WorkerGlobalScope::ImportClassicScriptPausable(
+void WorkerGlobalScope::ImportClassicScript(
     const KURL& script_url,
     FetchClientSettingsObjectSnapshot* outside_settings_object,
     const v8_inspector::V8StackTraceId& stack_id) {
   DCHECK(RuntimeEnabledFeatures::OffMainThreadWorkerScriptFetchEnabled());
-  if (IsContextPaused()) {
-    AddPausedCall(WTF::Bind(&WorkerGlobalScope::ImportClassicScriptPausable,
-                            WrapWeakPersistent(this), script_url,
-                            WrapPersistent(outside_settings_object), stack_id));
-    return;
-  }
+  DCHECK(!IsContextPaused());
 
   // Step 12. "Fetch a classic worker script given url, outside settings,
   // destination, and inside settings."
@@ -466,33 +437,13 @@ void WorkerGlobalScope::DidImportClassicScript(
 
   // Step 13.7. "Asynchronously complete the perform the fetch steps with
   // response."
-
-  EvaluateClassicScriptPausable(
+  EvaluateClassicScript(
       classic_script_loader->ResponseURL(), classic_script_loader->SourceText(),
       classic_script_loader->ReleaseCachedMetadata(), stack_id);
 }
 
-void WorkerGlobalScope::ImportModuleScriptPausable(
-    const KURL& module_url_record,
-    FetchClientSettingsObjectSnapshot* outside_settings_object,
-    network::mojom::FetchCredentialsMode mode) {
-  if (IsContextPaused()) {
-    AddPausedCall(WTF::Bind(&WorkerGlobalScope::ImportModuleScriptPausable,
-                            WrapWeakPersistent(this), module_url_record,
-                            WrapPersistent(outside_settings_object), mode));
-    return;
-  }
-  ImportModuleScript(module_url_record, outside_settings_object, mode);
-}
-
-void WorkerGlobalScope::ReceiveMessagePausable(
-    BlinkTransferableMessage message) {
-  if (IsContextPaused()) {
-    AddPausedCall(WTF::Bind(&WorkerGlobalScope::ReceiveMessagePausable,
-                            WrapWeakPersistent(this), std::move(message)));
-    return;
-  }
-
+void WorkerGlobalScope::ReceiveMessage(BlinkTransferableMessage message) {
+  DCHECK(!IsContextPaused());
   MessagePortArray* ports =
       MessagePort::EntanglePorts(*this, std::move(message.ports));
   ThreadDebugger* debugger = ThreadDebugger::From(GetThread()->GetIsolate());
@@ -510,7 +461,7 @@ void WorkerGlobalScope::ReceiveMessagePausable(
     debugger->ExternalAsyncTaskFinished(message.sender_stack_trace_id);
 }
 
-void WorkerGlobalScope::EvaluateClassicScript(
+void WorkerGlobalScope::EvaluateClassicScriptInternal(
     const KURL& script_url,
     String source_code,
     std::unique_ptr<Vector<char>> cached_meta_data) {
@@ -531,10 +482,6 @@ void WorkerGlobalScope::EvaluateClassicScript(
   ReportingProxy().DidEvaluateClassicScript(success);
 }
 
-void WorkerGlobalScope::AddPausedCall(base::OnceClosure closure) {
-  paused_calls_.push_back(std::move(closure));
-}
-
 WorkerGlobalScope::WorkerGlobalScope(
     std::unique_ptr<GlobalScopeCreationParams> creation_params,
     WorkerThread* thread,

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

@@ -141,23 +141,24 @@ class CORE_EXPORT WorkerGlobalScope
   ExecutionContext* GetExecutionContext() const final;
   bool IsWindowOrWorkerGlobalScope() const final { return true; }
 
-  // The following methods implement PausbaleObject semantic
-  // so that WorkerGlobalScope can be paused.
-  void EvaluateClassicScriptPausable(
-      const KURL& script_url,
-      String source_code,
-      std::unique_ptr<Vector<char>> cached_meta_data,
-      const v8_inspector::V8StackTraceId& stack_id);
-  void ImportClassicScriptPausable(
+  // These methods should be called in the scope of a pausable
+  // task runner. ie. They should not be called when the context
+  // is paused.
+  void EvaluateClassicScript(const KURL& script_url,
+                             String source_code,
+                             std::unique_ptr<Vector<char>> cached_meta_data,
+                             const v8_inspector::V8StackTraceId& stack_id);
+  void ImportClassicScript(
       const KURL& script_url,
       FetchClientSettingsObjectSnapshot* outside_settings_object,
       const v8_inspector::V8StackTraceId& stack_id);
-  void ImportModuleScriptPausable(
+  // Imports the top-level module script for |module_url_record|.
+  virtual void ImportModuleScript(
       const KURL& module_url_record,
       FetchClientSettingsObjectSnapshot* outside_settings_object,
-      network::mojom::FetchCredentialsMode);
-  void ReceiveMessagePausable(BlinkTransferableMessage);
+      network::mojom::FetchCredentialsMode) = 0;
 
+  void ReceiveMessage(BlinkTransferableMessage);
   base::TimeTicks TimeOrigin() const { return time_origin_; }
   WorkerSettings* GetWorkerSettings() const { return worker_settings_.get(); }
 
@@ -191,21 +192,11 @@ class CORE_EXPORT WorkerGlobalScope
   void RemoveURLFromMemoryCache(const KURL&) final;
 
   // Evaluates the given top-level classic script.
-  virtual void EvaluateClassicScript(
+  virtual void EvaluateClassicScriptInternal(
       const KURL& script_url,
       String source_code,
       std::unique_ptr<Vector<char>> cached_meta_data);
 
-  // Imports the top-level module script for |module_url_record|.
-  virtual void ImportModuleScript(
-      const KURL& module_url_record,
-      FetchClientSettingsObjectSnapshot* outside_settings_object,
-      network::mojom::FetchCredentialsMode) = 0;
-
-  void AddPausedCall(base::OnceClosure closure);
-
-  void MaybeRunPausedTasks();
-
   mojom::ScriptType GetScriptType() const { return script_type_; }
 
  private:
@@ -252,7 +243,6 @@ class CORE_EXPORT WorkerGlobalScope
 
   // ExecutionContext
   EventTarget* ErrorEventTarget() final { return this; }
-  void TasksWereUnpaused() override;
 
   const KURL url_;
   const mojom::ScriptType script_type_;
@@ -284,8 +274,6 @@ class CORE_EXPORT WorkerGlobalScope
   const base::UnguessableToken agent_cluster_id_;
 
   HttpsState https_state_;
-
-  Vector<base::OnceClosure> paused_calls_;
 };
 
 template <>

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

@@ -500,8 +500,8 @@ void WorkerThread::EvaluateClassicScriptOnWorkerThread(
     std::unique_ptr<Vector<char>> cached_meta_data,
     const v8_inspector::V8StackTraceId& stack_id) {
   To<WorkerGlobalScope>(GlobalScope())
-      ->EvaluateClassicScriptPausable(script_url, std::move(source_code),
-                                      std::move(cached_meta_data), stack_id);
+      ->EvaluateClassicScript(script_url, std::move(source_code),
+                              std::move(cached_meta_data), stack_id);
 }
 
 void WorkerThread::ImportClassicScriptOnWorkerThread(
@@ -510,7 +510,7 @@ void WorkerThread::ImportClassicScriptOnWorkerThread(
         outside_settings_object,
     const v8_inspector::V8StackTraceId& stack_id) {
   To<WorkerGlobalScope>(GlobalScope())
-      ->ImportClassicScriptPausable(
+      ->ImportClassicScript(
           script_url,
           MakeGarbageCollected<FetchClientSettingsObjectSnapshot>(
               std::move(outside_settings_object)),
@@ -526,7 +526,7 @@ void WorkerThread::ImportModuleScriptOnWorkerThread(
   // TODO(nhiroki): Consider excluding this code path from WorkerThread like
   // Worklets.
   To<WorkerGlobalScope>(GlobalScope())
-      ->ImportModuleScriptPausable(
+      ->ImportModuleScript(
           script_url,
           MakeGarbageCollected<FetchClientSettingsObjectSnapshot>(
               std::move(outside_settings_object)),

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

@@ -135,62 +135,6 @@ bool ServiceWorkerGlobalScope::ShouldInstallV8Extensions() const {
   return Platform::Current()->AllowScriptExtensionForServiceWorker(Url());
 }
 
-void ServiceWorkerGlobalScope::EvaluateClassicScript(
-    const KURL& script_url,
-    String source_code,
-    std::unique_ptr<Vector<char>> cached_meta_data) {
-  DCHECK(IsContextThread());
-
-  if (!evaluate_script_ready_) {
-    evaluate_script_ =
-        WTF::Bind(&ServiceWorkerGlobalScope::EvaluateClassicScript,
-                  WrapWeakPersistent(this), script_url, std::move(source_code),
-                  std::move(cached_meta_data));
-    return;
-  }
-
-  // Receive the main script via script streaming if needed.
-  InstalledScriptsManager* installed_scripts_manager =
-      GetThread()->GetInstalledScriptsManager();
-  if (installed_scripts_manager &&
-      installed_scripts_manager->IsScriptInstalled(script_url)) {
-    // GetScriptData blocks until the script is received from the browser.
-    std::unique_ptr<InstalledScriptsManager::ScriptData> script_data =
-        installed_scripts_manager->GetScriptData(script_url);
-    if (!script_data) {
-      ReportingProxy().DidFailToLoadInstalledClassicScript();
-      // This will eventually initiate worker thread termination. See
-      // ServiceWorkerGlobalScopeProxy::DidCloseWorkerGlobalScope() for details.
-      close();
-      return;
-    }
-
-    DCHECK(source_code.IsEmpty());
-    DCHECK(!cached_meta_data);
-    source_code = script_data->TakeSourceText();
-    cached_meta_data = script_data->TakeMetaData();
-
-    base::Optional<ContentSecurityPolicyResponseHeaders>
-        content_security_policy_raw_headers =
-            script_data->GetContentSecurityPolicyResponseHeaders();
-    ApplyContentSecurityPolicyFromHeaders(
-        content_security_policy_raw_headers.value());
-
-    String referrer_policy = script_data->GetReferrerPolicy();
-    if (!referrer_policy.IsNull())
-      ParseAndSetReferrerPolicy(referrer_policy);
-
-    std::unique_ptr<Vector<String>> origin_trial_tokens =
-        script_data->CreateOriginTrialTokens();
-    OriginTrialContext::AddTokens(this, origin_trial_tokens.get());
-
-    ReportingProxy().DidLoadInstalledScript();
-  }
-
-  WorkerGlobalScope::EvaluateClassicScript(script_url, source_code,
-                                           std::move(cached_meta_data));
-}
-
 void ServiceWorkerGlobalScope::ImportModuleScript(
     const KURL& module_url_record,
     FetchClientSettingsObjectSnapshot* outside_settings_object,
@@ -351,6 +295,62 @@ bool ServiceWorkerGlobalScope::AddEventListenerInternal(
                                                      options);
 }
 
+void ServiceWorkerGlobalScope::EvaluateClassicScriptInternal(
+    const KURL& script_url,
+    String source_code,
+    std::unique_ptr<Vector<char>> cached_meta_data) {
+  DCHECK(IsContextThread());
+
+  if (!evaluate_script_ready_) {
+    evaluate_script_ =
+        WTF::Bind(&ServiceWorkerGlobalScope::EvaluateClassicScriptInternal,
+                  WrapWeakPersistent(this), script_url, std::move(source_code),
+                  std::move(cached_meta_data));
+    return;
+  }
+
+  // Receive the main script via script streaming if needed.
+  InstalledScriptsManager* installed_scripts_manager =
+      GetThread()->GetInstalledScriptsManager();
+  if (installed_scripts_manager &&
+      installed_scripts_manager->IsScriptInstalled(script_url)) {
+    // GetScriptData blocks until the script is received from the browser.
+    std::unique_ptr<InstalledScriptsManager::ScriptData> script_data =
+        installed_scripts_manager->GetScriptData(script_url);
+    if (!script_data) {
+      ReportingProxy().DidFailToLoadInstalledClassicScript();
+      // This will eventually initiate worker thread termination. See
+      // ServiceWorkerGlobalScopeProxy::DidCloseWorkerGlobalScope() for details.
+      close();
+      return;
+    }
+
+    DCHECK(source_code.IsEmpty());
+    DCHECK(!cached_meta_data);
+    source_code = script_data->TakeSourceText();
+    cached_meta_data = script_data->TakeMetaData();
+
+    base::Optional<ContentSecurityPolicyResponseHeaders>
+        content_security_policy_raw_headers =
+            script_data->GetContentSecurityPolicyResponseHeaders();
+    ApplyContentSecurityPolicyFromHeaders(
+        content_security_policy_raw_headers.value());
+
+    String referrer_policy = script_data->GetReferrerPolicy();
+    if (!referrer_policy.IsNull())
+      ParseAndSetReferrerPolicy(referrer_policy);
+
+    std::unique_ptr<Vector<String>> origin_trial_tokens =
+        script_data->CreateOriginTrialTokens();
+    OriginTrialContext::AddTokens(this, origin_trial_tokens.get());
+
+    ReportingProxy().DidLoadInstalledScript();
+  }
+
+  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

@@ -80,10 +80,6 @@ class MODULES_EXPORT ServiceWorkerGlobalScope final : public WorkerGlobalScope {
   bool ShouldInstallV8Extensions() const final;
 
   // Implements WorkerGlobalScope.
-  void EvaluateClassicScript(
-      const KURL& script_url,
-      String source_code,
-      std::unique_ptr<Vector<char>> cached_meta_data) override;
   void ImportModuleScript(
       const KURL& module_url_record,
       FetchClientSettingsObjectSnapshot* outside_settings_object,
@@ -158,6 +154,12 @@ 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<char>> cached_meta_data) override;
+
  private:
   void importScripts(const HeapVector<StringOrTrustedScriptURL>& urls,
                      ExceptionState&) override;