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}diff --git a/base/message_loop/message_loop.cc b/base/message_loop/message_loop.cc
index 2c8899bf..519eb8f 100644
--- a/base/message_loop/message_loop.cc
+++ b/base/message_loop/message_loop.cc
@@ -334,7 +334,7 @@
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 @@
}
bool MessageLoop::ProcessNextDelayedNonNestableTask() {
- if (run_loop_client_->IsNested())
+ if (RunLoop::IsNestedOnCurrentThread())
return false;
while (incoming_task_queue_->deferred_tasks().HasTasks()) {
@@ -399,7 +399,7 @@
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!).
diff --git a/base/run_loop.cc b/base/run_loop.cc
index 61230c62..880625a 100644
--- a/base/run_loop.cc
+++ b/base/run_loop.cc
@@ -50,13 +50,8 @@
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 @@
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),
diff --git a/base/run_loop.h b/base/run_loop.h
index 57aae610..bdaeda5 100644
--- a/base/run_loop.h
+++ b/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 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 @@
// 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 @@
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 @@
// 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
diff --git a/base/run_loop_unittest.cc b/base/run_loop_unittest.cc
index beebda3..1f626ca 100644
--- a/base/run_loop_unittest.cc
+++ b/base/run_loop_unittest.cc
@@ -87,7 +87,7 @@
return origin_thread_checker_.CalledOnValidThread();
}
- bool ProcessTask() {
+ bool ProcessSingleTask() {
OnceClosure task;
{
AutoLock auto_lock(tasks_lock_);
@@ -97,11 +97,18 @@
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 @@
DISALLOW_COPY_AND_ASSIGN(SimpleSingleThreadTaskRunner);
};
-// A simple test RunLoop::Delegate to exercise Runloop logic independent of any
-// other base constructs.
-class TestDelegate final : public RunLoop::Delegate {
+// 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:
- TestDelegate() = default;
+ 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. BindToCurrentThread() must be called before this
+// TestBoundDelegate is operational.
+class TestBoundDelegate final : public InjectableTestDelegate {
+ public:
+ 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 @@
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 @@
// 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 @@
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(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 @@
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());
diff --git a/base/threading/thread_task_runner_handle.cc b/base/threading/thread_task_runner_handle.cc
index 1d9756f..1e27675d 100644
--- a/base/threading/thread_task_runner_handle.cc
+++ b/base/threading/thread_task_runner_handle.cc
@@ -39,7 +39,8 @@
// 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 @@
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,
diff --git a/base/threading/thread_task_runner_handle.h b/base/threading/thread_task_runner_handle.h
index 480a03c..a24d68b 100644
--- a/base/threading/thread_task_runner_handle.h
+++ b/base/threading/thread_task_runner_handle.h
@@ -34,9 +34,19 @@
// 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.
