Worker: Fix a race condition on task runner handling

WebSharedWorkerImpl accesses WorkerScheduler from the main thread to
take a task runner, and then dispatches a connect event to
SharedWorkerGlobalScope using the task runner.

This causes a race condition if close() is called on the global scope
on the worker thread while the task runner is being taken on the main
thread: close() call disposes of WorkerScheduler, and accessing the
scheduler after that is not allowed. See the issue for details.

To fix this, this CL makes WebSharedWorkerImpl capture the task runner
between starting a worker thread (initializing WorkerScheduler) and
posting a task to evaluate worker scripts that may call close(). This
ensures that WebSharedWorkerImpl accesses WorkerScheduler before the
scheduler is disposed of.

Bug: 1104046
Change-Id: I145cd39f706019c33220fcb01ed81f76963ffff0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2308550
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790284}

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

@@ -168,11 +168,9 @@ void WebSharedWorkerImpl::Connect(int connection_request_id,
 
 void WebSharedWorkerImpl::ConnectToChannel(int connection_request_id,
                                            MessagePortChannel channel) {
-  // The HTML spec requires to queue a connect event using the DOM manipulation
-  // task source.
-  // https://html.spec.whatwg.org/C/#shared-workers-and-the-sharedworker-interface
+  DCHECK(IsMainThread());
   PostCrossThreadTask(
-      *GetWorkerThread()->GetTaskRunner(TaskType::kDOMManipulation), FROM_HERE,
+      *task_runner_for_connect_event_, FROM_HERE,
       CrossThreadBindOnce(&WebSharedWorkerImpl::ConnectTaskOnWorkerThread,
                           WTF::CrossThreadUnretained(this),
                           WTF::Passed(std::move(channel))));
@@ -180,6 +178,7 @@ void WebSharedWorkerImpl::ConnectToChannel(int connection_request_id,
 }
 
 void WebSharedWorkerImpl::DispatchPendingConnections() {
+  DCHECK(IsMainThread());
   for (auto& item : pending_channels_)
     ConnectToChannel(item.first, std::move(item.second));
   pending_channels_.clear();
@@ -293,6 +292,18 @@ void WebSharedWorkerImpl::StartWorkerContext(
 
   GetWorkerThread()->Start(std::move(creation_params), thread_startup_data,
                            std::move(devtools_params));
+
+  // Capture the task runner for dispatching connect events. This is necessary
+  // for avoiding race condition with WorkerScheduler termination induced by
+  // close() call on SharedWorkerGlobalScope. See https://crbug.com/1104046 for
+  // details.
+  //
+  // The HTML spec requires to queue a connect event using the DOM manipulation
+  // task source.
+  // https://html.spec.whatwg.org/C/#shared-workers-and-the-sharedworker-interface
+  task_runner_for_connect_event_ =
+      GetWorkerThread()->GetTaskRunner(TaskType::kDOMManipulation);
+
   switch (script_type) {
     case mojom::ScriptType::kClassic:
       GetWorkerThread()->FetchAndRunClassicScript(

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

@@ -138,6 +138,8 @@ class CORE_EXPORT WebSharedWorkerImpl final : public WebSharedWorker {
       std::pair<int /* connection_request_id */, blink::MessagePortChannel>;
   Vector<PendingChannel> pending_channels_;
 
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_for_connect_event_;
+
   bool running_ = false;
   bool asked_to_terminate_ = false;