Loosen thread-safety checks and update documentation on RunLoop.

base/run_loop.cc

 // Copyright (c) 2012 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 "base/run_loop.h"
 
 #include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/threading/thread_local.h"
 #include "base/tracked_objects.h"
@@ skipping 52 matching lines @@
   return &delegate->client_interface_;
 }
 
 RunLoop::RunLoop() : delegate_(tls_delegate.Get().Get()), weak_factory_(this) {
   // A RunLoop::Delegate must be bound to this thread prior to using RunLoop.
   DCHECK(delegate_);
 }
 
 RunLoop::~RunLoop() {
   // TODO(gab): Fix bad usage and enable this check, http://crbug.com/715235.
-  // DCHECK(thread_checker_.CalledOnValidThread());
+  // DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
 void RunLoop::Run() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   if (!BeforeRun())
     return;
 
+  // It is okay to access this RunLoop from another sequence while Run() is
+  // active as this RunLoop won't touch its state until after that returns (if
+  // the RunLoop's state is accessed while processing Run(), it will be re-bound
+  // to the accessing sequence for the remainder of that Run() -- accessing from
+  // multiple sequences is still disallowed).
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+
   // Use task stopwatch to exclude the loop run time from the current task, if
   // any.
   tracked_objects::TaskStopwatch stopwatch;
   stopwatch.Start();
   delegate_->Run();
   stopwatch.Stop();
 
+  // Rebind this RunLoop to the current thread after Run().

[danakj, 2017/05/18 15:25:27]

current sequence?

[gab, 2017/05/18 16:14:02]

I debated that but in practice RunLoop always runs on a thread so we can still
sequence check to avoid over-specifying but the bound sequence is a thread --
using a SequenceChecker instead of ThreadChecker is still nice as it allows
another sequence (not necessarily thread) to do checks on the RunLoop while it's
running (detached from thread above)

+  DETACH_FROM_SEQUENCE(sequence_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   AfterRun();
 }
 
 void RunLoop::RunUntilIdle() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   quit_when_idle_received_ = true;
   Run();
 }
 
 void RunLoop::Quit() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   quit_called_ = true;
   if (running_ && delegate_->active_run_loops_.top() == this) {
     // This is the inner-most RunLoop, so quit now.
     delegate_->Quit();
   }
 }
 
 void RunLoop::QuitWhenIdle() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   quit_when_idle_received_ = true;
 }
 
 base::Closure RunLoop::QuitClosure() {
   // TODO(gab): Fix bad usage and enable this check, http://crbug.com/715235.
-  // DCHECK(thread_checker_.CalledOnValidThread());
+  // DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return 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(thread_checker_.CalledOnValidThread());
+  // DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return base::Bind(&RunLoop::QuitWhenIdle, weak_factory_.GetWeakPtr());
 }
 
 // static
 bool RunLoop::IsRunningOnCurrentThread() {
   Delegate* delegate = tls_delegate.Get().Get();
   return delegate && !delegate->active_run_loops_.empty();
 }
 
 // static
@@ skipping 22 matching lines @@
 bool RunLoop::IsNestingAllowedOnCurrentThread() {
   return tls_delegate.Get().Get()->allow_nesting_;
 }
 
 // static
 void RunLoop::DisallowNestingOnCurrentThread() {
   tls_delegate.Get().Get()->allow_nesting_ = false;
 }
 
 bool RunLoop::BeforeRun() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   DCHECK(!run_called_);
   run_called_ = true;
 
   // Allow Quit to be called before Run.
   if (quit_called_)
     return false;
 
   auto& active_run_loops_ = delegate_->active_run_loops_;
   active_run_loops_.push(this);
 
   const bool is_nested = active_run_loops_.size() > 1;
 
   if (is_nested) {
     CHECK(delegate_->allow_nesting_);
     for (auto& observer : delegate_->nesting_observers_)
       observer.OnBeginNestedRunLoop();
   }
 
   running_ = true;
   return true;
 }
 
 void RunLoop::AfterRun() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   running_ = false;
 
   auto& active_run_loops_ = delegate_->active_run_loops_;
   DCHECK_EQ(active_run_loops_.top(), this);
   active_run_loops_.pop();
 
   RunLoop* previous_run_loop =
       active_run_loops_.empty() ? nullptr : active_run_loops_.top();
 
   // Execute deferred QuitNow, if any:
   if (previous_run_loop && previous_run_loop->quit_called_)
     delegate_->Quit();
 }
 
 }  // namespace base