diff --git a/PRESUBMIT.py b/PRESUBMIT.py index cae96c3fca8bad..10d86abe89aa17 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -426,6 +426,14 @@ ), True, (), + ), + ( + 'base::ScopedMockTimeMessageLoopTaskRunner', + ( + 'ScopedMockTimeMessageLoopTaskRunner is deprecated.', + ), + True, + (), ) ) diff --git a/base/BUILD.gn b/base/BUILD.gn index 457d8716c8d21f..5f2a418258cde9 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -2170,6 +2170,7 @@ test("base_unittests") { "test/scoped_feature_list_unittest.cc", "test/scoped_mock_time_message_loop_task_runner_unittest.cc", "test/scoped_task_environment_unittest.cc", + "test/test_mock_time_task_runner_unittest.cc", "test/test_pending_task_unittest.cc", "test/test_reg_util_win_unittest.cc", "test/trace_event_analyzer_unittest.cc", diff --git a/base/run_loop.cc b/base/run_loop.cc index b11e71364877bd..96af51e7f4b1c8 100644 --- a/base/run_loop.cc +++ b/base/run_loop.cc @@ -244,10 +244,40 @@ void RunLoop::QuitCurrentWhenIdleDeprecated() { 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() diff --git a/base/run_loop.h b/base/run_loop.h index 8c6e64967f9482..dcabfc73711a51 100644 --- a/base/run_loop.h +++ b/base/run_loop.h @@ -226,6 +226,10 @@ class BASE_EXPORT RunLoop { RunLoopStack active_run_loops_; ObserverList nesting_observers_; +#if DCHECK_IS_ON() + bool allow_running_for_testing_ = true; +#endif + // True once this Delegate is bound to a thread via // RegisterDelegateForCurrentThread(). bool bound_ = false; @@ -253,6 +257,28 @@ class BASE_EXPORT RunLoop { static void QuitCurrentDeprecated(); static void QuitCurrentWhenIdleDeprecated(); + // Run() will DCHECK if called while there's a ScopedDisallowRunningForTesting + // in scope on its thread. This is useful to add safety to some test + // constructs which allow multiple task runners to share the main thread in + // unit tests. While the main thread can be shared by multiple runners to + // deterministically fake multi threading, there can still only be a single + // RunLoop::Delegate per thread and RunLoop::Run() should only be invoked from + // it (or it would result in incorrectly driving TaskRunner A while in + // TaskRunner B's context). + class BASE_EXPORT ScopedDisallowRunningForTesting { + public: + ScopedDisallowRunningForTesting(); + ~ScopedDisallowRunningForTesting(); + + private: +#if DCHECK_IS_ON() + Delegate* current_delegate_; + const bool previous_run_allowance_; +#endif // DCHECK_IS_ON() + + DISALLOW_COPY_AND_ASSIGN(ScopedDisallowRunningForTesting); + }; + private: #if defined(OS_ANDROID) // Android doesn't support the blocking RunLoop::Run, so it calls diff --git a/base/run_loop_unittest.cc b/base/run_loop_unittest.cc index ac4f8963e31f29..b739242b3540eb 100644 --- a/base/run_loop_unittest.cc +++ b/base/run_loop_unittest.cc @@ -524,6 +524,17 @@ TEST_P(RunLoopTest, DisallowNestingDeathTest) { } #endif // defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID) +TEST_P(RunLoopTest, DisallowRunningForTesting) { + RunLoop::ScopedDisallowRunningForTesting disallow_running; + EXPECT_DCHECK_DEATH({ run_loop_.Run(); }); +} + +TEST_P(RunLoopTest, ExpiredDisallowRunningForTesting) { + { RunLoop::ScopedDisallowRunningForTesting disallow_running; } + // Running should be fine after |disallow_running| goes out of scope. + run_loop_.RunUntilIdle(); +} + INSTANTIATE_TEST_CASE_P(Real, RunLoopTest, testing::Values(RunLoopTestType::kRealEnvironment)); diff --git a/base/test/scoped_mock_time_message_loop_task_runner.h b/base/test/scoped_mock_time_message_loop_task_runner.h index fe0d2c488ec927..dc8d9eebf66283 100644 --- a/base/test/scoped_mock_time_message_loop_task_runner.h +++ b/base/test/scoped_mock_time_message_loop_task_runner.h @@ -21,6 +21,10 @@ class SingleThreadTaskRunner; // Note: RunLoop() will not work in the scope of a // ScopedMockTimeMessageLoopTaskRunner, the underlying TestMockTimeTaskRunner's // methods must be used instead to pump tasks. +// +// DEPRECATED: Use a TestMockTimeTaskRunner::Type::kBoundToThread instead of a +// MessageLoop + ScopedMockTimeMessageLoopTaskRunner. +// TODO(gab): Remove usage of this API and delete it. class ScopedMockTimeMessageLoopTaskRunner { public: ScopedMockTimeMessageLoopTaskRunner(); diff --git a/base/test/test_mock_time_task_runner.cc b/base/test/test_mock_time_task_runner.cc index 9268d7b60eeb3d..4f482de6fc4520 100644 --- a/base/test/test_mock_time_task_runner.cc +++ b/base/test/test_mock_time_task_runner.cc @@ -70,6 +70,49 @@ Time MockClock::Now() { return task_runner_->Now(); } +// A SingleThreadTaskRunner which forwards everything to its |target_|. This is +// useful to break ownership chains when it is known that |target_| will outlive +// the NonOwningProxyTaskRunner it's injected into. In particular, +// TestMockTimeTaskRunner is forced to be ref-counted by virtue of being a +// SingleThreadTaskRunner. As such it is impossible for it to have a +// ThreadTaskRunnerHandle member that points back to itself as the +// ThreadTaskRunnerHandle which it owns would hold a ref back to it. To break +// this dependency cycle, the ThreadTaskRunnerHandle is instead handed a +// NonOwningProxyTaskRunner which allows the TestMockTimeTaskRunner to not hand +// a ref to its ThreadTaskRunnerHandle while promising in return that it will +// outlive that ThreadTaskRunnerHandle instance. +class NonOwningProxyTaskRunner : public SingleThreadTaskRunner { + public: + explicit NonOwningProxyTaskRunner(SingleThreadTaskRunner* target) + : target_(target) { + DCHECK(target_); + } + + // SingleThreadTaskRunner: + bool RunsTasksInCurrentSequence() const override { + return target_->RunsTasksInCurrentSequence(); + } + bool PostDelayedTask(const tracked_objects::Location& from_here, + OnceClosure task, + TimeDelta delay) override { + return target_->PostDelayedTask(from_here, std::move(task), delay); + } + bool PostNonNestableDelayedTask(const tracked_objects::Location& from_here, + OnceClosure task, + TimeDelta delay) override { + return target_->PostNonNestableDelayedTask(from_here, std::move(task), + delay); + } + + private: + friend class RefCountedThreadSafe; + ~NonOwningProxyTaskRunner() override = default; + + SingleThreadTaskRunner* const target_; + + DISALLOW_COPY_AND_ASSIGN(NonOwningProxyTaskRunner); +}; + } // namespace // TestMockTimeTaskRunner::TestOrderedPendingTask ----------------------------- @@ -145,13 +188,19 @@ bool TestMockTimeTaskRunner::TemporalOrder::operator()( return first_task.GetTimeToRun() > second_task.GetTimeToRun(); } -TestMockTimeTaskRunner::TestMockTimeTaskRunner() - : now_(Time::UnixEpoch()), next_task_ordinal_(0) { -} +TestMockTimeTaskRunner::TestMockTimeTaskRunner(Type type) + : TestMockTimeTaskRunner(Time::UnixEpoch(), TimeTicks(), type) {} TestMockTimeTaskRunner::TestMockTimeTaskRunner(Time start_time, - TimeTicks start_ticks) - : now_(start_time), now_ticks_(start_ticks), next_task_ordinal_(0) {} + TimeTicks start_ticks, + Type type) + : now_(start_time), now_ticks_(start_ticks), tasks_lock_cv_(&tasks_lock_) { + if (type == Type::kBoundToThread) { + run_loop_client_ = RunLoop::RegisterDelegateForCurrentThread(this); + thread_task_runner_handle_ = std::make_unique( + MakeRefCounted(this)); + } +} TestMockTimeTaskRunner::~TestMockTimeTaskRunner() { } @@ -256,10 +305,6 @@ bool TestMockTimeTaskRunner::PostNonNestableDelayedTask( return PostDelayedTask(from_here, std::move(task), delay); } -bool TestMockTimeTaskRunner::IsElapsingStopped() { - return false; -} - void TestMockTimeTaskRunner::OnBeforeSelectingTask() { // Empty default implementation. } @@ -285,7 +330,7 @@ void TestMockTimeTaskRunner::ProcessAllTasksNoLaterThan(TimeDelta max_delta) { } const TimeTicks original_now_ticks = now_ticks_; - while (!IsElapsingStopped()) { + while (!quit_run_loop_) { OnBeforeSelectingTask(); TestPendingTask task_info; if (!DequeueNextTask(original_now_ticks, max_delta, &task_info)) @@ -312,6 +357,7 @@ void TestMockTimeTaskRunner::ForwardClocksUntilTickTime(TimeTicks later_ticks) { bool TestMockTimeTaskRunner::DequeueNextTask(const TimeTicks& reference, const TimeDelta& max_delta, TestPendingTask* next_task) { + DCHECK(thread_checker_.CalledOnValidThread()); AutoLock scoped_lock(tasks_lock_); if (!tasks_.empty() && (tasks_.top().GetTimeToRun() - reference) <= max_delta) { @@ -324,4 +370,50 @@ bool TestMockTimeTaskRunner::DequeueNextTask(const TimeTicks& reference, return false; } +void TestMockTimeTaskRunner::Run() { + DCHECK(thread_checker_.CalledOnValidThread()); + + // Since TestMockTimeTaskRunner doesn't process system messages: there's no + // hope for anything but a chrome task to call Quit(). If this RunLoop can't + // process chrome tasks (i.e. disallowed by default in nested RunLoops), it's + // thereby guaranteed to hang... + DCHECK(run_loop_client_->ProcessingTasksAllowed()) + << "This is a nested RunLoop instance and needs to be of " + "Type::NESTABLE_TASKS_ALLOWED."; + + while (!quit_run_loop_) { + RunUntilIdle(); + if (quit_run_loop_ || run_loop_client_->ShouldQuitWhenIdle()) + break; + + // Peek into |tasks_| to perform one of two things: + // A) If there are no remaining tasks, wait until one is posted and + // restart from the top. + // B) If there is a remaining delayed task. Fast-forward to reach the next + // round of tasks. + TimeDelta auto_fast_forward_by; + { + AutoLock scoped_lock(tasks_lock_); + if (tasks_.empty()) { + while (tasks_.empty()) + tasks_lock_cv_.Wait(); + continue; + } + auto_fast_forward_by = tasks_.top().GetTimeToRun() - now_ticks_; + } + FastForwardBy(auto_fast_forward_by); + } + quit_run_loop_ = false; +} + +void TestMockTimeTaskRunner::Quit() { + DCHECK(thread_checker_.CalledOnValidThread()); + quit_run_loop_ = true; +} + +void TestMockTimeTaskRunner::EnsureWorkScheduled() { + // Nothing to do: TestMockTimeTaskRunner::Run() will always process tasks and + // doesn't need an extra kick on nested runs. +} + } // namespace base diff --git a/base/test/test_mock_time_task_runner.h b/base/test/test_mock_time_task_runner.h index 73b0e5315c2f06..6218734037077a 100644 --- a/base/test/test_mock_time_task_runner.h +++ b/base/test/test_mock_time_task_runner.h @@ -15,6 +15,7 @@ #include "base/callback.h" #include "base/callback_helpers.h" #include "base/macros.h" +#include "base/run_loop.h" #include "base/single_thread_task_runner.h" #include "base/synchronization/lock.h" #include "base/test/test_pending_task.h" @@ -25,6 +26,7 @@ namespace base { class Clock; class TickClock; +class ThreadTaskRunnerHandle; // Runs pending tasks in the order of the tasks' post time + delay, and keeps // track of a mock (virtual) tick clock time that can be fast-forwarded. @@ -33,7 +35,7 @@ class TickClock; // // - Methods RunsTasksInCurrentSequence() and Post[Delayed]Task() can be // called from any thread, but the rest of the methods must be called on -// the same thread the TaskRunner was created on. +// the same thread the TestMockTimeTaskRunner was created on. // - It allows for reentrancy, in that it handles the running of tasks that in // turn call back into it (e.g., to post more tasks). // - Tasks are stored in a priority queue, and executed in the increasing @@ -43,17 +45,34 @@ class TickClock; // posted task delays and fast-forward increments is still representable by // a TimeDelta, and that adding this delta to the starting values of Time // and TickTime is still within their respective range. -// - Tasks aren't guaranteed to be destroyed immediately after they're run. +// +// A TestMockTimeTaskRunner of Type::kBoundToThread has the following additional +// properties: +// - Thread/SequencedTaskRunnerHandle refers to it on its thread. +// - It can be driven by a RunLoop on the thread it was created on. +// RunLoop::Run() will result in running non-delayed tasks until idle and +// then, if RunLoop::QuitWhenIdle() wasn't invoked, fast-forwarding time to +// the next delayed task and looping again. And so on, until either +// RunLoop::Quit() is invoked (quits immediately after the current task) or +// RunLoop::QuitWhenIdle() is invoked (quits before having to fast forward +// time once again). Should RunLoop::Run() process all tasks (including +// delayed ones), it will block until more are posted. As usual, +// RunLoop::RunUntilIdle() is equivalent to RunLoop::Run() followed by an +// immediate RunLoop::QuitWhenIdle(). +// - // // This is a slightly more sophisticated version of TestSimpleTaskRunner, in // that it supports running delayed tasks in the correct temporal order. -class TestMockTimeTaskRunner : public SingleThreadTaskRunner { +class TestMockTimeTaskRunner : public SingleThreadTaskRunner, + public RunLoop::Delegate { public: // Everything that is executed in the scope of a ScopedContext will behave as // though it ran under |scope| (i.e. ThreadTaskRunnerHandle, // RunsTasksInCurrentSequence, etc.). This allows the test body to be all in - // one block when multiple TestMockTimeTaskRunners share the main thread. For - // example: + // one block when multiple TestMockTimeTaskRunners share the main thread. + // Note: RunLoop isn't supported: will DCHECK if used inside a ScopedContext. + // + // For example: // // class ExampleFixture { // protected: @@ -86,7 +105,7 @@ class TestMockTimeTaskRunner : public SingleThreadTaskRunner { public: // Note: |scope| is ran until idle as part of this constructor to ensure // that anything which runs in the underlying scope runs after any already - // pending tasks (the contrary would break the SequencedTraskRunner + // pending tasks (the contrary would break the SequencedTaskRunner // contract). explicit ScopedContext(scoped_refptr scope); ~ScopedContext(); @@ -96,12 +115,25 @@ class TestMockTimeTaskRunner : public SingleThreadTaskRunner { DISALLOW_COPY_AND_ASSIGN(ScopedContext); }; + enum class Type { + // A TestMockTimeTaskRunner which can only be driven directly through its + // API. Thread/SequencedTaskRunnerHandle will refer to it only in the scope + // of its tasks. + kStandalone, + // A TestMockTimeTaskRunner which will associate to the thread it is created + // on, enabling RunLoop to drive it and making + // Thread/SequencedTaskRunnerHandle refer to it on that thread. + kBoundToThread, + }; + // Constructs an instance whose virtual time will start at the Unix epoch, and // whose time ticks will start at zero. - TestMockTimeTaskRunner(); + TestMockTimeTaskRunner(Type type = Type::kStandalone); // Constructs an instance starting at the given virtual time and time ticks. - TestMockTimeTaskRunner(Time start_time, TimeTicks start_ticks); + TestMockTimeTaskRunner(Time start_time, + TimeTicks start_ticks, + Type type = Type::kStandalone); // Fast-forwards virtual time by |delta|, causing all tasks with a remaining // delay less than or equal to |delta| to be executed. |delta| must be @@ -150,11 +182,6 @@ class TestMockTimeTaskRunner : public SingleThreadTaskRunner { protected: ~TestMockTimeTaskRunner() override; - // Whether the elapsing of virtual time is stopped or not. Subclasses can - // override this method to perform early exits from a running task runner. - // Defaults to always return false. - virtual bool IsElapsingStopped(); - // Called before the next task to run is selected, so that subclasses have a // last chance to make sure all tasks are posted. virtual void OnBeforeSelectingTask(); @@ -199,6 +226,11 @@ class TestMockTimeTaskRunner : public SingleThreadTaskRunner { const TimeDelta& max_delta, TestPendingTask* next_task); + // RunLoop::Delegate: + void Run() override; + void Quit() override; + void EnsureWorkScheduled() override; + // Also used for non-dcheck logic (RunsTasksInCurrentSequence()) and as such // needs to be a ThreadCheckerImpl. ThreadCheckerImpl thread_checker_; @@ -212,9 +244,19 @@ class TestMockTimeTaskRunner : public SingleThreadTaskRunner { // The ordinal to use for the next task. Must only be accessed while the // |tasks_lock_| is held. - size_t next_task_ordinal_; + size_t next_task_ordinal_ = 0; Lock tasks_lock_; + ConditionVariable tasks_lock_cv_; + + // Members used to in TestMockTimeTaskRunners of Type::kBoundToThread to take + // ownership of the thread it was created on. + RunLoop::Delegate::Client* run_loop_client_ = nullptr; + std::unique_ptr thread_task_runner_handle_; + + // Set to true in RunLoop::Delegate::Quit() to signal the topmost + // RunLoop::Delegate::Run() instance to stop, reset to false when it does. + bool quit_run_loop_ = false; DISALLOW_COPY_AND_ASSIGN(TestMockTimeTaskRunner); }; diff --git a/base/test/test_mock_time_task_runner_unittest.cc b/base/test/test_mock_time_task_runner_unittest.cc new file mode 100644 index 00000000000000..5ecf9d4e8c30c1 --- /dev/null +++ b/base/test/test_mock_time_task_runner_unittest.cc @@ -0,0 +1,181 @@ +// Copyright 2017 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/test/test_mock_time_task_runner.h" + +#include "base/memory/ref_counted.h" +#include "base/run_loop.h" +#include "base/test/gtest_util.h" +#include "base/threading/sequenced_task_runner_handle.h" +#include "base/threading/thread_task_runner_handle.h" +#include "base/time/time.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace base { + +// Basic usage should work the same from default and bound +// TestMockTimeTaskRunners. +TEST(TestMockTimeTaskRunnerTest, Basic) { + static constexpr TestMockTimeTaskRunner::Type kTestCases[] = { + TestMockTimeTaskRunner::Type::kStandalone, + TestMockTimeTaskRunner::Type::kBoundToThread}; + + for (auto type : kTestCases) { + SCOPED_TRACE(static_cast(type)); + + auto mock_time_task_runner = MakeRefCounted(type); + int counter = 0; + + mock_time_task_runner->PostTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 1; }, Unretained(&counter))); + mock_time_task_runner->PostTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 32; }, Unretained(&counter))); + mock_time_task_runner->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 256; }, Unretained(&counter)), + TimeDelta::FromSeconds(3)); + mock_time_task_runner->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 64; }, Unretained(&counter)), + TimeDelta::FromSeconds(1)); + mock_time_task_runner->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 1024; }, + Unretained(&counter)), + TimeDelta::FromMinutes(20)); + mock_time_task_runner->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 4096; }, + Unretained(&counter)), + TimeDelta::FromDays(20)); + + int expected_value = 0; + EXPECT_EQ(expected_value, counter); + mock_time_task_runner->RunUntilIdle(); + expected_value += 1; + expected_value += 32; + EXPECT_EQ(expected_value, counter); + + mock_time_task_runner->RunUntilIdle(); + EXPECT_EQ(expected_value, counter); + + mock_time_task_runner->FastForwardBy(TimeDelta::FromSeconds(1)); + expected_value += 64; + EXPECT_EQ(expected_value, counter); + + mock_time_task_runner->FastForwardBy(TimeDelta::FromSeconds(5)); + expected_value += 256; + EXPECT_EQ(expected_value, counter); + + mock_time_task_runner->FastForwardUntilNoTasksRemain(); + expected_value += 1024; + expected_value += 4096; + EXPECT_EQ(expected_value, counter); + } +} + +// A default TestMockTimeTaskRunner shouldn't result in a thread association. +TEST(TestMockTimeTaskRunnerTest, DefaultUnbound) { + auto unbound_mock_time_task_runner = MakeRefCounted(); + EXPECT_FALSE(ThreadTaskRunnerHandle::IsSet()); + EXPECT_FALSE(SequencedTaskRunnerHandle::IsSet()); + EXPECT_DCHECK_DEATH({ RunLoop().RunUntilIdle(); }); +} + +TEST(TestMockTimeTaskRunnerTest, RunLoopDriveableWhenBound) { + auto bound_mock_time_task_runner = MakeRefCounted( + TestMockTimeTaskRunner::Type::kBoundToThread); + + int counter = 0; + ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 1; }, Unretained(&counter))); + ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 32; }, Unretained(&counter))); + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 256; }, Unretained(&counter)), + TimeDelta::FromSeconds(3)); + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 64; }, Unretained(&counter)), + TimeDelta::FromSeconds(1)); + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 1024; }, Unretained(&counter)), + TimeDelta::FromMinutes(20)); + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 4096; }, Unretained(&counter)), + TimeDelta::FromDays(20)); + + int expected_value = 0; + EXPECT_EQ(expected_value, counter); + RunLoop().RunUntilIdle(); + expected_value += 1; + expected_value += 32; + EXPECT_EQ(expected_value, counter); + + RunLoop().RunUntilIdle(); + EXPECT_EQ(expected_value, counter); + + { + RunLoop run_loop; + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, run_loop.QuitClosure(), TimeDelta::FromSeconds(1)); + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 8192; }, + Unretained(&counter)), + TimeDelta::FromSeconds(1)); + + // The QuitClosure() should be ordered between the 64 and the 8192 + // increments and should preempt the latter. + run_loop.Run(); + expected_value += 64; + EXPECT_EQ(expected_value, counter); + + // Running until idle should process the 8192 increment whose delay has + // expired in the previous Run(). + RunLoop().RunUntilIdle(); + expected_value += 8192; + EXPECT_EQ(expected_value, counter); + } + + { + RunLoop run_loop; + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, run_loop.QuitWhenIdleClosure(), TimeDelta::FromSeconds(5)); + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, + base::Bind([](int* counter) { *counter += 16384; }, + Unretained(&counter)), + TimeDelta::FromSeconds(5)); + + // The QuitWhenIdleClosure() shouldn't preempt equally delayed tasks and as + // such the 16384 increment should be processed before quitting. + run_loop.Run(); + expected_value += 256; + expected_value += 16384; + EXPECT_EQ(expected_value, counter); + } + + // Process the remaining tasks (note: do not mimic this elsewhere, + // TestMockTimeTaskRunner::FastForwardUntilNoTasksRemain() is a better API to + // do this, this is just done here for the purpose of extensively testing the + // RunLoop approach). + RunLoop run_loop; + ThreadTaskRunnerHandle::Get()->PostDelayedTask( + FROM_HERE, run_loop.QuitWhenIdleClosure(), TimeDelta::FromDays(50)); + + run_loop.Run(); + expected_value += 1024; + expected_value += 4096; + EXPECT_EQ(expected_value, counter); +} + +} // namespace base diff --git a/base/threading/thread_task_runner_handle.cc b/base/threading/thread_task_runner_handle.cc index 11c9b6c029a67b..883f921d4e6184 100644 --- a/base/threading/thread_task_runner_handle.cc +++ b/base/threading/thread_task_runner_handle.cc @@ -10,6 +10,7 @@ #include "base/lazy_instance.h" #include "base/logging.h" #include "base/memory/ptr_util.h" +#include "base/run_loop.h" #include "base/threading/sequenced_task_runner_handle.h" #include "base/threading/thread_local.h" @@ -53,9 +54,8 @@ ScopedClosureRunner ThreadTaskRunnerHandle::OverrideForTesting( DCHECK(!SequencedTaskRunnerHandle::IsSet() || IsSet()); if (!IsSet()) { - std::unique_ptr top_level_ttrh = - std::make_unique( - std::move(overriding_task_runner)); + auto top_level_ttrh = std::make_unique( + std::move(overriding_task_runner)); return ScopedClosureRunner(base::Bind( [](std::unique_ptr ttrh_to_release) {}, base::Passed(&top_level_ttrh))); @@ -66,9 +66,14 @@ ScopedClosureRunner ThreadTaskRunnerHandle::OverrideForTesting( // previous one, as the |task_runner_to_restore|). ttrh->task_runner_.swap(overriding_task_runner); + auto no_running_during_override = + std::make_unique(); + return ScopedClosureRunner(base::Bind( [](scoped_refptr task_runner_to_restore, - SingleThreadTaskRunner* expected_task_runner_before_restore) { + SingleThreadTaskRunner* expected_task_runner_before_restore, + std::unique_ptr + no_running_during_override) { ThreadTaskRunnerHandle* ttrh = lazy_tls_ptr.Pointer()->Get(); DCHECK_EQ(expected_task_runner_before_restore, ttrh->task_runner_.get()) @@ -78,7 +83,8 @@ ScopedClosureRunner ThreadTaskRunnerHandle::OverrideForTesting( ttrh->task_runner_.swap(task_runner_to_restore); }, base::Passed(&overriding_task_runner), - base::Unretained(ttrh->task_runner_.get()))); + base::Unretained(ttrh->task_runner_.get()), + base::Passed(&no_running_during_override))); } ThreadTaskRunnerHandle::ThreadTaskRunnerHandle( diff --git a/components/policy/core/common/remote_commands/remote_commands_service_unittest.cc b/components/policy/core/common/remote_commands/remote_commands_service_unittest.cc index 1a8cc6661219b5..26fa373047c487 100644 --- a/components/policy/core/common/remote_commands/remote_commands_service_unittest.cc +++ b/components/policy/core/common/remote_commands/remote_commands_service_unittest.cc @@ -172,84 +172,14 @@ class TestingCloudPolicyClientForRemoteCommands : public CloudPolicyClient { DISALLOW_COPY_AND_ASSIGN(TestingCloudPolicyClientForRemoteCommands); }; -// A scoped TestMockTimeTaskRunner capable to run tasks until Quit() is called. -class ScopedMockTimeTaskRunner : public base::TestMockTimeTaskRunner { - public: - class ScopedRunner { - public: - explicit ScopedRunner( - const scoped_refptr& task_runner) - : task_runner_(task_runner) { - DCHECK(!task_runner_->attached_runner_); - task_runner_->attached_runner_ = this; - } - - virtual ~ScopedRunner() { - DCHECK_EQ(this, task_runner_->attached_runner_); - DCHECK(run_called_); - DCHECK(quit_called_); - - task_runner_->attached_runner_ = nullptr; - } - - void Run() { - DCHECK(!run_called_); - run_called_ = true; - - // It's okay to call Quit() before calling Run(). - if (quit_called_) - return; - task_runner_->FastForwardUntilNoTasksRemain(); - } - - void Quit() { - DCHECK(!quit_called_); - quit_called_ = true; - } - - base::Closure QuitClosure() { - // It's safe to use Unretained here since Quit() is required to be - // called before dtor is called. - return base::Bind(&ScopedRunner::Quit, base::Unretained(this)); - } - - private: - friend class ScopedMockTimeTaskRunner; - - bool run_called_ = false; - bool quit_called_ = false; - - scoped_refptr task_runner_; - - DISALLOW_COPY_AND_ASSIGN(ScopedRunner); - }; - - ScopedMockTimeTaskRunner() {} - - private: - ~ScopedMockTimeTaskRunner() override { DCHECK(!attached_runner_); } - - bool IsElapsingStopped() override { - return attached_runner_ && attached_runner_->quit_called_; - } - - // Points to the current attached ScopedRunner, and is null if no runner is - // attached. - ScopedRunner* attached_runner_ = nullptr; - - DISALLOW_COPY_AND_ASSIGN(ScopedMockTimeTaskRunner); -}; - // Base class for unit tests regarding remote commands service. class RemoteCommandsServiceTest : public testing::Test { protected: - RemoteCommandsServiceTest() - : task_runner_(new ScopedMockTimeTaskRunner()), - runner_handle_(task_runner_) {} + RemoteCommandsServiceTest() = default; void SetUp() override { server_.reset(new TestingRemoteCommandsServer()); - server_->SetClock(task_runner_->GetMockTickClock()); + server_->SetClock(mock_task_runner_->GetMockTickClock()); cloud_policy_client_.reset( new TestingCloudPolicyClientForRemoteCommands(server_.get())); } @@ -264,22 +194,20 @@ class RemoteCommandsServiceTest : public testing::Test { remote_commands_service_.reset(new RemoteCommandsService( std::move(factory), cloud_policy_client_.get())); remote_commands_service_->SetClockForTesting( - task_runner_->GetMockTickClock()); + mock_task_runner_->GetMockTickClock()); } - void FlushAllTasks() { - task_runner_->FastForwardUntilNoTasksRemain(); - } + void FlushAllTasks() { mock_task_runner_->FastForwardUntilNoTasksRemain(); } std::unique_ptr server_; std::unique_ptr cloud_policy_client_; std::unique_ptr remote_commands_service_; - scoped_refptr task_runner_; - private: - base::ThreadTaskRunnerHandle runner_handle_; + const scoped_refptr mock_task_runner_ = + base::MakeRefCounted( + base::TestMockTimeTaskRunner::Type::kBoundToThread); DISALLOW_COPY_AND_ASSIGN(RemoteCommandsServiceTest); }; @@ -306,7 +234,7 @@ TEST_F(RemoteCommandsServiceTest, ExistingCommand) { EXPECT_CALL(*factory, BuildTestCommand()).Times(1); { - ScopedMockTimeTaskRunner::ScopedRunner scoped_runner(task_runner_); + base::RunLoop run_loop; // Issue a command before service started. server_->IssueCommand(em::RemoteCommand_Type_COMMAND_ECHO_TEST, @@ -314,12 +242,11 @@ TEST_F(RemoteCommandsServiceTest, ExistingCommand) { base::Bind(&ExpectSucceededJob, kTestPayload), false); // Start the service, run until the command is fetched. - cloud_policy_client_->ExpectFetchCommands(0u, 1u, - scoped_runner.QuitClosure()); + cloud_policy_client_->ExpectFetchCommands(0u, 1u, run_loop.QuitClosure()); StartService(std::move(factory)); EXPECT_TRUE(remote_commands_service_->FetchRemoteCommands()); - scoped_runner.Run(); + run_loop.Run(); } // And run again so that the result can be reported. @@ -364,15 +291,14 @@ TEST_F(RemoteCommandsServiceTest, NewCommandFollwingFetch) { StartService(std::move(factory)); { - ScopedMockTimeTaskRunner::ScopedRunner scoped_runner(task_runner_); + base::RunLoop run_loop; // Add a command which will be issued after first fetch. server_->IssueCommand(em::RemoteCommand_Type_COMMAND_ECHO_TEST, kTestPayload, base::Bind(&ExpectSucceededJob, kTestPayload), true); - cloud_policy_client_->ExpectFetchCommands(0u, 0u, - scoped_runner.QuitClosure()); + cloud_policy_client_->ExpectFetchCommands(0u, 0u, run_loop.QuitClosure()); // Attempts to fetch commands. EXPECT_TRUE(remote_commands_service_->FetchRemoteCommands()); @@ -387,7 +313,7 @@ TEST_F(RemoteCommandsServiceTest, NewCommandFollwingFetch) { EXPECT_FALSE(remote_commands_service_->FetchRemoteCommands()); // Run until first fetch request is completed. - scoped_runner.Run(); + run_loop.Run(); } // The command should be issued now. Note that this command was actually diff --git a/remoting/host/heartbeat_sender_unittest.cc b/remoting/host/heartbeat_sender_unittest.cc index 37da6887a2c1d8..62dfcee50c0b4d 100644 --- a/remoting/host/heartbeat_sender_unittest.cc +++ b/remoting/host/heartbeat_sender_unittest.cc @@ -85,9 +85,9 @@ class HeartbeatSenderTest base::TimeDelta interval = base::TimeDelta(), int expected_sequence_id = 0); - base::MessageLoop message_loop_; scoped_refptr fake_time_task_runner_ = - new base::TestMockTimeTaskRunner(); + base::MakeRefCounted( + base::TestMockTimeTaskRunner::Type::kBoundToThread); FakeSignalStrategy signal_strategy_; FakeSignalStrategy bot_signal_strategy_; @@ -283,19 +283,14 @@ TEST_F(HeartbeatSenderTest, HostOsInfo) { } TEST_F(HeartbeatSenderTest, ResponseTimeout) { - base::TestMockTimeTaskRunner::ScopedContext scoped_context( - fake_time_task_runner_.get()); - signal_strategy_.Connect(); base::RunLoop().RunUntilIdle(); // Simulate heartbeat timeout. fake_time_task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(1)); - base::RunLoop().RunUntilIdle(); // SignalStrategy should be disconnected in response to the second timeout. fake_time_task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(1)); - base::RunLoop().RunUntilIdle(); EXPECT_EQ(SignalStrategy::DISCONNECTED, signal_strategy_.GetState()); }