Support base::test::ScopedTaskEnvironment and content::TestBrowserThreadBundle in the same scope.

content/public/test/test_browser_thread_bundle.cc

Index: content/public/test/test_browser_thread_bundle.cc
diff --git a/content/public/test/test_browser_thread_bundle.cc b/content/public/test/test_browser_thread_bundle.cc
index 23cc3b54934277f6bd2468197b9bd6e0daee3667..e7f3447e69ac501608f35953cd2775529e022252 100644
--- a/content/public/test/test_browser_thread_bundle.cc
+++ b/content/public/test/test_browser_thread_bundle.cc
@@ -8,8 +8,10 @@
 #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 "content/browser/browser_thread_impl.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread.h"
 
 namespace content {
@@ -59,30 +61,37 @@ TestBrowserThreadBundle::~TestBrowserThreadBundle() {
   scoped_async_task_scheduler_.reset();
 
   base::RunLoop().RunUntilIdle();
+  DCHECK(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().
-  CHECK(message_loop_->IsIdleForTesting());

[gab, 2017/05/03 15:33:10]

Why move (this was checking above comment) and change from CHECK (since this is
test-only code CHECK makes sure this asserts even in non-dcheck builds)?

[fdoray, 2017/05/03 17:03:50]

Done.

   message_loop_.reset();
 }
 
 void TestBrowserThreadBundle::Init() {
   // Check for conflicting options can't have two IO threads.
-  CHECK(!(options_ & IO_MAINLOOP) || !(options_ & REAL_IO_THREAD));

[gab, 2017/05/03 15:33:10]

ditto

[fdoray, 2017/05/03 17:03:50]

Done.

+  DCHECK(!(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);
+  DCHECK((options_ & ~IO_MAINLOOP) != DONT_CREATE_THREADS);
 
   // Create the UI thread. In production, this work is done in
   // BrowserMainLoop::MainMessageLoopStart().

[gab, 2017/05/03 15:33:10]

Update comment to mention why it could exist already in tests.

[fdoray, 2017/05/03 17:03:50]

Done.

-  if (options_ & IO_MAINLOOP) {
-    message_loop_.reset(new base::MessageLoopForIO());
-  } else {
-    message_loop_.reset(new base::MessageLoopForUI());
-  }
-
-  ui_thread_.reset(
-      new TestBrowserThread(BrowserThread::UI, message_loop_.get()));
+  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);
+  DCHECK(base::MessageLoop::current()->IsType(message_loop_type));
+
+  // Check that the UI thread hasn't already been initialized. This will fail if
+  // multiple TestBrowserThreadBundles are initialized in the same scope. This
+  // needs to be after MessageLoop initialization because
+  // BrowserThread::CurrentlyOn() checks that there is a MessageLoop.
+  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));

[gab, 2017/05/03 15:33:10]

You can use BrowserThread::IsThreadInitialized() to avoid requiring a
MessageLoop for the check (and then have the check at the top).

[fdoray, 2017/05/03 17:03:50]

Done.

+
+  ui_thread_ = base::MakeUnique<TestBrowserThread>(
+      BrowserThread::UI, base::MessageLoop::current());
 
   if (!(options_ & DONT_CREATE_THREADS))
     CreateThreads();
@@ -92,38 +101,40 @@ void TestBrowserThreadBundle::Init() {
 void TestBrowserThreadBundle::CreateThreads() {
   DCHECK(!threads_created_);
 
-  scoped_async_task_scheduler_ =
-      base::MakeUnique<base::test::ScopedAsyncTaskScheduler>();
+  if (!base::TaskScheduler::GetInstance()) {

[gab, 2017/05/03 15:33:10]

Above this:

// TaskScheduler can sometimes be externally provided by a
base::test::ScopedTaskEnvironment in a sub-fixture. In that case it's expected
to have provided the MessageLoop as well (in which case our |message_loop_|
remains null in Init()).
DCHECK(!base::TaskScheduler::GetInstance() || !message_loop_);

[fdoray, 2017/05/03 17:03:50]

Done.

+    scoped_async_task_scheduler_ =
+        base::MakeUnique<base::test::ScopedAsyncTaskScheduler>();
+  }
 
   if (options_ & REAL_DB_THREAD) {
-    db_thread_.reset(new TestBrowserThread(BrowserThread::DB));
+    db_thread_ = base::MakeUnique<TestBrowserThread>(BrowserThread::DB);
     db_thread_->Start();
   } else {
-    db_thread_.reset(
-        new TestBrowserThread(BrowserThread::DB, message_loop_.get()));
+    db_thread_ = base::MakeUnique<TestBrowserThread>(
+        BrowserThread::DB, base::MessageLoop::current());
   }
 
   if (options_ & REAL_FILE_THREAD) {
-    file_thread_.reset(new TestBrowserThread(BrowserThread::FILE));
+    file_thread_ = base::MakeUnique<TestBrowserThread>(BrowserThread::FILE);
     file_thread_->Start();
   } else {
-    file_thread_.reset(
-        new TestBrowserThread(BrowserThread::FILE, message_loop_.get()));
+    file_thread_ = base::MakeUnique<TestBrowserThread>(
+        BrowserThread::FILE, base::MessageLoop::current());
   }
 
-  file_user_blocking_thread_.reset(new TestBrowserThread(
-      BrowserThread::FILE_USER_BLOCKING, message_loop_.get()));
-  process_launcher_thread_.reset(new TestBrowserThread(
-      BrowserThread::PROCESS_LAUNCHER, message_loop_.get()));
-  cache_thread_.reset(
-      new TestBrowserThread(BrowserThread::CACHE, message_loop_.get()));
+  file_user_blocking_thread_ = base::MakeUnique<TestBrowserThread>(
+      BrowserThread::FILE_USER_BLOCKING, base::MessageLoop::current());
+  process_launcher_thread_ = base::MakeUnique<TestBrowserThread>(
+      BrowserThread::PROCESS_LAUNCHER, base::MessageLoop::current());
+  cache_thread_ = base::MakeUnique<TestBrowserThread>(
+      BrowserThread::CACHE, base::MessageLoop::current());
 
   if (options_ & REAL_IO_THREAD) {
-    io_thread_.reset(new TestBrowserThread(BrowserThread::IO));
+    io_thread_ = base::MakeUnique<TestBrowserThread>(BrowserThread::IO);
     io_thread_->StartIOThread();
   } else {
-    io_thread_.reset(
-        new TestBrowserThread(BrowserThread::IO, message_loop_.get()));
+    io_thread_ = base::MakeUnique<TestBrowserThread>(
+        BrowserThread::IO, base::MessageLoop::current());
   }
 
   threads_created_ = true;