Add BrowserThread::RunAllPendingTasksOnThreadForTesting

Soon there will be multiple queues with different priorities, so the
trick of posting a task that quits a run loop currentlyl in use in
test_utils.cc will no longer work. In preparation for that this patch
add a better implementation of the same functionality.

Bug: 863341
Change-Id: I643e22a491145a3bae00d06ae9164a450f501298
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1541251Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Commit-Queue: Carlos Caballero <carlscab@google.com>
Cr-Commit-Position: refs/heads/master@{#646252}

content/browser/browser_thread_impl.cc

@@ -21,6 +21,7 @@
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
+#include "content/browser/scheduler/browser_task_executor.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/content_browser_client.h"
 
@@ -254,4 +255,9 @@ BrowserThread::GetTaskRunnerForThread(ID identifier) {
   return globals.task_runners[identifier];
 }
 
+// static
+void BrowserThread::RunAllPendingTasksOnThreadForTesting(ID identifier) {
+  BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(identifier);
+}
+
 }  // namespace content

content/browser/scheduler/browser_task_executor.cc

@@ -9,10 +9,12 @@
 #include "base/bind.h"
 #include "base/deferred_sequenced_task_runner.h"
 #include "base/no_destructor.h"
+#include "base/task/post_task.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/scheduler/browser_ui_thread_scheduler.h"
 #include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 
 #if defined(OS_ANDROID)
 #include "base/android/task_scheduler/post_task_android.h"
@@ -226,6 +228,35 @@ void BrowserTaskExecutor::ResetForTesting() {
   }
 }
 
