workers: Use base::TaskRunner to run tasks on worker threads.

Use the worker thread's TaskRunner to post-task to the worker threads,
instead of using WorkerThreadTaskRunner, which adds an extra level of
indirection for posting tasks without any benefits. This is the last place
WorkerThreadTaskRunner was being used. So remove it.

BUG=457033
R=kinuko@chromium.org, nasko@chromium.org

Review URL: https://codereview.chromium.org/903363002

Cr-Commit-Position: refs/heads/master@{#315829}

9 files changed, 69 insertions, 98 deletions

diff --git a/content/child/bluetooth/bluetooth_dispatcher.cc b/content/child/bluetooth/bluetooth_dispatcher.cc
index 4775424..0bd7f2b 100644
--- a/content/child/bluetooth/bluetooth_dispatcher.cc
+++ b/content/child/bluetooth/bluetooth_dispatcher.cc
@@ -9,7 +9,6 @@
 #include "base/message_loop/message_loop.h"
 #include "base/thread_task_runner_handle.h"
 #include "content/child/thread_safe_sender.h"
-#include "content/child/worker_thread_task_runner.h"
 #include "content/common/bluetooth/bluetooth_messages.h"
 #include "third_party/WebKit/public/platform/WebBluetoothDevice.h"
 #include "third_party/WebKit/public/platform/WebBluetoothError.h"
diff --git a/content/child/geofencing/geofencing_dispatcher.cc b/content/child/geofencing/geofencing_dispatcher.cc
index ef8ae9b..37ba161 100644
--- a/content/child/geofencing/geofencing_dispatcher.cc
+++ b/content/child/geofencing/geofencing_dispatcher.cc
@@ -10,7 +10,6 @@
 #include "base/thread_task_runner_handle.h"
 #include "content/child/service_worker/web_service_worker_registration_impl.h"
 #include "content/child/thread_safe_sender.h"
-#include "content/child/worker_thread_task_runner.h"
 #include "content/common/geofencing_messages.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "third_party/WebKit/public/platform/WebCircularGeofencingRegion.h"
diff --git a/content/child/worker_task_runner.cc b/content/child/worker_task_runner.cc
index 4fc5bde..82410fc 100644
--- a/content/child/worker_task_runner.cc
+++ b/content/child/worker_task_runner.cc
@@ -6,9 +6,13 @@
 
 #include "base/callback.h"
 #include "base/lazy_instance.h"
+#include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
+#include "base/single_thread_task_runner.h"
+#include "base/stl_util.h"
+#include "base/thread_task_runner_handle.h"
 
 using blink::WebWorkerRunLoop;
 
@@ -16,43 +20,49 @@ namespace content {
 
 namespace {
 
-class RunClosureTask : public WebWorkerRunLoop::Task {
+// A task-runner that refuses to run any tasks.
+class DoNothingTaskRunner : public base::TaskRunner {
  public:
-  RunClosureTask(const base::Closure& task) : task_(task) {}
-  virtual ~RunClosureTask() {}
-  virtual void Run() {
-    task_.Run();
-  }
+  DoNothingTaskRunner() {}
+
  private:
-  base::Closure task_;
+  ~DoNothingTaskRunner() override {}
+
+  bool PostDelayedTask(const tracked_objects::Location& from_here,
+                       const base::Closure& task,
+                       base::TimeDelta delay) override {
+    return false;
+  }
+
+  bool RunsTasksOnCurrentThread() const override { return false; }
 };
 
-} // namespace
+}  // namespace
 
 struct WorkerTaskRunner::ThreadLocalState {
   ThreadLocalState() {}
   ObserverList<WorkerTaskRunner::Observer> stop_observers_;
 };
 
-WorkerTaskRunner::WorkerTaskRunner() {
+WorkerTaskRunner::WorkerTaskRunner()
+    : task_runner_for_dead_worker_(new DoNothingTaskRunner()) {
 }
 
 bool WorkerTaskRunner::PostTask(
     int id, const base::Closure& closure) {
   DCHECK(id > 0);
-  base::AutoLock locker(loop_map_lock_);
-  IDToLoopMap::iterator found = loop_map_.find(id);
-  if (found == loop_map_.end())
+  base::AutoLock locker(task_runner_map_lock_);
+  IDToTaskRunnerMap::iterator found = task_runner_map_.find(id);
+  if (found == task_runner_map_.end())
     return false;
-  return found->second.postTask(new RunClosureTask(closure));
+  return found->second->PostTask(FROM_HERE, closure);
 }
 
 int WorkerTaskRunner::PostTaskToAllThreads(const base::Closure& closure) {
-  base::AutoLock locker(loop_map_lock_);
-  IDToLoopMap::iterator it;
-  for (it = loop_map_.begin(); it != loop_map_.end(); ++it)
-    it->second.postTask(new RunClosureTask(closure));
-  return static_cast<int>(loop_map_.size());
+  base::AutoLock locker(task_runner_map_lock_);
+  for (const auto& it : task_runner_map_)
+    it.second->PostTask(FROM_HERE, closure);
+  return static_cast<int>(task_runner_map_.size());
 }
 
 int WorkerTaskRunner::CurrentWorkerId() {
@@ -86,8 +96,9 @@ void WorkerTaskRunner::OnWorkerRunLoopStarted(const WebWorkerRunLoop& loop) {
   current_tls_.Set(new ThreadLocalState());
 
   int id = base::PlatformThread::CurrentId();
-  base::AutoLock locker_(loop_map_lock_);
-  loop_map_[id] = loop;
+  base::AutoLock locker_(task_runner_map_lock_);
+  task_runner_map_[id] = base::ThreadTaskRunnerHandle::Get().get();
+  CHECK(task_runner_map_[id]);
 }
 
 void WorkerTaskRunner::OnWorkerRunLoopStopped(const WebWorkerRunLoop& loop) {
@@ -95,12 +106,17 @@ void WorkerTaskRunner::OnWorkerRunLoopStopped(const WebWorkerRunLoop& loop) {
   FOR_EACH_OBSERVER(Observer, current_tls_.Get()->stop_observers_,
                     OnWorkerRunLoopStopped());
   {
-    base::AutoLock locker(loop_map_lock_);
-    DCHECK(loop_map_[CurrentWorkerId()] == loop);
-    loop_map_.erase(CurrentWorkerId());
+    base::AutoLock locker(task_runner_map_lock_);
+    task_runner_map_.erase(CurrentWorkerId());
   }
   delete current_tls_.Get();
   current_tls_.Set(NULL);
 }
 
+base::TaskRunner* WorkerTaskRunner::GetTaskRunnerFor(int worker_id) {
+  base::AutoLock locker(task_runner_map_lock_);
+  return ContainsKey(task_runner_map_, worker_id) ? task_runner_map_[worker_id]
+                                           : task_runner_for_dead_worker_.get();
+}
+
 }  // namespace content
diff --git a/content/child/worker_task_runner.h b/content/child/worker_task_runner.h
index 6d6c472..8e9ff28 100644
--- a/content/child/worker_task_runner.h
+++ b/content/child/worker_task_runner.h
@@ -8,12 +8,17 @@
 #include <map>
 
 #include "base/callback_forward.h"
+#include "base/memory/ref_counted.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_local.h"
 #include "content/common/content_export.h"
 #include "third_party/WebKit/public/platform/WebWorkerRunLoop.h"
 
+namespace base {
+class TaskRunner;
+}
+
 namespace content {
 
 class CONTENT_EXPORT WorkerTaskRunner {
@@ -39,18 +44,28 @@ class CONTENT_EXPORT WorkerTaskRunner {
   void OnWorkerRunLoopStarted(const blink::WebWorkerRunLoop& loop);
   void OnWorkerRunLoopStopped(const blink::WebWorkerRunLoop& loop);
 
+  base::TaskRunner* GetTaskRunnerFor(int worker_id);
+
  private:
   friend class WorkerTaskRunnerTest;
 
-  typedef std::map<base::PlatformThreadId, blink::WebWorkerRunLoop> IDToLoopMap;
+  using IDToTaskRunnerMap = std::map<base::PlatformThreadId, base::TaskRunner*>;
 
   ~WorkerTaskRunner();
 
+  // It is possible for an IPC message to arrive for a worker thread that has
+  // already gone away. In such cases, it is still necessary to provide a
+  // task-runner for that particular thread, because otherwise the message will
+  // end up being handled as per usual in the main-thread, causing incorrect
+  // results. |task_runner_for_dead_worker_| is used to handle such messages,
+  // which silently discards all the tasks it receives.
+  scoped_refptr<base::TaskRunner> task_runner_for_dead_worker_;
+
   struct ThreadLocalState;
   base::ThreadLocalPointer<ThreadLocalState> current_tls_;
 
-  IDToLoopMap loop_map_;
-  base::Lock loop_map_lock_;
+  IDToTaskRunnerMap task_runner_map_;
+  base::Lock task_runner_map_lock_;
 };
 
 }  // namespace content
diff --git a/content/child/worker_task_runner_unittest.cc b/content/child/worker_task_runner_unittest.cc
index 530c8a2..ca231d1 100644
--- a/content/child/worker_task_runner_unittest.cc
+++ b/content/child/worker_task_runner_unittest.cc
@@ -5,6 +5,7 @@
 #include "content/child/worker_task_runner.h"
 
 #include "base/logging.h"
+#include "base/message_loop/message_loop.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -19,6 +20,9 @@ class WorkerTaskRunnerTest : public testing::Test {
     task_runner_.OnWorkerRunLoopStopped(blink::WebWorkerRunLoop());
   }
   WorkerTaskRunner task_runner_;
+
+ private:
+  base::MessageLoop message_loop_;
 };
 
 class MockObserver : public WorkerTaskRunner::Observer {
diff --git a/content/child/worker_thread_message_filter.cc b/content/child/worker_thread_message_filter.cc
index ac11a3d..aa11b3b 100644
--- a/content/child/worker_thread_message_filter.cc
+++ b/content/child/worker_thread_message_filter.cc
@@ -6,7 +6,7 @@
 
 #include "base/thread_task_runner_handle.h"
 #include "content/child/thread_safe_sender.h"
-#include "content/child/worker_thread_task_runner.h"
+#include "content/child/worker_task_runner.h"
 #include "ipc/ipc_message_macros.h"
 
 namespace content {
@@ -29,12 +29,18 @@ base::TaskRunner* WorkerThreadMessageFilter::OverrideTaskRunnerForMessage(
   DCHECK(success);
   if (!ipc_thread_id)
     return main_thread_task_runner_.get();
-  return new WorkerThreadTaskRunner(ipc_thread_id);
+  return WorkerTaskRunner::Instance()->GetTaskRunnerFor(ipc_thread_id);
 }
 
 bool WorkerThreadMessageFilter::OnMessageReceived(const IPC::Message& msg) {
   if (!ShouldHandleMessage(msg))
     return false;
+  // If the IPC message is received in a worker thread, but it has already been
+  // stopped, then drop the message.
+  if (!main_thread_task_runner_->BelongsToCurrentThread() &&
+      !WorkerTaskRunner::Instance()->CurrentWorkerId()) {
+    return false;
+  }
   OnFilteredMessageReceived(msg);
   return true;
 }
diff --git a/content/child/worker_thread_task_runner.cc b/content/child/worker_thread_task_runner.cc
deleted file mode 100644
index da7bc91..0000000
--- a/content/child/worker_thread_task_runner.cc
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright 2013 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "content/child/worker_thread_task_runner.h"
-
-#include "base/logging.h"
-#include "content/child/worker_task_runner.h"
-
-namespace content {
-
-WorkerThreadTaskRunner::WorkerThreadTaskRunner(int worker_thread_id)
-    : worker_thread_id_(worker_thread_id) {
-}
-
-bool WorkerThreadTaskRunner::PostDelayedTask(
-    const tracked_objects::Location& /* from_here */,
-    const base::Closure& task,
-    base::TimeDelta delay) {
-  // Currently non-zero delay is not supported.
-  DCHECK(!delay.ToInternalValue());
-  return WorkerTaskRunner::Instance()->PostTask(worker_thread_id_, task);
-}
-
-bool WorkerThreadTaskRunner::RunsTasksOnCurrentThread() const {
-  return worker_thread_id_ == WorkerTaskRunner::Instance()->CurrentWorkerId();
-}
-
-WorkerThreadTaskRunner::~WorkerThreadTaskRunner() {}
-
-}  // namespace content
diff --git a/content/child/worker_thread_task_runner.h b/content/child/worker_thread_task_runner.h
deleted file mode 100644
index 8e70128..0000000
--- a/content/child/worker_thread_task_runner.h
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright 2013 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CONTENT_CHILD_WORKER_THREAD_TASK_RUNNER_H_
-#define CONTENT_CHILD_WORKER_THREAD_TASK_RUNNER_H_
-
-#include "base/task_runner.h"
-
-namespace content {
-
-// A task runner that runs tasks on a single webkit worker thread which
-// is managed by WorkerTaskRunner.
-// Note that this implementation ignores the delay duration for PostDelayedTask
-// and have it behave the same as PostTask.
-class WorkerThreadTaskRunner : public base::TaskRunner {
- public:
-  explicit WorkerThreadTaskRunner(int worker_thread_id);
-
-  // TaskRunner overrides.
-  bool PostDelayedTask(const tracked_objects::Location& from_here,
-                       const base::Closure& task,
-                       base::TimeDelta delay) override;
-  bool RunsTasksOnCurrentThread() const override;
-
- protected:
-  ~WorkerThreadTaskRunner() override;
-
- private:
-  const int worker_thread_id_;
-};
-
-}  // namespace content
-
-#endif  // CONTENT_CHILD_WORKER_THREAD_TASK_RUNNER_H_
diff --git a/content/content_child.gypi b/content/content_child.gypi
index fedf374..509fed3 100644
--- a/content/content_child.gypi
+++ b/content/content_child.gypi
@@ -275,8 +275,6 @@
       'child/worker_task_runner.h',
       'child/worker_thread_message_filter.cc',
       'child/worker_thread_message_filter.h',
-      'child/worker_thread_task_runner.cc',
-      'child/worker_thread_task_runner.h',
     ],
     'webcrypto_nss_sources': [
       'child/webcrypto/nss/aes_algorithm_nss.cc',