Introduce RunLoop::OverrideDelegateForCurrentThreadForTesting().

This is a prerequisite to enable ScopedTaskEnvironment MOCK_TIME
on top of any RunLoop::Delegate (i.e. on top of MessageLoopForUI/ForIO).

This CL also removes RunLoop::Delegate::Client::IsNested() as it
was a mere shortcut for its TLS complement and had to switch to
using TLS itself to remain valid in override scenarios...
Ran base_perftests.exe --gtest_filter=*MessageLoop* in static/Release
and things look the same.

Bug: 708584
Change-Id: I143f6e6afb47de11f95702c337dbe63eb0887596
Reviewed-on: https://chromium-review.googlesource.com/784214
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521228}

base/message_loop/message_loop.cc

@@ -334,7 +334,7 @@ void MessageLoop::Run(bool application_tasks_allowed) {
   DCHECK_EQ(this, current());
   if (application_tasks_allowed && !task_execution_allowed_) {
     // Allow nested task execution as explicitly requested.
-    DCHECK(run_loop_client_->IsNested());
+    DCHECK(RunLoop::IsNestedOnCurrentThread());
     task_execution_allowed_ = true;
     pump_->Run(this);
     task_execution_allowed_ = false;
@@ -363,7 +363,7 @@ void MessageLoop::SetThreadTaskRunnerHandle() {
 }
 
 bool MessageLoop::ProcessNextDelayedNonNestableTask() {
-  if (run_loop_client_->IsNested())
+  if (RunLoop::IsNestedOnCurrentThread())
     return false;
 
   while (incoming_task_queue_->deferred_tasks().HasTasks()) {
@@ -399,7 +399,7 @@ void MessageLoop::RunTask(PendingTask* pending_task) {
 
 bool MessageLoop::DeferOrRunPendingTask(PendingTask pending_task) {
   if (pending_task.nestable == Nestable::kNestable ||
-      !run_loop_client_->IsNested()) {
+      !RunLoop::IsNestedOnCurrentThread()) {
     RunTask(&pending_task);
     // Show that we ran a task (Note: a new one might arrive as a
     // consequence!).

base/run_loop.cc

@@ -50,13 +50,8 @@ RunLoop::Delegate::~Delegate() {
 bool RunLoop::Delegate::Client::ShouldQuitWhenIdle() const {
   DCHECK_CALLED_ON_VALID_THREAD(outer_->bound_thread_checker_);
   DCHECK(outer_->bound_);
-  return outer_->active_run_loops_.top()->quit_when_idle_received_;
-}
-
-bool RunLoop::Delegate::Client::IsNested() const {
-  DCHECK_CALLED_ON_VALID_THREAD(outer_->bound_thread_checker_);
-  DCHECK(outer_->bound_);
-  return outer_->active_run_loops_.size() > 1;
+  return outer_->was_overriden_ ||
+         outer_->active_run_loops_.top()->quit_when_idle_received_;
 }
 
 RunLoop::Delegate::Client::Client(Delegate* outer) : outer_(outer) {}
@@ -76,6 +71,27 @@ RunLoop::Delegate::Client* RunLoop::RegisterDelegateForCurrentThread(
   return &delegate->client_interface_;
 }
 
+// static
+std::pair<RunLoop::Delegate::Client*, RunLoop::Delegate*>
+RunLoop::OverrideDelegateForCurrentThreadForTesting(Delegate* delegate) {
+  // Bind |delegate| to this thread.
+  DCHECK(!delegate->bound_);
+  DCHECK_CALLED_ON_VALID_THREAD(delegate->bound_thread_checker_);
+
+  // Overriding cannot be performed while running.
+  DCHECK(!IsRunningOnCurrentThread());
+
+  // Override the current Delegate (there must be one).
+  Delegate* overriden_delegate = tls_delegate.Get().Get();
+  DCHECK(overriden_delegate);
+  DCHECK(overriden_delegate->bound_);
+  overriden_delegate->was_overriden_ = true;
+  tls_delegate.Get().Set(delegate);
+  delegate->bound_ = true;
+
+  return std::make_pair(&delegate->client_interface_, overriden_delegate);
+}
+
 RunLoop::RunLoop(Type type)
     : delegate_(tls_delegate.Get().Get()),
       type_(type),

base/run_loop.h

@@ -5,6 +5,7 @@
 #ifndef BASE_RUN_LOOP_H_
 #define BASE_RUN_LOOP_H_
 
+#include <utility>
 #include <vector>
 
 #include "base/base_export.h"
@@ -159,10 +160,11 @@ class BASE_EXPORT RunLoop {
   class BASE_EXPORT Delegate {
    public:
     Delegate();
-    ~Delegate();
+    virtual ~Delegate();
 
     // The client interface provided back to the caller who registers this
-    // Delegate via RegisterDelegateForCurrentThread.
+    // Delegate via RegisterDelegateForCurrentThread() or
+    // OverrideDelegateForCurrentThreadForTesting().
     class BASE_EXPORT Client {
      public:
       // Returns true if the Delegate should return from the topmost Run() when
@@ -170,11 +172,6 @@ class BASE_EXPORT RunLoop {
       // becomes idle.
       bool ShouldQuitWhenIdle() const;
 
-      // Returns true if this |outer_| is currently in nested runs. This is a
-      // shortcut for RunLoop::IsNestedOnCurrentThread() for the owner of this
-      // interface.
-      bool IsNested() const;
-
      private:
       // Only a Delegate can instantiate a Delegate::Client.
       friend class Delegate;
@@ -221,6 +218,12 @@ class BASE_EXPORT RunLoop {
     RunLoopStack active_run_loops_;
     ObserverList<RunLoop::NestingObserver> nesting_observers_;
 
+    // True if this Delegate was overriden through
+    // OverrideDelegateForCurrentThreadForTesting(). It will from then on always
+    // return from Run() when idle (i.e. its Client's ShouldQuitWhenIdle() will
+    // always be true).
+    bool was_overriden_ = false;
+
 #if DCHECK_IS_ON()
     bool allow_running_for_testing_ = true;
 #endif
@@ -243,6 +246,15 @@ class BASE_EXPORT RunLoop {
   // Delegate::Client is valid as long as |delegate| is kept alive.
   static Delegate::Client* RegisterDelegateForCurrentThread(Delegate* delegate);
 
+  // Akin to RegisterDelegateForCurrentThread but overrides an existing Delegate
+  // (there must be one). Returning |delegate|'s client interface like
+  // RegisterDelegateForCurrentThread() as well as the overriden Delegate which
+  // the caller is now in charge of driving. The overriden Delegate's Run()
+  // method will always return when idle (or earlier if the Delegate's Quit()
+  // method is explicitly invoked).
+  static std::pair<Delegate::Client*, Delegate*>
+  OverrideDelegateForCurrentThreadForTesting(Delegate* delegate);
+
   // Quits the active RunLoop (when idle) -- there must be one. These were
   // introduced as prefered temporary replacements to the long deprecated
   // MessageLoop::Quit(WhenIdle) methods. Callers should properly plumb a

base/run_loop_unittest.cc

@@ -87,7 +87,7 @@ class SimpleSingleThreadTaskRunner : public SingleThreadTaskRunner {
     return origin_thread_checker_.CalledOnValidThread();
   }
 
-  bool ProcessTask() {
+  bool ProcessSingleTask() {
     OnceClosure task;
     {
       AutoLock auto_lock(tasks_lock_);
@@ -97,11 +97,18 @@ class SimpleSingleThreadTaskRunner : public SingleThreadTaskRunner {
       pending_tasks_.pop();
     }
     // It's important to Run() after pop() and outside the lock as |task| may
-    // run a nested loop which will re-enter ProcessTask().
+    // run a nested loop which will re-enter ProcessSingleTask().
     std::move(task).Run();
     return true;
   }
 
+  base::queue<OnceClosure> TakePendingTasks() {
+    AutoLock auto_lock(tasks_lock_);
+    base::queue<OnceClosure> pending_tasks;
+    std::swap(pending_tasks, pending_tasks_);
+    return pending_tasks;
+  }
+
  private:
   ~SimpleSingleThreadTaskRunner() override = default;
 
@@ -116,49 +123,66 @@ class SimpleSingleThreadTaskRunner : public SingleThreadTaskRunner {
   DISALLOW_COPY_AND_ASSIGN(SimpleSingleThreadTaskRunner);
 };
 
+// The basis of all TestDelegates, allows safely injecting a OnceClosure to be
+// run in the next idle phase of this delegate's Run() implementation. This can
+// be used to have code run on a thread that is otherwise livelocked in an idle
+// phase (sometimes a simple PostTask() won't do it -- e.g. when processing
+// application tasks is disallowed).
+class InjectableTestDelegate : public RunLoop::Delegate {
+ public:
+  void InjectClosureOnDelegate(OnceClosure closure) {
+    AutoLock auto_lock(closure_lock_);
+    closure_ = std::move(closure);
+  }
+
+  bool RunInjectedClosure() {
+    AutoLock auto_lock(closure_lock_);
+    if (closure_.is_null())
+      return false;
+    std::move(closure_).Run();
+    return true;
+  }
+
+ private:
+  Lock closure_lock_;
+  OnceClosure closure_;
+};
+
 // A simple test RunLoop::Delegate to exercise Runloop logic independent of any
-// other base constructs.
-class TestDelegate final : public RunLoop::Delegate {
+// other base constructs. BindToCurrentThread() must be called before this
+// TestBoundDelegate is operational.
+class TestBoundDelegate final : public InjectableTestDelegate {
  public:
-  TestDelegate() = default;
+  TestBoundDelegate() = default;
 
+  // Makes this TestBoundDelegate become the RunLoop::Delegate and
+  // ThreadTaskRunnerHandle for this thread.
   void BindToCurrentThread() {
+    DCHECK(!run_loop_client_);
     thread_task_runner_handle_ =
         std::make_unique<ThreadTaskRunnerHandle>(simple_task_runner_);
     run_loop_client_ = RunLoop::RegisterDelegateForCurrentThread(this);
   }
 
-  // Runs |closure| on the TestDelegate thread as part of Run(). Useful to
-  // inject code in an otherwise livelocked Run() state.
-  void RunClosureOnDelegate(OnceClosure closure) {
-    AutoLock auto_lock(closure_lock_);
-    closure_ = std::move(closure);
-  }
-
  private:
   void Run(bool application_tasks_allowed) override {
     if (nested_run_allowing_tasks_incoming_) {
-      EXPECT_TRUE(run_loop_client_->IsNested());
+      EXPECT_TRUE(RunLoop::IsNestedOnCurrentThread());
       EXPECT_TRUE(application_tasks_allowed);
-    } else if (run_loop_client_->IsNested()) {
+    } else if (RunLoop::IsNestedOnCurrentThread()) {
       EXPECT_FALSE(application_tasks_allowed);
     }
     nested_run_allowing_tasks_incoming_ = false;
 
     while (!should_quit_) {
-      if (application_tasks_allowed && simple_task_runner_->ProcessTask())
+      if (application_tasks_allowed && simple_task_runner_->ProcessSingleTask())
         continue;
 
       if (run_loop_client_->ShouldQuitWhenIdle())
         break;
 
-      {
-        AutoLock auto_lock(closure_lock_);
-        if (!closure_.is_null()) {
-          std::move(closure_).Run();
-          continue;
-        }
-      }
+      if (RunInjectedClosure())
+        continue;
 
       PlatformThread::YieldCurrentThread();
     }
@@ -177,12 +201,83 @@ class TestDelegate final : public RunLoop::Delegate {
 
   scoped_refptr<SimpleSingleThreadTaskRunner> simple_task_runner_ =
       MakeRefCounted<SimpleSingleThreadTaskRunner>();
+
   std::unique_ptr<ThreadTaskRunnerHandle> thread_task_runner_handle_;
 
   bool should_quit_ = false;
 
-  Lock closure_lock_;
-  OnceClosure closure_;
+  RunLoop::Delegate::Client* run_loop_client_ = nullptr;
+};
+
+// A test RunLoop::Delegate meant to override an existing RunLoop::Delegate.
+// TakeOverCurrentThread() must be called before this TestBoundDelegate is
+// operational.
+class TestOverridingDelegate final : public InjectableTestDelegate {
+ public:
+  TestOverridingDelegate() = default;
+
+  // Overrides the existing RunLoop::Delegate and ThreadTaskRunnerHandles on
+  // this thread with this TestOverridingDelegate's.
+  void TakeOverCurrentThread() {
+    DCHECK(!run_loop_client_);
+
+    overriden_task_runner_ = ThreadTaskRunnerHandle::Get();
+    DCHECK(overriden_task_runner_);
+    thread_task_runner_handle_override_scope_ =
+        ThreadTaskRunnerHandle::OverrideForTesting(
+            simple_task_runner_,
+            ThreadTaskRunnerHandle::OverrideType::kTakeOverThread);
+
+    auto delegate_override_pair =
+        RunLoop::OverrideDelegateForCurrentThreadForTesting(this);
+    run_loop_client_ = delegate_override_pair.first;
+    overriden_delegate_ = delegate_override_pair.second;
+    DCHECK(overriden_delegate_);
+  }
+
+ private:
+  void Run(bool application_tasks_allowed) override {
+    while (!should_quit_) {
+      auto pending_tasks = simple_task_runner_->TakePendingTasks();
+      if (!pending_tasks.empty()) {
+        while (!pending_tasks.empty()) {
+          overriden_task_runner_->PostTask(FROM_HERE,
+                                           std::move(pending_tasks.front()));
+          pending_tasks.pop();
+        }
+        overriden_delegate_->Run(application_tasks_allowed);
+        continue;
+      }
+
+      if (run_loop_client_->ShouldQuitWhenIdle())
+        break;
+
+      if (RunInjectedClosure())
+        continue;
+
+      PlatformThread::YieldCurrentThread();
+    }
+    should_quit_ = false;
+  }
+
+  void Quit() override {
+    should_quit_ = true;
+    overriden_delegate_->Quit();
+  }
+
+  void EnsureWorkScheduled() override {
+    overriden_delegate_->EnsureWorkScheduled();
+  }
+
+  scoped_refptr<SimpleSingleThreadTaskRunner> simple_task_runner_ =
+      MakeRefCounted<SimpleSingleThreadTaskRunner>();
+
+  ScopedClosureRunner thread_task_runner_handle_override_scope_;
+
+  scoped_refptr<SingleThreadTaskRunner> overriden_task_runner_;
+  RunLoop::Delegate* overriden_delegate_;
+
+  bool should_quit_ = false;
 
   RunLoop::Delegate::Client* run_loop_client_ = nullptr;
 };
@@ -195,6 +290,10 @@ enum class RunLoopTestType {
   // Runs all RunLoopTests under a test RunLoop::Delegate to make sure the
   // delegate interface fully works standalone.
   kTestDelegate,
+
+  // Runs all RunLoopTests through a RunLoop::Delegate which overrides a
+  // kRealEnvironment's registered RunLoop::Delegate.
+  kOverridingTestDelegate,
 };
 
 // The task environment for the RunLoopTest of a given type. A separate class
@@ -203,20 +302,31 @@ class RunLoopTestEnvironment {
  public:
   RunLoopTestEnvironment(RunLoopTestType type) {
     switch (type) {
-      case RunLoopTestType::kRealEnvironment:
+      case RunLoopTestType::kRealEnvironment: {
         task_environment_ = std::make_unique<test::ScopedTaskEnvironment>();
         break;
-      case RunLoopTestType::kTestDelegate:
-        test_delegate_ = std::make_unique<TestDelegate>();
-        test_delegate_->BindToCurrentThread();
+      }
+      case RunLoopTestType::kTestDelegate: {
+        auto test_delegate = std::make_unique<TestBoundDelegate>();
+        test_delegate->BindToCurrentThread();
+        test_delegate_ = std::move(test_delegate);
+        break;
+      }
+      case RunLoopTestType::kOverridingTestDelegate: {
+        task_environment_ = std::make_unique<test::ScopedTaskEnvironment>();
+        auto test_delegate = std::make_unique<TestOverridingDelegate>();
+        test_delegate->TakeOverCurrentThread();
+        test_delegate_ = std::move(test_delegate);
         break;
+      }
     }
   }
 
  private:
-  // Instantiates one or the other based on the RunLoopTestType.
+  // Instantiates one or the other based on the RunLoopTestType (or both in the
+  // kOverridingTestDelegate case).
   std::unique_ptr<test::ScopedTaskEnvironment> task_environment_;
-  std::unique_ptr<TestDelegate> test_delegate_;
+  std::unique_ptr<InjectableTestDelegate> test_delegate_;
 };
 
 class RunLoopTest : public testing::TestWithParam<RunLoopTestType> {
@@ -539,15 +649,19 @@ INSTANTIATE_TEST_CASE_P(Real,
 INSTANTIATE_TEST_CASE_P(Mock,
                         RunLoopTest,
                         testing::Values(RunLoopTestType::kTestDelegate));
+INSTANTIATE_TEST_CASE_P(
+    OverridingMock,
+    RunLoopTest,
+    testing::Values(RunLoopTestType::kOverridingTestDelegate));
 
 TEST(RunLoopDeathTest, MustRegisterBeforeInstantiating) {
-  TestDelegate unbound_test_delegate_;
+  TestBoundDelegate unbound_test_delegate_;
   // Exercise the DCHECK in RunLoop::RunLoop().
   EXPECT_DCHECK_DEATH({ RunLoop(); });
 }
 
 TEST(RunLoopDelegateTest, NestableTasksDontRunInDefaultNestedLoops) {
-  TestDelegate test_delegate;
+  TestBoundDelegate test_delegate;
   test_delegate.BindToCurrentThread();
 
   base::Thread other_thread("test");
@@ -599,8 +713,8 @@ TEST(RunLoopDelegateTest, NestableTasksDontRunInDefaultNestedLoops) {
   other_thread.task_runner()->PostDelayedTask(
       FROM_HERE,
       BindOnce(
-          [](TestDelegate* test_delegate, OnceClosure injected_closure) {
-            test_delegate->RunClosureOnDelegate(std::move(injected_closure));
+          [](TestBoundDelegate* test_delegate, OnceClosure injected_closure) {
+            test_delegate->InjectClosureOnDelegate(std::move(injected_closure));
           },
           Unretained(&test_delegate), nested_run_loop.QuitWhenIdleClosure()),
       TestTimeouts::tiny_timeout());

base/threading/thread_task_runner_handle.cc

@@ -39,7 +39,8 @@ bool ThreadTaskRunnerHandle::IsSet() {
 
 // static
 ScopedClosureRunner ThreadTaskRunnerHandle::OverrideForTesting(
-    scoped_refptr<SingleThreadTaskRunner> overriding_task_runner) {
+    scoped_refptr<SingleThreadTaskRunner> overriding_task_runner,
+    ThreadTaskRunnerHandle::OverrideType type) {
   // OverrideForTesting() is not compatible with a SequencedTaskRunnerHandle
   // being set (but SequencedTaskRunnerHandle::IsSet() includes
   // ThreadTaskRunnerHandle::IsSet() so that's discounted as the only valid
@@ -67,7 +68,9 @@ ScopedClosureRunner ThreadTaskRunnerHandle::OverrideForTesting(
   ttrh->task_runner_.swap(overriding_task_runner);
 
   auto no_running_during_override =
-      std::make_unique<RunLoop::ScopedDisallowRunningForTesting>();
+      type == OverrideType::kTakeOverThread
+          ? nullptr
+          : std::make_unique<RunLoop::ScopedDisallowRunningForTesting>();
 
   return ScopedClosureRunner(base::Bind(
       [](scoped_refptr<SingleThreadTaskRunner> task_runner_to_restore,

base/threading/thread_task_runner_handle.h

@@ -34,9 +34,19 @@ class BASE_EXPORT ThreadTaskRunnerHandle {
   // ScopedClosureRunners expire in LIFO (stack) order. Note: nesting
   // ThreadTaskRunnerHandles isn't generally desired but it's useful in unit
   // tests where multiple task runners can share the main thread for simplicity
-  // and determinism.
+  // and determinism (in which case RunLoop::Run() is banned for the scope of
+  // the override as it would execute tasks from the wrong task runner). It's
+  // also useful in unit test frameworks in which a task runner takes over the
+  // main thread; in that case it's fine to allow running through
+  // |type = kTakeOverThread| iff RunLoop::Run() will result in running tasks
+  // posted to the overriding ThreadTaskRunnerHandle.
+  enum class OverrideType {
+    kDefault,
+    kTakeOverThread,
+  };
   static ScopedClosureRunner OverrideForTesting(
-      scoped_refptr<SingleThreadTaskRunner> overriding_task_runner);
+      scoped_refptr<SingleThreadTaskRunner> overriding_task_runner,
+      OverrideType type = OverrideType::kDefault);
 
   // Binds |task_runner| to the current thread. |task_runner| must belong
   // to the current thread for this to succeed.