+// static
+void BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(
+    BrowserThread::ID identifier) {
+  DCHECK(g_browser_task_executor);
+  DCHECK(g_browser_task_executor->browser_ui_thread_scheduler_);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  switch (identifier) {
+    case BrowserThread::UI:
+      g_browser_task_executor->browser_ui_thread_scheduler_
+          ->RunAllPendingTasksForTesting();
+      break;
+
+    case BrowserThread::IO: {
+      // TODO(https://crbug/863341): Do something more clever once we have a
+      // scheduler
+      base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
+      base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO},
+                               run_loop.QuitClosure());
+      run_loop.Run();
+      break;
+    }
+
+    case BrowserThread::ID_COUNT:
+      NOTREACHED();
+      break;
+  }
+}
+
 bool BrowserTaskExecutor::PostDelayedTaskWithTraits(
     const base::Location& from_here,
     const base::TaskTraits& traits,

content/browser/scheduler/browser_task_executor.h

@@ -13,6 +13,7 @@
 #include "build/build_config.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 
 namespace content {
 
@@ -44,6 +45,19 @@ class CONTENT_EXPORT BrowserTaskExecutor : public base::TaskExecutor {
   // Unregister and delete the TaskExecutor after a test.
   static void ResetForTesting();
 
+  // Runs all pending tasks for the given thread. Tasks posted after this method
+  // is called (in particular any task posted from within any of the pending
+  // tasks) will be queued but not run. Conceptually this call will disable all
+  // queues, run any pending tasks, and re-enable all the queues.
+  //
+  // If any of the pending tasks posted a task, these could be run by calling
+  // this method again or running a regular RunLoop. But if that were the case
+  // you should probably rewrite you tests to wait for a specific event instead.
+  //
+  // NOTE: Can only be called from the UI thread.
+  static void RunAllPendingTasksOnThreadForTesting(
+      BrowserThread::ID identifier);
+
   // base::TaskExecutor implementation.
   bool PostDelayedTaskWithTraits(const base::Location& from_here,
                                  const base::TaskTraits& traits,

content/browser/scheduler/browser_task_executor_unittest.cc

@@ -11,15 +11,21 @@
 #include "base/bind_helpers.h"
 #include "base/task/post_task.h"
 #include "base/task/task_scheduler/task_scheduler.h"
+#include "base/test/mock_callback.h"
 #include "base/test/scoped_task_environment.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/scheduler/browser_ui_thread_scheduler.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/test/test_content_browser_client.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
+using ::testing::Invoke;
+using ::testing::Mock;
+
 class BrowserTaskExecutorTest : public testing::Test {
  public:
   BrowserTaskExecutorTest() {
@@ -81,6 +87,67 @@ void SetBoolFlag(bool* flag) {
 }
 }  // namespace
 
+using MockTask =
+    testing::StrictMock<base::MockCallback<base::RepeatingCallback<void()>>>;
+
+TEST_F(BrowserTaskExecutorTest, RunAllPendingTasksForTestingOnUI) {
+  MockTask task_1;
+  MockTask task_2;
+  EXPECT_CALL(task_1, Run).WillOnce(testing::Invoke([&]() {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task_2.Get());
+  }));
+
+  base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, task_1.Get());
+
+  BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::UI);
+
+  // Cleanup pending tasks, as TestBrowserThreadBundle will run them.
+  Mock::VerifyAndClearExpectations(&task_1);
+  EXPECT_CALL(task_2, Run);
+  BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
+}
+
+TEST_F(BrowserTaskExecutorTest, RunAllPendingTasksForTestingOnIO) {
+  MockTask task_1;
+  MockTask task_2;
+  EXPECT_CALL(task_1, Run).WillOnce(testing::Invoke([&]() {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task_2.Get());
+  }));
+
+  base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO}, task_1.Get());
+
+  BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
+
+  // Cleanup pending tasks, as TestBrowserThreadBundle will run them.
+  Mock::VerifyAndClearExpectations(&task_1);
+  EXPECT_CALL(task_2, Run);
+  BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
+}
+
+TEST_F(BrowserTaskExecutorTest, RunAllPendingTasksForTestingOnIOIsReentrant) {
+  MockTask task_1;
+  MockTask task_2;
+  MockTask task_3;
+
+  EXPECT_CALL(task_1, Run).WillOnce(Invoke([&]() {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task_2.Get());
+    BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(
+        BrowserThread::IO);
+  }));
+  EXPECT_CALL(task_2, Run).WillOnce(Invoke([&]() {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task_3.Get());
+  }));
+
+  base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO}, task_1.Get());
+  BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
+
+  // Cleanup pending tasks, as TestBrowserThreadBundle will run them.
+  Mock::VerifyAndClearExpectations(&task_1);
+  Mock::VerifyAndClearExpectations(&task_2);
+  EXPECT_CALL(task_3, Run);
+  BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
+}
+
 TEST_F(BrowserTaskExecutorTest, UserVisibleOrBlockingTasksRunDuringStartup) {
   bool ran_best_effort = false;
   bool ran_user_visible = false;

content/browser/scheduler/browser_ui_thread_scheduler.cc

@@ -10,6 +10,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/process/process.h"
+#include "base/run_loop.h"
 #include "base/task/sequence_manager/sequence_manager.h"
 #include "base/task/sequence_manager/sequence_manager_impl.h"
 #include "base/task/sequence_manager/time_domain.h"
@@ -105,4 +106,23 @@ BrowserUIThreadScheduler::GetTaskRunner(QueueType queue_type) {
   return scoped_refptr<base::SingleThreadTaskRunner>();
 }
 
+void BrowserUIThreadScheduler::RunAllPendingTasksForTesting() {
+  std::vector<scoped_refptr<BrowserUIThreadTaskQueue>> fenced_queues;
+  for (const auto& queue : task_queues_) {
+    bool had_fence = queue.second->HasActiveFence();
+    queue.second->InsertFence(
+        base::sequence_manager::TaskQueue::InsertFencePosition::kNow);
+    // If there was a fence already this must be a re-entrant call to this
+    // method. The previous statement just moved the fence further back. In this
+    // case we do not remove the fence as the parent run loop needs all queues
+    // to be fenced to be able to exit the run loop (i.e. become idle)
+    if (!had_fence)
+      fenced_queues.push_back(queue.second);
+  }
+  base::RunLoop(base::RunLoop::Type::kNestableTasksAllowed).RunUntilIdle();
+  for (const auto& queue : fenced_queues) {
+    queue->RemoveFence();
+  }
+}
+
 }  // namespace content

content/browser/scheduler/browser_ui_thread_scheduler.h

