Make RunLoop::Quit() thread-safe.

base/run_loop.cc

Index: base/run_loop.cc
diff --git a/base/run_loop.cc b/base/run_loop.cc
index 6de1707b8f6885e9b2eb2e9be5882311f2752de5..87128c99d16087d03067af47f477f0628d21497f 100644
--- a/base/run_loop.cc
+++ b/base/run_loop.cc
@@ -5,8 +5,11 @@
 #include "base/run_loop.h"
 
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/lazy_instance.h"
+#include "base/single_thread_task_runner.h"
 #include "base/threading/thread_local.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/tracked_objects.h"
 #include "build/build_config.h"
 
@@ -17,6 +20,17 @@ namespace {
 LazyInstance<ThreadLocalPointer<RunLoop::Delegate>>::Leaky tls_delegate =
     LAZY_INSTANCE_INITIALIZER;
 
+// Runs |closure| immediately if this is called on |task_runner|, otherwise
+// forwards |closure| to it.
+void ProxyToTaskRunner(scoped_refptr<SequencedTaskRunner> task_runner,
+                       OnceClosure closure) {
+  if (task_runner->RunsTasksInCurrentSequence()) {
+    std::move(closure).Run();
+    return;
+  }
+  task_runner->PostTask(FROM_HERE, std::move(closure));
+}
+
 }  // namespace
 
 RunLoop::Delegate::Delegate() {
@@ -63,9 +77,13 @@ RunLoop::Delegate::Client* RunLoop::RegisterDelegateForCurrentThread(
   return &delegate->client_interface_;
 }
 
-RunLoop::RunLoop() : delegate_(tls_delegate.Get().Get()), weak_factory_(this) {
+RunLoop::RunLoop()
+    : delegate_(tls_delegate.Get().Get()),
+      origin_task_runner_(ThreadTaskRunnerHandle::Get()),
+      weak_factory_(this) {
   // A RunLoop::Delegate must be bound to this thread prior to using RunLoop.
   DCHECK(delegate_);
+  DCHECK(origin_task_runner_);
 }
 
 RunLoop::~RunLoop() {
@@ -108,7 +126,16 @@ void RunLoop::RunUntilIdle() {
 }
 
 void RunLoop::Quit() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // Thread-safe.
+
+  // This can only be hit if run_loop->Quit() is called directly (QuitClosure()
+  // proxies through ProxyToTaskRunner() as it can only deref its WeakPtr on
+  // |origin_task_runner_|).
+  if (!origin_task_runner_->RunsTasksInCurrentSequence()) {
+    origin_task_runner_->PostTask(FROM_HERE,
+                                  base::Bind(&RunLoop::Quit, Unretained(this)));
+    return;
+  }
 
   quit_called_ = true;
   if (running_ && delegate_->active_run_loops_.top() == this) {
@@ -118,20 +145,41 @@ void RunLoop::Quit() {
 }
 
 void RunLoop::QuitWhenIdle() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // Thread-safe.
+
+  // This can only be hit if run_loop->QuitWhenIdle() is called directly
+  // (QuitWhenIdleClosure() proxies through ProxyToTaskRunner() as it can only
+  // deref its WeakPtr on |origin_task_runner_|).
+  if (!origin_task_runner_->RunsTasksInCurrentSequence()) {
+    origin_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&RunLoop::QuitWhenIdle, Unretained(this)));
+    return;
+  }
+
   quit_when_idle_received_ = true;
 }
 
 base::Closure RunLoop::QuitClosure() {
   // TODO(gab): Fix bad usage and enable this check, http://crbug.com/715235.
   // DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return base::Bind(&RunLoop::Quit, weak_factory_.GetWeakPtr());
+
+  // Need to use ProxyToTaskRunner() as WeakPtrs vended from
+  // |weak_factory_| may only be accessed on |origin_task_runner_|.
+  // TODO(gab): It feels wrong that QuitClosure() is bound to a WeakPtr.
+  return base::Bind(&ProxyToTaskRunner, origin_task_runner_,
+                    base::Bind(&RunLoop::Quit, weak_factory_.GetWeakPtr()));
 }
 
 base::Closure RunLoop::QuitWhenIdleClosure() {
   // TODO(gab): Fix bad usage and enable this check, http://crbug.com/715235.
   // DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return base::Bind(&RunLoop::QuitWhenIdle, weak_factory_.GetWeakPtr());
+
+  // Need to use ProxyToTaskRunner() as WeakPtrs vended from
+  // |weak_factory_| may only be accessed on |origin_task_runner_|.
+  // TODO(gab): It feels wrong that QuitWhenIdleClosure() is bound to a WeakPtr.
+  return base::Bind(
+      &ProxyToTaskRunner, origin_task_runner_,
+      base::Bind(&RunLoop::QuitWhenIdle, weak_factory_.GetWeakPtr()));
 }
 
 // static
@@ -175,8 +223,10 @@ void RunLoop::DisallowNestingOnCurrentThread() {
 bool RunLoop::BeforeRun() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+#if DCHECK_IS_ON()
   DCHECK(!run_called_);
   run_called_ = true;
+#endif  // DCHECK_IS_ON()
 
   // Allow Quit to be called before Run.
   if (quit_called_)