Skip to content

GitLab

T
tangled
Commit 69f889fb authored by Francois Doray's avatar Francois Doray Committed by Commit Bot
Browse files
Options

Remove usage of ScopedAsyncTaskScheduler from TestBrowserThreadBundle. 

ScopedAsyncTaskScheduler is deprecated.

Bug: 708584
Change-Id: I795e7d07947369c202ea767b05543bb44474ff47
Reviewed-on: https://chromium-review.googlesource.com/591788Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491383}
parent 13381c21
Hide whitespace changes
Inline Side-by-side
Showing with 52 additions and 66 deletions
chrome/browser/io_thread_unittest.cc
View file @ 69f889fb
......@@ -117,8 +117,9 @@ class IOThreadTestWithIOThreadObject : public testing::Test {
protected:
IOThreadTestWithIOThreadObject()
: thread_bundle_(content::TestBrowserThreadBundle::REAL_IO_THREAD |
content::TestBrowserThreadBundle::DONT_CREATE_THREADS) {
: thread_bundle_(
content::TestBrowserThreadBundle::REAL_IO_THREAD |
content::TestBrowserThreadBundle::DONT_CREATE_BROWSER_THREADS) {
#if BUILDFLAG(ENABLE_EXTENSIONS)
event_router_forwarder_ = new extensions::EventRouterForwarder;
#endif
......@@ -153,7 +154,7 @@ class IOThreadTestWithIOThreadObject : public testing::Test {
// Now that IOThread object is registered starting the threads will
// call the IOThread::Init(). This sets up the environment needed for
// these tests.
thread_bundle_.CreateThreads();
thread_bundle_.CreateBrowserThreads();
}
~IOThreadTestWithIOThreadObject() override {
......
content/public/test/test_browser_thread_bundle.cc
View file @ 69f889fb
......@@ -8,8 +8,7 @@
#include "base/memory/ptr_util.h"
#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
#include "base/task_scheduler/task_scheduler.h"
#include "base/test/scoped_async_task_scheduler.h"
#include "base/test/scoped_task_environment.h"
#include "content/browser/browser_thread_impl.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/test/test_browser_thread.h"
......@@ -66,30 +65,25 @@ TestBrowserThreadBundle::~TestBrowserThreadBundle() {
// Skip the following step when TaskScheduler isn't managed by this
// TestBrowserThreadBundle, otherwise it can hang (e.g.
// RunAllBlockingPoolTasksUntilIdle() hangs when the TaskScheduler is managed
// by a ScopedTaskEnvironment with ExecutionMode::QUEUED). This is fine as (1)
// it's rare and (2) it mimics production where BrowserThreads are shutdown
// before TaskScheduler.
if (scoped_async_task_scheduler_) {
// This is required to ensure we run all remaining tasks in an atomic step
// (instead of ~ScopedAsyncTaskScheduler() followed by another
// RunLoop().RunUntilIdle()). Otherwise If a pending task in
// |scoped_async_task_scheduler_| posts to |message_loop_|, that task can
// then post back to |scoped_async_task_scheduler_| after the former was
// destroyed. This is a bit different than production where the main thread
// is not flushed after it's done running but this approach is preferred in
// unit tests as running more tasks can merely uncover more issues (e.g. if
// a bad tasks is posted but never blocked upon it could make a test flaky
// whereas by flushing we guarantee it will blow up).
// by an external ScopedTaskEnvironment with ExecutionMode::QUEUED). This is
// fine as (1) it's rare and (2) it mimics production where BrowserThreads are
// shutdown before TaskScheduler.
if (scoped_task_environment_) {
// This is required to ensure we run all remaining MessageLoop and
// TaskScheduler tasks in an atomic step. This is a bit different than
// production where the main thread is not flushed after it's done running
// but this approach is preferred in unit tests as running more tasks can
// merely uncover more issues (e.g. if a bad tasks is posted but never
// blocked upon it could make a test flaky whereas by flushing we guarantee
// it will blow up).
RunAllBlockingPoolTasksUntilIdle();
scoped_async_task_scheduler_.reset();
CHECK(base::MessageLoop::current()->IsIdleForTesting());
}
// |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().
message_loop_.reset();
// |scoped_task_environment_| needs to explicitly go away before fake threads
// in order for DestructionObservers hooked to the main MessageLoop to be able
// to invoke BrowserThread::CurrentlyOn() -- ref. ~TestBrowserThread().
scoped_task_environment_.reset();
#if defined(OS_WIN)
com_initializer_.reset();
......@@ -103,8 +97,8 @@ void TestBrowserThreadBundle::Init() {
// Check for conflicting options can't have two IO threads.
CHECK(!(options_ & IO_MAINLOOP) || !(options_ & REAL_IO_THREAD));
// There must be a thread to start to use DONT_CREATE_THREADS
CHECK((options_ & ~IO_MAINLOOP) != DONT_CREATE_THREADS);
// There must be a thread to start to use DONT_CREATE_BROWSER_THREADS
CHECK((options_ & ~IO_MAINLOOP) != DONT_CREATE_BROWSER_THREADS);
#if defined(OS_WIN)
// Similar to Chrome's UI thread, we need to initialize COM separately for
......@@ -114,39 +108,32 @@ void TestBrowserThreadBundle::Init() {
CHECK(com_initializer_->succeeded());
#endif
// Create the main MessageLoop, if it doesn't already exist, and set the
// current thread as the UI thread. In production, this work is done in
// BrowserMainLoop::MainMessageLoopStart(). The main MessageLoop may already
// exist if this TestBrowserThreadBundle is instantiated in a test whose
// parent fixture provides a base::test::ScopedTaskEnvironment.
const base::MessageLoop::Type message_loop_type =
options_ & IO_MAINLOOP ? base::MessageLoop::TYPE_IO
: base::MessageLoop::TYPE_UI;
if (!base::MessageLoop::current())
message_loop_ = base::MakeUnique<base::MessageLoop>(message_loop_type);
CHECK(base::MessageLoop::current()->IsType(message_loop_type));
// Create the ScopedTaskEnvironment if it doesn't already exist. A
// ScopedTaskEnvironment may already exist if this TestBrowserThreadBundle is
// instantiated in a test whose parent fixture provides a
// ScopedTaskEnvironment.
if (!base::MessageLoop::current()) {
scoped_task_environment_ =
base::MakeUnique<base::test::ScopedTaskEnvironment>(
options_ & IO_MAINLOOP
? base::test::ScopedTaskEnvironment::MainThreadType::IO
: base::test::ScopedTaskEnvironment::MainThreadType::UI);
}
CHECK(base::MessageLoop::current()->IsType(options_ & IO_MAINLOOP
? base::MessageLoop::TYPE_IO
: base::MessageLoop::TYPE_UI));
// Set the current thread as the UI thread.
ui_thread_ = base::MakeUnique<TestBrowserThread>(
BrowserThread::UI, base::MessageLoop::current());
if (!(options_ & DONT_CREATE_THREADS))
CreateThreads();
if (!(options_ & DONT_CREATE_BROWSER_THREADS))
CreateBrowserThreads();
}
// This method mimics the work done in BrowserMainLoop::CreateThreads().
void TestBrowserThreadBundle::CreateThreads() {
void TestBrowserThreadBundle::CreateBrowserThreads() {
CHECK(!threads_created_);
// TaskScheduler can sometimes be externally provided by a
// base::test::ScopedTaskEnvironment in a parent fixture. In that case it's
// expected to have provided the MessageLoop as well (in which case
// |message_loop_| remains null in Init()).
CHECK(!base::TaskScheduler::GetInstance() || !message_loop_);
if (!base::TaskScheduler::GetInstance()) {
scoped_async_task_scheduler_ =
base::MakeUnique<base::test::ScopedAsyncTaskScheduler>();
}
if (options_ & REAL_DB_THREAD) {
db_thread_ = base::MakeUnique<TestBrowserThread>(BrowserThread::DB);
db_thread_->Start();
......
content/public/test/test_browser_thread_bundle.h
View file @ 69f889fb
......@@ -47,12 +47,13 @@
// REAL_IO_THREAD.
//
// For some tests it is important to emulate real browser startup. During real
// browser startup, the main MessageLoop is created before other threads.
// Passing DONT_CREATE_THREADS to constructor will delay creating other threads
// until the test explicitly calls CreateThreads().
// browser startup, the main MessageLoop and the TaskScheduler are created
// before browser threads. Passing DONT_CREATE_BROWSER_THREADS to constructor
// will delay creating browser threads until the test explicitly calls
// CreateBrowserThreads().
//
// DONT_CREATE_THREADS should only be used when the options specify at least
// one real thread other than the main thread.
// DONT_CREATE_BROWSER_THREADS should only be used when the options specify at
// least one real thread other than the main thread.
//
// TestBrowserThreadBundle may be instantiated in a scope where there is already
// a base::test::ScopedTaskEnvironment. In that case, it will use the
......@@ -90,9 +91,8 @@
#include "build/build_config.h"
namespace base {
class MessageLoop;
namespace test {
class ScopedAsyncTaskScheduler;
class ScopedTaskEnvironment;
} // namespace test
#if defined(OS_WIN)
namespace win {
......@@ -116,24 +116,22 @@ class TestBrowserThreadBundle {
REAL_DB_THREAD = 1 << 1,
REAL_FILE_THREAD = 1 << 2,
REAL_IO_THREAD = 1 << 3,
DONT_CREATE_THREADS = 1 << 4,
DONT_CREATE_BROWSER_THREADS = 1 << 4,
};
TestBrowserThreadBundle();
explicit TestBrowserThreadBundle(int options);
// Creates threads; should only be called from other classes if the
// DONT_CREATE_THREADS option was used when the bundle was created.
void CreateThreads();
// Creates browser threads; should only be called from other classes if the
// DONT_CREATE_BROWSER_THREADS option was used when the bundle was created.
void CreateBrowserThreads();
~TestBrowserThreadBundle();
private:
void Init();
std::unique_ptr<base::MessageLoop> message_loop_;
std::unique_ptr<base::test::ScopedAsyncTaskScheduler>
scoped_async_task_scheduler_;
std::unique_ptr<base::test::ScopedTaskEnvironment> scoped_task_environment_;
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