@@ -46,6 +46,11 @@ class CONTENT_EXPORT BrowserUIThreadScheduler {
   scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunnerForTesting(
       QueueType queue_type);
 
+  // Adds a fence to all queues, runs all tasks until idle, and finally removes
+  // the fences. Note that the run loop will eventually become idle, as new
+  // tasks will not be scheduled due to the fence.
+  void RunAllPendingTasksForTesting();
+
  private:
   friend class BrowserTaskExecutor;
 

content/browser/scheduler/browser_ui_thread_scheduler_unittest.cc

@@ -12,16 +12,17 @@
 #include "base/run_loop.h"
 #include "base/task/post_task.h"
 #include "base/task/task_scheduler/task_scheduler.h"
+#include "base/test/mock_callback.h"
 #include "content/public/browser/browser_task_traits.h"
-#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using testing::ElementsAre;
-
 namespace content {
-
 namespace {
 
+using ::testing::ElementsAre;
+using ::testing::Invoke;
+using ::testing::Mock;
+
 void RecordRunOrder(std::vector<int>* run_order, int order) {
   run_order->push_back(order);
 }
@@ -48,24 +49,68 @@ base::OnceClosure PostOnDestruction(
 
 class BrowserUIThreadSchedulerTest : public testing::Test {
  public:
-  void SetUp() override {
+  BrowserUIThreadSchedulerTest() {
     browser_ui_thread_scheduler_ = std::make_unique<BrowserUIThreadScheduler>();
-  }
-
-  void TearDown() override { ShutdownBrowserUIThreadScheduler(); }
-
-  void ShutdownBrowserUIThreadScheduler() {
-    browser_ui_thread_scheduler_.reset();
+    for (int i = 0;
+         i < static_cast<int>(BrowserUIThreadTaskQueue::QueueType::kCount);
+         i++) {
+      auto queue_type = static_cast<BrowserUIThreadTaskQueue::QueueType>(i);
+      task_runners_.emplace(
+          queue_type,
+          browser_ui_thread_scheduler_->GetTaskRunnerForTesting(queue_type));
+    }
   }
 
  protected:
+  using QueueType = BrowserUIThreadScheduler::QueueType;
+  using MockTask =
+      testing::StrictMock<base::MockCallback<base::RepeatingCallback<void()>>>;
+
   std::unique_ptr<BrowserUIThreadScheduler> browser_ui_thread_scheduler_;
+  base::flat_map<BrowserUIThreadScheduler::QueueType,
+                 scoped_refptr<base::SingleThreadTaskRunner>>
+      task_runners_;
 };
 
+TEST_F(BrowserUIThreadSchedulerTest, RunAllPendingTasksForTesting) {
+  MockTask task;
+  MockTask nested_task;
+  EXPECT_CALL(task, Run).WillOnce(Invoke([&]() {
+    task_runners_[QueueType::kDefault]->PostTask(FROM_HERE, nested_task.Get());
+    task_runners_[QueueType::kBestEffort]->PostTask(FROM_HERE,
+                                                    nested_task.Get());
+  }));
+
+  task_runners_[QueueType::kDefault]->PostTask(FROM_HERE, task.Get());
+
+  browser_ui_thread_scheduler_->RunAllPendingTasksForTesting();
+
+  Mock::VerifyAndClearExpectations(&task);
+  EXPECT_CALL(nested_task, Run).Times(2);
+
+  browser_ui_thread_scheduler_->RunAllPendingTasksForTesting();
+}
+
+TEST_F(BrowserUIThreadSchedulerTest, RunAllPendingTasksForTestingIsReentrant) {
+  MockTask task_1;
+  MockTask task_2;
+  MockTask task_3;
+
+  EXPECT_CALL(task_1, Run).WillOnce(Invoke([&]() {
+    task_runners_[QueueType::kDefault]->PostTask(FROM_HERE, task_2.Get());
+    browser_ui_thread_scheduler_->RunAllPendingTasksForTesting();
+  }));
+  EXPECT_CALL(task_2, Run).WillOnce(Invoke([&]() {
+    task_runners_[QueueType::kDefault]->PostTask(FROM_HERE, task_3.Get());
+  }));
+
+  task_runners_[QueueType::kDefault]->PostTask(FROM_HERE, task_1.Get());
+  browser_ui_thread_scheduler_->RunAllPendingTasksForTesting();
+}
+
 TEST_F(BrowserUIThreadSchedulerTest, SimplePosting) {
   scoped_refptr<base::SingleThreadTaskRunner> tq =
-      browser_ui_thread_scheduler_->GetTaskRunnerForTesting(
-          BrowserUIThreadScheduler::QueueType::kDefault);
+      task_runners_[QueueType::kDefault];
 
   std::vector<int> order;
   tq->PostTask(FROM_HERE, base::BindOnce(RecordRunOrder, &order, 1));
@@ -79,8 +124,7 @@ TEST_F(BrowserUIThreadSchedulerTest, SimplePosting) {
 
 TEST_F(BrowserUIThreadSchedulerTest, DestructorPostChainDuringShutdown) {
   scoped_refptr<base::SingleThreadTaskRunner> task_queue =
-      browser_ui_thread_scheduler_->GetTaskRunnerForTesting(
-          BrowserUIThreadScheduler::QueueType::kDefault);
+      task_runners_[QueueType::kDefault];
 
   bool run = false;
   task_queue->PostTask(
@@ -92,7 +136,7 @@ TEST_F(BrowserUIThreadSchedulerTest, DestructorPostChainDuringShutdown) {
                                 [](bool* run) { *run = true; }, &run)))));
 
   EXPECT_FALSE(run);
-  ShutdownBrowserUIThreadScheduler();
+  browser_ui_thread_scheduler_.reset();
 
   EXPECT_TRUE(run);
 }

content/public/browser/browser_thread.h

@@ -140,9 +140,9 @@ class CONTENT_EXPORT BrowserThread {
   // creating thread etc). Note: see base::OnTaskRunnerDeleter and
   // base::RefCountedDeleteOnSequence to bind to SequencedTaskRunner instead of
   // specific BrowserThreads.
-  template<ID thread>
+  template <ID thread>
   struct DeleteOnThread {
-    template<typename T>
+    template <typename T>
     static void Destruct(const T* x) {
       if (CurrentlyOn(thread)) {
         delete x;
@@ -180,12 +180,24 @@ class CONTENT_EXPORT BrowserThread {
   //
   // Note: see base::OnTaskRunnerDeleter and base::RefCountedDeleteOnSequence to
   // bind to SequencedTaskRunner instead of specific BrowserThreads.
-  struct DeleteOnUIThread : public DeleteOnThread<UI> { };
-  struct DeleteOnIOThread : public DeleteOnThread<IO> { };
+  struct DeleteOnUIThread : public DeleteOnThread<UI> {};
+  struct DeleteOnIOThread : public DeleteOnThread<IO> {};
 
   // Returns an appropriate error message for when DCHECK_CURRENTLY_ON() fails.
   static std::string GetDCheckCurrentlyOnErrorMessage(ID expected);
 
+  // Runs all pending tasks for the given thread. Tasks posted after this method
+  // is called (in particular any task posted from within any of the pending
+  // tasks) will be queued but not run. Conceptually this call will disable all
+  // queues, run any pending tasks, and re-enable all the queues.
+  //
+  // If any of the pending tasks posted a task, these could be run by calling
+  // this method again or running a regular RunLoop. But if that were the case
+  // you should probably rewrite you tests to wait for a specific event instead.
+  //
+  // NOTE: Can only be called from the UI thread.
+  static void RunAllPendingTasksOnThreadForTesting(ID identifier);
+
  protected:
   // For DeleteSoon(). Requires that the BrowserThread with the provided
   // |identifier| was started.

content/public/test/test_utils.cc

@@ -17,6 +17,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
+#include "base/task/sequence_manager/sequence_manager.h"
 #include "base/task/task_scheduler/task_scheduler.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
@@ -28,6 +29,7 @@
 #include "content/public/browser/browser_child_process_host_iterator.h"
 #include "content/public/browser/browser_plugin_guest_delegate.h"
 #include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ -128,31 +130,14 @@ void RunThisRunLoop(base::RunLoop* run_loop) {
 
 void RunAllPendingInMessageLoop() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  base::RunLoop run_loop;
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, GetDeferredQuitTaskForRunLoop(&run_loop));
-  RunThisRunLoop(&run_loop);
+  RunAllPendingInMessageLoop(BrowserThread::UI);
 }
 
 void RunAllPendingInMessageLoop(BrowserThread::ID thread_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (thread_id == BrowserThread::UI) {
-    RunAllPendingInMessageLoop();
-    return;
+  // See comment for |kNumQuitDeferrals| for why this is needed.
+  for (int i = 0; i <= kNumQuitDeferrals; ++i) {
+    BrowserThread::RunAllPendingTasksOnThreadForTesting(thread_id);
   }
-
-  // Post a DeferredQuitRunLoop() task to |thread_id|. Then, run a RunLoop on
-  // this thread. When a few generations of pending tasks have run on
-  // |thread_id|, a task will be posted to this thread to exit the RunLoop.
-  base::RunLoop run_loop;
-  const base::Closure post_quit_run_loop_to_ui_thread = base::Bind(
-      base::IgnoreResult(&base::SingleThreadTaskRunner::PostTask),
-      base::ThreadTaskRunnerHandle::Get(), FROM_HERE, run_loop.QuitClosure());
-  base::PostTaskWithTraits(
-      FROM_HERE, {thread_id},
-      base::BindOnce(&DeferredQuitRunLoop, post_quit_run_loop_to_ui_thread,
-                     kNumQuitDeferrals));
-  RunThisRunLoop(&run_loop);
 }
 
 void RunAllTasksUntilIdle() {