Make TestMockTimeTaskRunner a RunLoop::Delegate.
Introducing TestMockTimeTaskRunner::Type::kBound which will make that
TestMockTimeTaskRunner takeover the thread it's created on (a la
MessageLoop), enabling RunLoop and Thread/SequencedTaskRunnerHandle.
Also introduces RunLoop::ScopedDisallowRunningForTesting to enforce
mutual exclusion TestMockTimeTaskRunner::ScopedContext (used to toggle
context to another task runner on the main thread) and RunLoop::Run()
(meant to run the current thread's associated task runner). Mixing the
two would result in running the incorrect task runner. While I don't
think this is a use case worth supporting, experience with //base
APIs has taught me that if there's a way to use it wrong, someone
will, and it's much easier to prevent than to heal; hence this check.
(there should already be no RunLoop usage during TaskRunnerHandle
overrides per RunLoop not being previously supported by
TestMockTimeTaskRunner and this check ensures it stays that way :))
EDIT: Well except that HeartbeatSenderTest had found a way to use it
the deprecated way, but it's a nice fit for using a kBound
TestMockTimeTaskRunner so all good :).
Had to drop support for virtual
TestMockTimeTaskRunner::IsElapsingStopped() which in turn forced
removal of custom task runner in remote_commands_service_unittest.cc
whose use case is now supported by the new API :).
This enables follow-ups to:
1) Add mock time to base::test::ScopedTaskEnvironment :)
https://docs.google.com/document/d/1QabRo8c7D9LsYY3cEcaPQbOCLo8Tu-6VLykYXyl3Pkk/edit
2) Fixing a race in base::Timer which requires RunLoop from
TestMockTimeTaskRunner to get rid of the two remaining problematic
use cases (see bug blocked by 703346).
Bug: 703346
Change-Id: I062b77b669853a36c30813e44dd984d01fcefbe2
[email protected] (for components/policy test side-effects)
Change-Id: I062b77b669853a36c30813e44dd984d01fcefbe2
Reviewed-on: https://chromium-review.googlesource.com/614788
Reviewed-by: Gabriel Charette <[email protected]>
Reviewed-by: Dirk Pranke <[email protected]>
Reviewed-by: Scott Nichols <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Gabriel Charette <[email protected]>
Cr-Commit-Position: refs/heads/master@{#496111}diff --git a/base/run_loop.cc b/base/run_loop.cc
index b11e71364..96af51e 100644
--- a/base/run_loop.cc
+++ b/base/run_loop.cc
@@ -244,10 +244,40 @@
tls_delegate.Get().Get()->active_run_loops_.top()->QuitWhenIdle();
}
+#if DCHECK_IS_ON()
+RunLoop::ScopedDisallowRunningForTesting::ScopedDisallowRunningForTesting()
+ : current_delegate_(tls_delegate.Get().Get()),
+ previous_run_allowance_(
+ current_delegate_ ? current_delegate_->allow_running_for_testing_
+ : false) {
+ if (current_delegate_)
+ current_delegate_->allow_running_for_testing_ = false;
+}
+
+RunLoop::ScopedDisallowRunningForTesting::~ScopedDisallowRunningForTesting() {
+ DCHECK_EQ(current_delegate_, tls_delegate.Get().Get());
+ if (current_delegate_)
+ current_delegate_->allow_running_for_testing_ = previous_run_allowance_;
+}
+#else // DCHECK_IS_ON()
+// Defined out of line so that the compiler doesn't inline these and realize
+// the scope has no effect and then throws an "unused variable" warning in
+// non-dcheck builds.
+RunLoop::ScopedDisallowRunningForTesting::ScopedDisallowRunningForTesting() =
+ default;
+RunLoop::ScopedDisallowRunningForTesting::~ScopedDisallowRunningForTesting() =
+ default;
+#endif // DCHECK_IS_ON()
+
bool RunLoop::BeforeRun() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
#if DCHECK_IS_ON()
+ DCHECK(delegate_->allow_running_for_testing_)
+ << "RunLoop::Run() isn't allowed in the scope of a "
+ "ScopedDisallowRunningForTesting. Hint: if mixing "
+ "TestMockTimeTaskRunners on same thread, use TestMockTimeTaskRunner's "
+ "API instead of RunLoop to drive individual task runners.";
DCHECK(!run_called_);
run_called_ = true;
#endif // DCHECK_IS_ON()
