Commit ad58f838 authored by fdoray's avatar fdoray Committed by Commit bot

Run ScopedTaskScheduler tasks synchronously.

With this CL, tasks posted to the base/task_scheduler/post_task.h API
no longer run asynchronously on a dedicated thread. Instead, they are
posted to a MessageLoop on the thread where the ScopedTaskScheduler
lives and run synchronously when RunLoop::Run/RunUntilIdle() is
invoked. This CL also allows usage of the
base/task_scheduler/post_task.h API within the scope of a
TestBrowserThreadBundle.

Benefits of running TaskScheduler tasks synchronously:
- Tests are more deterministic.
- It is easier to wait for a chain of tasks involving TaskScheduler.
  E.g.:
   base::PostTaskAndReplyWithTraits(
       FROM_HERE, TaskTraits(), Bind(&A), Bind(&B));
   base::RunLoop().RunUntilIdle();
       // Without this CL, this returns immediately because there
       // are no pending tasks on the main thread initially. With
       // this CL, this returns when A and B have run.

Note that this CL prevents a TestBrowserThreadBundle or a MessageLoop
from being initialized in the scope of a ScopedTaskScheduler. It
fixes tests that previously did this.

BUG=553459

Review-Url: https://codereview.chromium.org/2557083002
Cr-Commit-Position: refs/heads/master@{#439505}
parent bfdcca7e
......@@ -2025,6 +2025,7 @@ test("base_unittests") {
"template_util_unittest.cc",
"test/histogram_tester_unittest.cc",
"test/scoped_mock_time_message_loop_task_runner_unittest.cc",
"test/scoped_task_scheduler_unittest.cc",
"test/test_pending_task_unittest.cc",
"test/test_reg_util_win_unittest.cc",
"test/trace_event_analyzer_unittest.cc",
......
......@@ -357,12 +357,20 @@ std::string MessageLoop::GetThreadName() const {
void MessageLoop::SetTaskRunner(
scoped_refptr<SingleThreadTaskRunner> task_runner) {
DCHECK_EQ(this, current());
DCHECK(task_runner);
DCHECK(task_runner->BelongsToCurrentThread());
DCHECK(!unbound_task_runner_);
task_runner_ = std::move(task_runner);
SetThreadTaskRunnerHandle();
}
void MessageLoop::ClearTaskRunnerForTesting() {
DCHECK_EQ(this, current());
DCHECK(!unbound_task_runner_);
task_runner_ = nullptr;
thread_task_runner_handle_.reset();
}
void MessageLoop::SetThreadTaskRunnerHandle() {
DCHECK_EQ(this, current());
// Clear the previous thread task runner first, because only one can exist at
......
......@@ -234,6 +234,10 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
// thread to which the message loop is bound.
void SetTaskRunner(scoped_refptr<SingleThreadTaskRunner> task_runner);
// Clears task_runner() and the ThreadTaskRunnerHandle for the target thread.
// Must be called on the thread to which the message loop is bound.
void ClearTaskRunnerForTesting();
// Enables or disables the recursive task processing. This happens in the case
// of recursive message loops. Some unwanted message loops may occur when
// using common controls or printer functions. By default, recursive task
......
......@@ -82,10 +82,6 @@ class BASE_EXPORT TaskScheduler {
// other threads during the call. Returns immediately when shutdown completes.
virtual void FlushForTesting() = 0;
// Joins all threads of this scheduler. Tasks that are already running are
// allowed to complete their execution. This can only be called once.
virtual void JoinForTesting() = 0;
// CreateAndSetSimpleTaskScheduler(), CreateAndSetDefaultTaskScheduler(), and
// SetInstance() register a TaskScheduler to handle tasks posted through the
// post_task.h API for this process. The registered TaskScheduler will only be
......
......@@ -62,7 +62,10 @@ class BASE_EXPORT TaskSchedulerImpl : public TaskScheduler {
std::vector<const HistogramBase*> GetHistograms() const override;
void Shutdown() override;
void FlushForTesting() override;
void JoinForTesting() override;
// Joins all threads. Tasks that are already running are allowed to complete
// their execution. This can only be called once.
void JoinForTesting();
private:
explicit TaskSchedulerImpl(const WorkerPoolIndexForTraitsCallback&
......
......@@ -288,6 +288,14 @@ bool TaskTracker::IsShutdownComplete() const {
}
void TaskTracker::SetHasShutdownStartedForTesting() {
AutoSchedulerLock auto_lock(shutdown_lock_);
// Create a dummy |shutdown_event_| to satisfy TaskTracker's expectation of
// its existence during shutdown (e.g. in OnBlockingShutdownTasksComplete()).
shutdown_event_.reset(
new WaitableEvent(WaitableEvent::ResetPolicy::MANUAL,
WaitableEvent::InitialState::NOT_SIGNALED));
state_->StartShutdown();
}
......
phajdan.jr@chromium.org
per-file scoped_task_scheduler*=file://base/task_scheduler/OWNERS
# For Android-specific changes:
per-file *android*=file://base/test/android/OWNERS
per-file BUILD.gn=file://base/test/android/OWNERS
This diff is collapsed.
......@@ -6,31 +6,58 @@
#define BASE_TEST_SCOPED_TASK_SCHEDULER_H_
#include "base/macros.h"
#include "base/threading/thread_checker.h"
namespace base {
class MessageLoop;
class TaskScheduler;
namespace test {
// Initializes a TaskScheduler and allows usage of the
// base/task_scheduler/post_task.h API within its scope.
// Allows usage of the base/task_scheduler/post_task.h API within its scope.
//
// To run pending tasks synchronously, call RunLoop::Run/RunUntilIdle() on the
// thread where the ScopedTaskScheduler lives. The destructor runs remaining
// BLOCK_SHUTDOWN tasks synchronously.
//
// Example usage:
//
// In this snippet, RunUntilIdle() returns after "A" is run.
// base::test::ScopedTaskScheduler scoped_task_scheduler;
// base::PostTask(FROM_HERE, base::Bind(&A));
// base::RunLoop::RunUntilIdle(); // Returns after running A.
//
// In this snippet, run_loop.Run() returns after running "B" and
// "RunLoop::Quit".
// base::RunLoop run_loop;
// base::PostTask(FROM_HERE, base::Bind(&B));
// base::PostTask(FROM_HERE, base::Bind(&RunLoop::Quit, &run_loop));
// base::PostTask(FROM_HERE, base::Bind(&C));
// base::PostTaskWithTraits(
// base::TaskTraits().WithShutdownBehavior(
// base::TaskShutdownBehavior::BLOCK_SHUTDOWN),
// base::Bind(&D));
// run_loop.Run(); // Returns after running B and RunLoop::Quit.
//
// At this point, |scoped_task_scheduler| will be destroyed. The destructor
// runs "D" because it's BLOCK_SHUTDOWN. "C" is skipped.
class ScopedTaskScheduler {
public:
// Initializes a TaskScheduler with default arguments.
// Registers a TaskScheduler that instantiates a MessageLoop on the current
// thread and runs its tasks on it.
ScopedTaskScheduler();
// Waits until all TaskScheduler tasks blocking shutdown complete their
// execution (see TaskShutdownBehavior). Then, joins all TaskScheduler threads
// and deletes the TaskScheduler.
//
// Note that joining TaskScheduler threads may involve waiting for
// CONTINUE_ON_SHUTDOWN tasks to complete their execution. Normally, in
// production, the process exits without joining TaskScheduler threads.
// Registers a TaskScheduler that runs its tasks on |external_message_loop|.
// |external_message_loop| must be bound to the current thread.
explicit ScopedTaskScheduler(MessageLoop* external_message_loop);
// Runs all pending BLOCK_SHUTDOWN tasks and unregisters the TaskScheduler.
~ScopedTaskScheduler();
private:
const TaskScheduler* task_scheduler_ = nullptr;
ThreadChecker thread_checker_;
DISALLOW_COPY_AND_ASSIGN(ScopedTaskScheduler);
};
......
// Copyright 2016 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/scoped_task_scheduler.h"
#include "base/bind.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/run_loop.h"
#include "base/sequence_checker.h"
#include "base/task_scheduler/post_task.h"
#include "base/task_scheduler/test_utils.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/threading/thread_checker.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace base {
namespace test {
TEST(ScopedTaskSchedulerTest, PostTask) {
ScopedTaskScheduler scoped_task_scheduler;
bool first_task_ran = false;
bool second_task_ran = false;
SequenceCheckerImpl sequence_checker;
ThreadCheckerImpl thread_checker;
// Detach |sequence_checker| and |thread_checker|. Otherwise, they are bound
// to the current thread without a SequenceToken or TaskToken (i.e.
// CalledOnValidSequence/Thread() will always return true on the current
// thread, even when the SequenceToken or TaskToken changes).
sequence_checker.DetachFromSequence();
thread_checker.DetachFromThread();
PostTask(FROM_HERE,
Bind(
[](SequenceCheckerImpl* sequence_checker,
ThreadCheckerImpl* thread_checker, bool* first_task_ran) {
EXPECT_FALSE(SequencedTaskRunnerHandle::IsSet());
EXPECT_FALSE(ThreadTaskRunnerHandle::IsSet());
EXPECT_TRUE(sequence_checker->CalledOnValidSequence());
EXPECT_TRUE(thread_checker->CalledOnValidThread());
*first_task_ran = true;
},
Unretained(&sequence_checker), Unretained(&thread_checker),
Unretained(&first_task_ran)));
PostTask(FROM_HERE,
Bind(
[](SequenceCheckerImpl* sequence_checker,
ThreadCheckerImpl* thread_checker, bool* second_task_ran) {
EXPECT_FALSE(SequencedTaskRunnerHandle::IsSet());
EXPECT_FALSE(ThreadTaskRunnerHandle::IsSet());
EXPECT_FALSE(sequence_checker->CalledOnValidSequence());
EXPECT_FALSE(thread_checker->CalledOnValidThread());
*second_task_ran = true;
},
Unretained(&sequence_checker), Unretained(&thread_checker),
Unretained(&second_task_ran)));
RunLoop().RunUntilIdle();
EXPECT_TRUE(first_task_ran);
EXPECT_TRUE(second_task_ran);
}
TEST(ScopedTaskSchedulerTest, CreateTaskRunnerAndPostTask) {
ScopedTaskScheduler scoped_task_scheduler;
auto task_runner = CreateTaskRunnerWithTraits(TaskTraits());
bool first_task_ran = false;
bool second_task_ran = false;
SequenceCheckerImpl sequence_checker;
ThreadCheckerImpl thread_checker;
// Detach |sequence_checker| and |thread_checker|. Otherwise, they are bound
// to the current thread without a SequenceToken or TaskToken (i.e.
// CalledOnValidSequence/Thread() will always return true on the current
// thread, even when the SequenceToken or TaskToken changes).
sequence_checker.DetachFromSequence();
thread_checker.DetachFromThread();
task_runner->PostTask(
FROM_HERE,
Bind(
[](SequenceCheckerImpl* sequence_checker,
ThreadCheckerImpl* thread_checker, bool* first_task_ran) {
EXPECT_FALSE(SequencedTaskRunnerHandle::IsSet());
EXPECT_FALSE(ThreadTaskRunnerHandle::IsSet());
EXPECT_TRUE(sequence_checker->CalledOnValidSequence());
EXPECT_TRUE(thread_checker->CalledOnValidThread());
*first_task_ran = true;
},
Unretained(&sequence_checker), Unretained(&thread_checker),
Unretained(&first_task_ran)));
task_runner->PostTask(
FROM_HERE,
Bind(
[](SequenceCheckerImpl* sequence_checker,
ThreadCheckerImpl* thread_checker, bool* second_task_ran) {
EXPECT_FALSE(SequencedTaskRunnerHandle::IsSet());
EXPECT_FALSE(ThreadTaskRunnerHandle::IsSet());
EXPECT_FALSE(sequence_checker->CalledOnValidSequence());
EXPECT_FALSE(thread_checker->CalledOnValidThread());
*second_task_ran = true;
},
Unretained(&sequence_checker), Unretained(&thread_checker),
Unretained(&second_task_ran)));
RunLoop().RunUntilIdle();
EXPECT_TRUE(first_task_ran);
EXPECT_TRUE(second_task_ran);
}
TEST(ScopedTaskSchedulerTest, CreateSequencedTaskRunnerAndPostTask) {
ScopedTaskScheduler scoped_task_scheduler;
auto task_runner = CreateSequencedTaskRunnerWithTraits(TaskTraits());
bool first_task_ran = false;
bool second_task_ran = false;
SequenceCheckerImpl sequence_checker;
ThreadCheckerImpl thread_checker;
// Detach |sequence_checker| and |thread_checker|. Otherwise, they are bound
// to the current thread without a SequenceToken or TaskToken (i.e.
// CalledOnValidSequence/Thread() will always return true on the current
// thread, even when the SequenceToken or TaskToken changes).
sequence_checker.DetachFromSequence();
thread_checker.DetachFromThread();
task_runner->PostTask(
FROM_HERE,
Bind(
[](SequenceCheckerImpl* sequence_checker,
ThreadCheckerImpl* thread_checker, bool* first_task_ran) {
EXPECT_TRUE(SequencedTaskRunnerHandle::IsSet());
EXPECT_FALSE(ThreadTaskRunnerHandle::IsSet());
EXPECT_TRUE(sequence_checker->CalledOnValidSequence());
EXPECT_TRUE(thread_checker->CalledOnValidThread());
*first_task_ran = true;
},
Unretained(&sequence_checker), Unretained(&thread_checker),
Unretained(&first_task_ran)));
task_runner->PostTask(
FROM_HERE,
Bind(
[](SequenceCheckerImpl* sequence_checker,
ThreadCheckerImpl* thread_checker, bool* second_task_ran) {
EXPECT_TRUE(SequencedTaskRunnerHandle::IsSet());
EXPECT_FALSE(ThreadTaskRunnerHandle::IsSet());
EXPECT_TRUE(sequence_checker->CalledOnValidSequence());
EXPECT_FALSE(thread_checker->CalledOnValidThread());
*second_task_ran = true;
},
Unretained(&sequence_checker), Unretained(&thread_checker),
Unretained(&second_task_ran)));
RunLoop().RunUntilIdle();
EXPECT_TRUE(first_task_ran);
EXPECT_TRUE(second_task_ran);
}
TEST(ScopedTaskSchedulerTest, CreateSingleThreadTaskRunnerAndPostTask) {
ScopedTaskScheduler scoped_task_scheduler;
auto task_runner = CreateSingleThreadTaskRunnerWithTraits(TaskTraits());
bool first_task_ran = false;
bool second_task_ran = false;
SequenceCheckerImpl sequence_checker;
ThreadCheckerImpl thread_checker;
// Detach |sequence_checker| and |thread_checker|. Otherwise, they are bound
// to the current thread without a SequenceToken or TaskToken (i.e.
// CalledOnValidSequence/Thread() will always return true on the current
// thread, even when the SequenceToken or TaskToken changes).
sequence_checker.DetachFromSequence();
thread_checker.DetachFromThread();
task_runner->PostTask(
FROM_HERE,
Bind(
[](SequenceCheckerImpl* sequence_checker,
ThreadCheckerImpl* thread_checker, bool* first_task_ran) {
EXPECT_TRUE(SequencedTaskRunnerHandle::IsSet());
EXPECT_TRUE(ThreadTaskRunnerHandle::IsSet());
EXPECT_TRUE(sequence_checker->CalledOnValidSequence());
EXPECT_TRUE(thread_checker->CalledOnValidThread());
*first_task_ran = true;
},
Unretained(&sequence_checker), Unretained(&thread_checker),
Unretained(&first_task_ran)));
task_runner->PostTask(
FROM_HERE,
Bind(
[](SequenceCheckerImpl* sequence_checker,
ThreadCheckerImpl* thread_checker, bool* second_task_ran) {
EXPECT_TRUE(SequencedTaskRunnerHandle::IsSet());
EXPECT_TRUE(ThreadTaskRunnerHandle::IsSet());
EXPECT_TRUE(sequence_checker->CalledOnValidSequence());
EXPECT_TRUE(thread_checker->CalledOnValidThread());
*second_task_ran = true;
},
Unretained(&sequence_checker), Unretained(&thread_checker),
Unretained(&second_task_ran)));
RunLoop().RunUntilIdle();
EXPECT_TRUE(first_task_ran);
EXPECT_TRUE(second_task_ran);
}
TEST(ScopedTaskSchedulerTest, ShutdownBehavior) {
bool block_shutdown_task_ran = false;
{
ScopedTaskScheduler scoped_task_scheduler;
PostTaskWithTraits(
FROM_HERE, TaskTraits().WithShutdownBehavior(
TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN),
Bind([]() {
ADD_FAILURE() << "CONTINUE_ON_SHUTDOWN task should not run";
}));
PostTaskWithTraits(FROM_HERE, TaskTraits().WithShutdownBehavior(
TaskShutdownBehavior::SKIP_ON_SHUTDOWN),
Bind([]() {
ADD_FAILURE()
<< "SKIP_ON_SHUTDOWN task should not run";
}));
PostTaskWithTraits(FROM_HERE, TaskTraits().WithShutdownBehavior(
TaskShutdownBehavior::BLOCK_SHUTDOWN),
Bind(
[](bool* block_shutdown_task_ran) {
*block_shutdown_task_ran = true;
},
Unretained(&block_shutdown_task_ran)));
}
EXPECT_TRUE(block_shutdown_task_ran);
}
} // namespace test
} // namespace base
......@@ -296,7 +296,8 @@ class SequencedWorkerPoolTest
// Destroys and unregisters the registered TaskScheduler, if any.
void DeleteTaskScheduler() {
if (TaskScheduler::GetInstance()) {
TaskScheduler::GetInstance()->JoinForTesting();
static_cast<internal::TaskSchedulerImpl*>(TaskScheduler::GetInstance())
->JoinForTesting();
TaskScheduler::SetInstance(nullptr);
}
}
......
......@@ -9,7 +9,6 @@
#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_task_scheduler.h"
#include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_tpm_key_manager.h"
#include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_tpm_key_manager_factory.h"
#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
......@@ -337,7 +336,6 @@ class EasyUnlockTpmKeyManagerTest : public testing::Test {
const AccountId test_account_id_ = AccountId::FromUserEmail(kTestUserId);
private:
base::test::ScopedTaskScheduler scoped_task_scheduler_;
content::TestBrowserThreadBundle thread_bundle_;
// The NSS system slot used by EasyUnlockTPMKeyManagers in tests.
......
......@@ -11,7 +11,6 @@
#include "base/location.h"
#include "base/memory/ptr_util.h"
#include "base/memory/ref_counted.h"
#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/test/scoped_task_scheduler.h"
......@@ -90,7 +89,6 @@ class ClientCertStoreChromeOSTest : public ::testing::Test {
private:
base::test::ScopedTaskScheduler scoped_task_scheduler_;
base::MessageLoopForIO message_loop_;
};
// Ensure that cert requests, that are started before the filter is initialized,
......
......@@ -7,6 +7,7 @@
#include "base/logging.h"
#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
#include "base/test/scoped_task_scheduler.h"
#include "content/browser/browser_thread_impl.h"
#include "content/public/test/test_browser_thread.h"
......@@ -52,6 +53,8 @@ TestBrowserThreadBundle::~TestBrowserThreadBundle() {
ui_thread_->Stop();
base::RunLoop().RunUntilIdle();
task_scheduler_.reset();
// |message_loop_| needs to explicitly go away before fake threads in order
// for DestructionObservers hooked to |message_loop_| to be able to invoke
// BrowserThread::CurrentlyOn() -- ref. ~TestBrowserThread().
......@@ -71,6 +74,9 @@ void TestBrowserThreadBundle::Init() {
message_loop_.reset(new base::MessageLoopForUI());
}
task_scheduler_.reset(
new base::test::ScopedTaskScheduler(message_loop_.get()));
ui_thread_.reset(
new TestBrowserThread(BrowserThread::UI, message_loop_.get()));
......
......@@ -3,34 +3,27 @@
// found in the LICENSE file.
// TestBrowserThreadBundle is a convenience class for creating a set of
// TestBrowserThreads in unit tests. For most tests, it is sufficient to
// just instantiate the TestBrowserThreadBundle as a member variable.
// It is a good idea to put the TestBrowserThreadBundle as the first member
// variable in test classes, so it is destroyed last, and the test threads
// always exist from the perspective of other classes.
// TestBrowserThreads, a blocking pool, and a task scheduler in unit tests. For
// most tests, it is sufficient to just instantiate the TestBrowserThreadBundle
// as a member variable. It is a good idea to put the TestBrowserThreadBundle as
// the first member variable in test classes, so it is destroyed last, and the
// test threads always exist from the perspective of other classes.
//
// By default, all of the created TestBrowserThreads will be backed by a single
// shared MessageLoop. If a test truly needs separate threads, it can do
// so by passing the appropriate combination of option values during
// the TestBrowserThreadBundle construction.
// By default, all of the created TestBrowserThreads and the task scheduler will
// be backed by a single shared MessageLoop. If a test truly needs separate
// threads, it can do so by passing the appropriate combination of option values
// during the TestBrowserThreadBundle construction.
//
// The TestBrowserThreadBundle will attempt to drain the MessageLoop on
// destruction. Sometimes a test needs to drain currently enqueued tasks
// mid-test. Browser tests should call content::RunAllPendingInMessageLoop().
// Unit tests should use base::RunLoop (e.g., base::RunLoop().RunUntilIdle()).
// TODO(phajdan.jr): Revise this comment after switch to Aura.
// To synchronously run tasks posted to task scheduler or to TestBrowserThreads
// that use the shared MessageLoop, call RunLoop::Run/RunUntilIdle() on the
// thread where the TestBrowserThreadBundle lives. The destructor of
// TestBrowserThreadBundle runs remaining TestBrowserThreads tasks, remaining
// blocking pool tasks, and remaining BLOCK_SHUTDOWN task scheduler tasks.
//
// The TestBrowserThreadBundle will also flush the blocking pool on destruction.
// We do this to avoid memory leaks, particularly in the case of threads posting
// tasks to the blocking pool via PostTaskAndReply. By ensuring that the tasks
// are run while the originating TestBroswserThreads still exist, we prevent
// leakage of PostTaskAndReplyRelay objects. We also flush the blocking pool
// again at the point where it would normally be shut down, to better simulate
// the normal thread shutdown process.
//
// Some tests using the IO thread expect a MessageLoopForIO. Passing
// IO_MAINLOOP will use a MessageLoopForIO for the main MessageLoop.
// Most of the time, this avoids needing to use a REAL_IO_THREAD.
// If a test needs a MessageLoopForIO on the main thread, it should use the
// IO_MAINLOOP option. This also allows task scheduler tasks to use
// FileDescriptorWatcher. Most of the time, IO_MAINLOOP avoids needing to use a
// REAL_IO_THREAD.
//
// For some tests it is important to emulate real browser startup. During real
// browser startup some initialization is done (e.g. creation of thread objects)
......@@ -51,6 +44,9 @@
namespace base {
class MessageLoop;
namespace test {
class ScopedTaskScheduler;
} // namespace test
} // namespace base
namespace content {
......@@ -84,6 +80,7 @@ class TestBrowserThreadBundle {
void Init();
std::unique_ptr<base::MessageLoop> message_loop_;
std::unique_ptr<base::test::ScopedTaskScheduler> task_scheduler_;
std::unique_ptr<TestBrowserThread> ui_thread_;
std::unique_ptr<TestBrowserThread> db_thread_;
std::unique_ptr<TestBrowserThread> file_thread_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment