Introduce content::GetUIThreadTaskRunner() and its IO counterpart

As agreed upon in design doc @
https://docs.google.com/document/d/1tssusPykvx3g0gvbvU4HxGyn3MjJlIylnsH13-Tv6s4/edit?ts=5e1c66d5&pli=1#bookmark=id.ll79iqi5rlpp

base::ThreadPool counterpart to move away from post_task.h is @
https://chromium-review.googlesource.com/c/chromium/src/+/1977964

API usage migration will be done in a follow-up mega CL.
This CL at least uses it in its own tests.

See https://chromium-review.googlesource.com/c/chromium/src/+/2015655
for how this will look in practice for a few callsites.

Bug: 1026641
Change-Id: I98771fd68a513bf0a4776672a8d75fcbbb200bea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2014055
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Darin Fisher <darin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735734}

base/task/post_task.cc

@@ -45,7 +45,8 @@ TaskExecutor* GetTaskExecutorForTraits(const TaskTraits& traits) {
       traits.extension_id() != TaskTraitsExtensionStorage::kInvalidExtensionId;
   DCHECK(has_extension ^ traits.use_thread_pool())
       << "A destination (e.g. ThreadPool or BrowserThread) must be specified "
-         "to use the post_task.h API.";
+         "to use the post_task.h API. However, you should prefer the direct "
+         "thread_pool.h or browser_thread.h APIs in new code.";
 
   if (traits.use_thread_pool()) {
     DCHECK(ThreadPoolInstance::Get())

content/browser/browser_thread_impl.cc

@@ -84,6 +84,16 @@ BrowserThreadGlobals& GetBrowserThreadGlobals() {
 
 }  // namespace
 
+scoped_refptr<base::SingleThreadTaskRunner> GetUIThreadTaskRunner(
+    const BrowserTaskTraits& traits) {
+  return BrowserTaskExecutor::GetUIThreadTaskRunner(traits);
+}
+
+scoped_refptr<base::SingleThreadTaskRunner> GetIOThreadTaskRunner(
+    const BrowserTaskTraits& traits) {
+  return BrowserTaskExecutor::GetIOThreadTaskRunner(traits);
+}
+
 BrowserThreadImpl::BrowserThreadImpl(
     ID identifier,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner)

content/browser/browser_thread_unittest.cc

@@ -165,8 +165,7 @@ class UIThreadDestructionObserver
   explicit UIThreadDestructionObserver(bool* did_shutdown,
                                        base::OnceClosure callback)
       : callback_task_runner_(base::ThreadTaskRunnerHandle::Get()),
-        ui_task_runner_(
-            base::CreateSingleThreadTaskRunner({BrowserThread::UI})),
+        ui_task_runner_(GetUIThreadTaskRunner({})),
         callback_(std::move(callback)),
         did_shutdown_(did_shutdown) {
     ui_task_runner_->PostTask(FROM_HERE, base::BindOnce(&Watch, this));
@@ -223,8 +222,7 @@ TEST_F(BrowserThreadTest, ReleasedOnCorrectThread) {
 }
 
 TEST_F(BrowserThreadTest, PostTaskViaTaskRunner) {
-  scoped_refptr<base::TaskRunner> task_runner =
-      base::CreateTaskRunner({BrowserThread::IO});
+  scoped_refptr<base::TaskRunner> task_runner = GetIOThreadTaskRunner({});
   base::RunLoop run_loop;
   EXPECT_TRUE(task_runner->PostTask(
       FROM_HERE, base::BindOnce(&BasicFunction, run_loop.QuitWhenIdleClosure(),
@@ -234,7 +232,7 @@ TEST_F(BrowserThreadTest, PostTaskViaTaskRunner) {
 
 TEST_F(BrowserThreadTest, PostTaskViaSequencedTaskRunner) {
   scoped_refptr<base::SequencedTaskRunner> task_runner =
-      base::CreateSequencedTaskRunner({BrowserThread::IO});
+      GetIOThreadTaskRunner({});
   base::RunLoop run_loop;
   EXPECT_TRUE(task_runner->PostTask(
       FROM_HERE, base::BindOnce(&BasicFunction, run_loop.QuitWhenIdleClosure(),
@@ -244,7 +242,7 @@ TEST_F(BrowserThreadTest, PostTaskViaSequencedTaskRunner) {
 
 TEST_F(BrowserThreadTest, PostTaskViaSingleThreadTaskRunner) {
   scoped_refptr<base::SingleThreadTaskRunner> task_runner =
-      base::CreateSingleThreadTaskRunner({BrowserThread::IO});
+      GetIOThreadTaskRunner({});
   base::RunLoop run_loop;
   EXPECT_TRUE(task_runner->PostTask(
       FROM_HERE, base::BindOnce(&BasicFunction, run_loop.QuitWhenIdleClosure(),
@@ -255,7 +253,7 @@ TEST_F(BrowserThreadTest, PostTaskViaSingleThreadTaskRunner) {
 #if defined(OS_WIN)
 TEST_F(BrowserThreadTest, PostTaskViaCOMSTATaskRunner) {
   scoped_refptr<base::SingleThreadTaskRunner> task_runner =
-      base::CreateCOMSTATaskRunner({BrowserThread::UI});
+      GetUIThreadTaskRunner({});
   base::RunLoop run_loop;
   EXPECT_TRUE(task_runner->PostTask(
       FROM_HERE, base::BindOnce(&BasicFunction, run_loop.QuitWhenIdleClosure(),
@@ -266,7 +264,7 @@ TEST_F(BrowserThreadTest, PostTaskViaCOMSTATaskRunner) {
 
 TEST_F(BrowserThreadTest, ReleaseViaTaskRunner) {
   scoped_refptr<base::SingleThreadTaskRunner> task_runner =
-      base::CreateSingleThreadTaskRunner({BrowserThread::UI});
+      GetUIThreadTaskRunner({});
   base::RunLoop run_loop;
   ExpectRelease(run_loop.QuitWhenIdleClosure());
   task_runner->ReleaseSoon(FROM_HERE, base::WrapRefCounted(this));
@@ -277,9 +275,8 @@ TEST_F(BrowserThreadTest, PostTaskAndReply) {
   // Most of the heavy testing for PostTaskAndReply() is done inside the
   // task runner test.  This just makes sure we get piped through at all.
   base::RunLoop run_loop;
-  ASSERT_TRUE(base::PostTaskAndReply(FROM_HERE, {BrowserThread::IO},
-                                     base::DoNothing(),
-                                     run_loop.QuitWhenIdleClosure()));
+  ASSERT_TRUE(GetIOThreadTaskRunner({})->PostTaskAndReply(
+      FROM_HERE, base::DoNothing(), run_loop.QuitWhenIdleClosure()));
   run_loop.Run();
 }
 

content/browser/scheduler/browser_task_executor.cc

@@ -12,6 +12,7 @@
 #include "base/no_destructor.h"
 #include "base/run_loop.h"
 #include "base/task/post_task.h"
+#include "base/task/task_traits_extension.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
@@ -24,47 +25,27 @@
 #include "base/android/task_scheduler/post_task_android.h"
 #endif
 
+using QueueType = content::BrowserTaskQueues::QueueType;
+
 namespace content {
 namespace {
 
-using QueueType = ::content::BrowserTaskQueues::QueueType;
+// Returns the BrowserThread::ID stored in |traits| which must be coming from a
+// call through BaseBrowserTaskExecutor and hence have the
+// BrowserTaskTraitsExtension.
+BrowserThread::ID ExtractBrowserThreadId(const base::TaskTraits& traits) {
+  DCHECK_EQ(BrowserTaskTraitsExtension::kExtensionId, traits.extension_id());
+  const BrowserTaskTraitsExtension extension =
+      traits.GetExtension<BrowserTaskTraitsExtension>();
+
+  const BrowserThread::ID thread_id = extension.browser_thread();
+  DCHECK_GE(thread_id, 0);
+  return thread_id;
+}
 
 // |g_browser_task_executor| is intentionally leaked on shutdown.
 BrowserTaskExecutor* g_browser_task_executor = nullptr;
 
-QueueType GetQueueType(const base::TaskTraits& traits,
-                       BrowserTaskType task_type) {
-  switch (task_type) {
-    case BrowserTaskType::kBootstrap:
-      // Note we currently ignore the priority for bootstrap tasks.
-      return QueueType::kBootstrap;
-
-    case BrowserTaskType::kNavigation:
-    case BrowserTaskType::kPreconnect:
-      // Note we currently ignore the priority for navigation and preconnection
-      // tasks.
-      return QueueType::kNavigationAndPreconnection;
-
-    case BrowserTaskType::kDefault:
-      // Defer to traits.priority() below.
-      break;
-
-    case BrowserTaskType::kBrowserTaskType_Last:
-      NOTREACHED();
-  }
-
-  switch (traits.priority()) {
-    case base::TaskPriority::BEST_EFFORT:
-      return QueueType::kBestEffort;
-
-    case base::TaskPriority::USER_VISIBLE:
-      return QueueType::kUserVisible;
-
-    case base::TaskPriority::USER_BLOCKING:
-      return QueueType::kUserBlocking;
-  }
-}
-
 }  // namespace
 
 BaseBrowserTaskExecutor::BaseBrowserTaskExecutor() = default;
@@ -77,30 +58,30 @@ bool BaseBrowserTaskExecutor::PostDelayedTask(const base::Location& from_here,
                                               base::TimeDelta delay) {
   if (traits.extension_id() != BrowserTaskTraitsExtension::kExtensionId ||
       traits.GetExtension<BrowserTaskTraitsExtension>().nestable()) {
-    return GetTaskRunner(traits)->PostDelayedTask(from_here, std::move(task),
-                                                  delay);
+    return GetTaskRunner(ExtractBrowserThreadId(traits), traits)
+        ->PostDelayedTask(from_here, std::move(task), delay);
   } else {
-    return GetTaskRunner(traits)->PostNonNestableDelayedTask(
-        from_here, std::move(task), delay);
+    return GetTaskRunner(ExtractBrowserThreadId(traits), traits)
+        ->PostNonNestableDelayedTask(from_here, std::move(task), delay);
   }
 }
 
 scoped_refptr<base::TaskRunner> BaseBrowserTaskExecutor::CreateTaskRunner(
     const base::TaskTraits& traits) {
-  return GetTaskRunner(traits);
+  return GetTaskRunner(ExtractBrowserThreadId(traits), traits);
 }
 
 scoped_refptr<base::SequencedTaskRunner>
 BaseBrowserTaskExecutor::CreateSequencedTaskRunner(
     const base::TaskTraits& traits) {
-  return GetTaskRunner(traits);
+  return GetTaskRunner(ExtractBrowserThreadId(traits), traits);
 }
 
 scoped_refptr<base::SingleThreadTaskRunner>
 BaseBrowserTaskExecutor::CreateSingleThreadTaskRunner(
     const base::TaskTraits& traits,
     base::SingleThreadTaskRunnerThreadMode thread_mode) {
-  return GetTaskRunner(traits);
+  return GetTaskRunner(ExtractBrowserThreadId(traits), traits);
 }
 
 #if defined(OS_WIN)
@@ -108,42 +89,71 @@ scoped_refptr<base::SingleThreadTaskRunner>
 BaseBrowserTaskExecutor::CreateCOMSTATaskRunner(
     const base::TaskTraits& traits,
     base::SingleThreadTaskRunnerThreadMode thread_mode) {
-  return GetTaskRunner(traits);
+  return GetTaskRunner(ExtractBrowserThreadId(traits), traits);
 }
 #endif  // defined(OS_WIN)
 
 scoped_refptr<base::SingleThreadTaskRunner>
-BaseBrowserTaskExecutor::GetTaskRunner(const base::TaskTraits& traits) const {
-  auto id_and_queue = GetThreadIdAndQueueType(traits);
+BaseBrowserTaskExecutor::GetTaskRunner(BrowserThread::ID identifier,
+                                       const base::TaskTraits& traits) const {
+  DCHECK(traits.extension_id() ==
+             base::TaskTraitsExtensionStorage::kInvalidExtensionId ||
+         ExtractBrowserThreadId(traits) == identifier);
+
+  const QueueType queue_type = GetQueueType(traits);
 
-  switch (id_and_queue.thread_id) {
+  switch (identifier) {
     case BrowserThread::UI: {
-      return browser_ui_thread_handle_->GetBrowserTaskRunner(
-          id_and_queue.queue_type);
+      return browser_ui_thread_handle_->GetBrowserTaskRunner(queue_type);
     }
     case BrowserThread::IO:
-      return browser_io_thread_handle_->GetBrowserTaskRunner(
-          id_and_queue.queue_type);
+      return browser_io_thread_handle_->GetBrowserTaskRunner(queue_type);
     case BrowserThread::ID_COUNT:
       NOTREACHED();
   }
   return nullptr;
 }
 
-BaseBrowserTaskExecutor::ThreadIdAndQueueType
-BaseBrowserTaskExecutor::GetThreadIdAndQueueType(
-    const base::TaskTraits& traits) const {
-  DCHECK_EQ(BrowserTaskTraitsExtension::kExtensionId, traits.extension_id());
-  const BrowserTaskTraitsExtension extension =
-      traits.GetExtension<BrowserTaskTraitsExtension>();
+// static
+QueueType BaseBrowserTaskExecutor::GetQueueType(
+    const base::TaskTraits& traits) {
+  if (traits.extension_id() == BrowserTaskTraitsExtension::kExtensionId) {
+    const BrowserTaskTraitsExtension extension =
+        traits.GetExtension<BrowserTaskTraitsExtension>();
+
+    const BrowserTaskType task_type = extension.task_type();
+    DCHECK_LT(task_type, BrowserTaskType::kBrowserTaskType_Last);
+
+    switch (task_type) {
+      case BrowserTaskType::kBootstrap:
+        // Note we currently ignore the priority for bootstrap tasks.
+        return QueueType::kBootstrap;
+
+      case BrowserTaskType::kNavigation:
+      case BrowserTaskType::kPreconnect:
+        // Note we currently ignore the priority for navigation and
+        // preconnection tasks.
+        return QueueType::kNavigationAndPreconnection;
+
+      case BrowserTaskType::kDefault:
+        // Defer to traits.priority() below.
+        break;
+
+      case BrowserTaskType::kBrowserTaskType_Last:
+        NOTREACHED();
+    }
+  }
 
-  const BrowserThread::ID thread_id = extension.browser_thread();
-  DCHECK_GE(thread_id, 0);
+  switch (traits.priority()) {
+    case base::TaskPriority::BEST_EFFORT:
+      return QueueType::kBestEffort;
 
-  const BrowserTaskType task_type = extension.task_type();
-  DCHECK_LT(task_type, BrowserTaskType::kBrowserTaskType_Last);
+    case base::TaskPriority::USER_VISIBLE:
+      return QueueType::kUserVisible;
 
-  return {thread_id, GetQueueType(traits, task_type)};
+    case base::TaskPriority::USER_BLOCKING:
+      return QueueType::kUserBlocking;
+  }
 }
 
 BrowserTaskExecutor::BrowserTaskExecutor(
@@ -282,6 +292,20 @@ void BrowserTaskExecutor::EnableAllQueues() {
   g_browser_task_executor->browser_io_thread_handle_->EnableAllQueues();
 }
 
+// static
+scoped_refptr<base::SingleThreadTaskRunner>
+BrowserTaskExecutor::GetUIThreadTaskRunner(const BrowserTaskTraits& traits) {
+  DCHECK(g_browser_task_executor);
+  return g_browser_task_executor->GetTaskRunner(BrowserThread::UI, traits);
+}
+
+// static
+scoped_refptr<base::SingleThreadTaskRunner>
+BrowserTaskExecutor::GetIOThreadTaskRunner(const BrowserTaskTraits& traits) {
+  DCHECK(g_browser_task_executor);
+  return g_browser_task_executor->GetTaskRunner(BrowserThread::IO, traits);
+}
+
 // static
 void BrowserTaskExecutor::InitializeIOThread() {
   DCHECK(g_browser_task_executor);

content/browser/scheduler/browser_task_executor.h

@@ -58,20 +58,25 @@ class CONTENT_EXPORT BaseBrowserTaskExecutor : public base::TaskExecutor {
       base::SingleThreadTaskRunnerThreadMode thread_mode) override;
 #endif  // defined(OS_WIN)
 
-  struct ThreadIdAndQueueType {
-    BrowserThread::ID thread_id;
-    BrowserTaskQueues::QueueType queue_type;
-  };
-
-  ThreadIdAndQueueType GetThreadIdAndQueueType(
+  // Returns the task runner for |traits| under |identifier|. Note: during the
+  // migration away from task traits extension, |traits| may also contain a
+  // browser thread id, if so, it should match |identifier| (|identifier| has to
+  // be provided explicitly because in the new source of tasks it's not part of
+  // |traits|) -- ref. crbug.com/1026641.
+  scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner(
+      BrowserThread::ID identifier,
       const base::TaskTraits& traits) const;
 
+  // Helper to match a QueueType from TaskTraits.
+  // TODO(1026641): Take BrowserTaskTraits as a parameter when getting off the
+  // need to support base::TaskTraits currently passed to this class in its role
+  // as a base::TaskExecutor.
+  static content::BrowserTaskQueues::QueueType GetQueueType(
+      const base::TaskTraits& traits);
+
  protected:
   virtual BrowserThread::ID GetCurrentThreadID() const = 0;
 
-  scoped_refptr<base::SingleThreadTaskRunner> GetTaskRunner(
-      const base::TaskTraits& traits) const;
-
   scoped_refptr<BrowserUIThreadScheduler::Handle> browser_ui_thread_handle_;
   scoped_refptr<BrowserIOThreadDelegate::Handle> browser_io_thread_handle_;
 };
@@ -126,6 +131,16 @@ class CONTENT_EXPORT BrowserTaskExecutor : public BaseBrowserTaskExecutor {
   // Can be called multiple times.
   static void EnableAllQueues();
 
+  // Helpers to statically call into BaseBrowserTaskExecutor::GetTaskRunner()
+  // from browser_thread_impl.cc. Callers should use browser_thread.h's
+  // GetUIThreadTaskRunner over this.
+  // TODO(1026641): Clean up this indirection after the migration (once
+  // registering a base::BrowserTaskExecutor is no longer necessary).
+  static scoped_refptr<base::SingleThreadTaskRunner> GetUIThreadTaskRunner(
+      const BrowserTaskTraits& traits);
+  static scoped_refptr<base::SingleThreadTaskRunner> GetIOThreadTaskRunner(
+      const BrowserTaskTraits& traits);
+
   // As Create but with the user provided objects. Must call
   // BindToUIThreadForTesting before tasks can be run on the UI thread.
   static void CreateForTesting(

content/browser/scheduler/browser_task_executor_unittest.cc

@@ -11,7 +11,6 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/task/post_task.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool/thread_pool_instance.h"
 #include "base/test/bind_test_util.h"
@@ -49,15 +48,15 @@ using StrictMockTask =
     testing::StrictMock<base::MockCallback<base::RepeatingCallback<void()>>>;
 
 TEST_F(BrowserTaskExecutorTest, RegisterExecutorForBothThreads) {
-  base::PostTask(FROM_HERE, {BrowserThread::UI}, base::BindOnce([]() {
-                   EXPECT_THAT(base::GetTaskExecutorForCurrentThread(),
-                               NotNull());
-                 }));
+  GetUIThreadTaskRunner({})->PostTask(
+      FROM_HERE, base::BindOnce([]() {
+        EXPECT_THAT(base::GetTaskExecutorForCurrentThread(), NotNull());
+      }));
 
-  base::PostTask(FROM_HERE, {BrowserThread::IO}, base::BindOnce([]() {
-                   EXPECT_THAT(base::GetTaskExecutorForCurrentThread(),
-                               NotNull());
-                 }));
+  GetIOThreadTaskRunner({})->PostTask(
+      FROM_HERE, base::BindOnce([]() {
+        EXPECT_THAT(base::GetTaskExecutorForCurrentThread(), NotNull());
+      }));
 
   BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::UI);
   BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
@@ -70,7 +69,7 @@ TEST_F(BrowserTaskExecutorTest, RunAllPendingTasksForTestingOnUI) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task_2.Get());
   }));
 
-  base::PostTask(FROM_HERE, {BrowserThread::UI}, task_1.Get());
+  GetUIThreadTaskRunner({})->PostTask(FROM_HERE, task_1.Get());
 
   BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::UI);
 
@@ -84,10 +83,10 @@ TEST_F(BrowserTaskExecutorTest, RunAllPendingTasksForTestingOnIO) {
   StrictMockTask task_1;
   StrictMockTask task_2;
   EXPECT_CALL(task_1, Run).WillOnce(testing::Invoke([&]() {
-    base::PostTask(FROM_HERE, {BrowserThread::IO}, task_2.Get());
+    GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_2.Get());
   }));
 
-  base::PostTask(FROM_HERE, {BrowserThread::IO}, task_1.Get());
+  GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_1.Get());
 
   BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
 
@@ -103,15 +102,15 @@ TEST_F(BrowserTaskExecutorTest, RunAllPendingTasksForTestingOnIOIsReentrant) {
   StrictMockTask task_3;
 
   EXPECT_CALL(task_1, Run).WillOnce(Invoke([&]() {
-    base::PostTask(FROM_HERE, {BrowserThread::IO}, task_2.Get());
+    GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_2.Get());
     BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(
         BrowserThread::IO);
   }));
   EXPECT_CALL(task_2, Run).WillOnce(Invoke([&]() {
-    base::PostTask(FROM_HERE, {BrowserThread::IO}, task_3.Get());
+    GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_3.Get());
   }));
 
-  base::PostTask(FROM_HERE, {BrowserThread::IO}, task_1.Get());
+  GetIOThreadTaskRunner({})->PostTask(FROM_HERE, task_1.Get());
   BrowserTaskExecutor::RunAllPendingTasksOnThreadForTesting(BrowserThread::IO);
 
   // Cleanup pending tasks, as BrowserTaskEnvironment will run them.
@@ -134,44 +133,33 @@ class BrowserTaskTraitsMappingTest : public BrowserTaskExecutorTest {
     }
   };
 
-  template <BrowserThread::ID ID>
-  void CheckExpectations() {
-    EXPECT_EQ(GetQueueType({ID, TaskPriority::BEST_EFFORT}),
-              QueueType::kBestEffort);
-    EXPECT_EQ(GetQueueType({ID, TaskPriority::USER_VISIBLE}),
-              QueueType::kUserVisible);
-    EXPECT_EQ(GetQueueType({ID, TaskPriority::USER_BLOCKING}),
-              QueueType::kUserBlocking);
-
-    EXPECT_EQ(GetQueueType({ID, BrowserTaskType::kBootstrap}),
-              QueueType::kBootstrap);
-    EXPECT_EQ(GetQueueType({ID, BrowserTaskType::kDefault}),
-              QueueType::kUserBlocking);
-    EXPECT_EQ(GetQueueType({ID, BrowserTaskType::kNavigation}),
-              QueueType::kNavigationAndPreconnection);
-    EXPECT_EQ(GetQueueType({ID, BrowserTaskType::kPreconnect}),
-              QueueType::kNavigationAndPreconnection);
-
-    EXPECT_EQ(GetQueueType({ID}), QueueType::kUserBlocking);
-  }
-
  private:
-  QueueType GetQueueType(const base::TaskTraits& traits) {
-    return test_executor_.GetThreadIdAndQueueType(traits).queue_type;
-  }
-
   TestExecutor test_executor_;
 };
 
 TEST_F(BrowserTaskTraitsMappingTest, BrowserTaskTraitsMapToProperPriorities) {
-  CheckExpectations<BrowserThread::UI>();
-  CheckExpectations<BrowserThread::IO>();
+  EXPECT_EQ(BrowserTaskExecutor::GetQueueType({TaskPriority::BEST_EFFORT}),
+            QueueType::kBestEffort);
+  EXPECT_EQ(BrowserTaskExecutor::GetQueueType({TaskPriority::USER_VISIBLE}),
+            QueueType::kUserVisible);
+  EXPECT_EQ(BrowserTaskExecutor::GetQueueType({TaskPriority::USER_BLOCKING}),
+            QueueType::kUserBlocking);
+
+  EXPECT_EQ(BrowserTaskExecutor::GetQueueType({BrowserTaskType::kBootstrap}),
+            QueueType::kBootstrap);
+  EXPECT_EQ(BrowserTaskExecutor::GetQueueType({BrowserTaskType::kDefault}),
+            QueueType::kUserBlocking);
+  EXPECT_EQ(BrowserTaskExecutor::GetQueueType({BrowserTaskType::kNavigation}),
+            QueueType::kNavigationAndPreconnection);
+  EXPECT_EQ(BrowserTaskExecutor::GetQueueType({BrowserTaskType::kPreconnect}),
+            QueueType::kNavigationAndPreconnection);
+
+  EXPECT_EQ(BrowserTaskExecutor::GetQueueType({}), QueueType::kUserBlocking);
 }
 
 TEST_F(BrowserTaskTraitsMappingTest,
        UIThreadTaskRunnerHasSamePriorityAsUIBlocking) {
-  auto ui_blocking = base::CreateSingleThreadTaskRunner(
-      {BrowserThread::UI, TaskPriority::USER_BLOCKING});
+  auto ui_blocking = GetUIThreadTaskRunner({TaskPriority::USER_BLOCKING});
   auto thread_task_runner = base::ThreadTaskRunnerHandle::Get();
 
   std::vector<int> order;
@@ -213,8 +201,6 @@ class BrowserTaskExecutorWithCustomSchedulerTest : public testing::Test {
   };
 
  public:
-  using QueueType = BrowserTaskQueues::QueueType;
-
   ~BrowserTaskExecutorWithCustomSchedulerTest() override {
     BrowserTaskExecutor::ResetForTesting();
   }
@@ -229,14 +215,17 @@ TEST_F(BrowserTaskExecutorWithCustomSchedulerTest,
   StrictMockTask user_visible;
   StrictMockTask user_blocking;
 
-  base::PostTask(FROM_HERE,
-                 {BrowserThread::UI, base::TaskPriority::BEST_EFFORT},
+  GetUIThreadTaskRunner({base::TaskPriority::BEST_EFFORT})
+      ->PostTask(FROM_HERE,
+
                  best_effort.Get());
-  base::PostTask(FROM_HERE,
-                 {BrowserThread::UI, base::TaskPriority::USER_VISIBLE},
+  GetUIThreadTaskRunner({base::TaskPriority::USER_VISIBLE})
+      ->PostTask(FROM_HERE,
+
                  user_visible.Get());
-  base::PostTask(FROM_HERE,
-                 {BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
+  GetUIThreadTaskRunner({base::TaskPriority::USER_BLOCKING})
+      ->PostTask(FROM_HERE,
+
                  user_blocking.Get());
 
   EXPECT_CALL(user_visible, Run);
@@ -247,19 +236,20 @@ TEST_F(BrowserTaskExecutorWithCustomSchedulerTest,
 
 TEST_F(BrowserTaskExecutorWithCustomSchedulerTest,
        BestEffortTasksRunAfterStartup) {
-  auto ui_best_effort_runner = base::CreateSingleThreadTaskRunner(
-      {BrowserThread::UI, base::TaskPriority::BEST_EFFORT});
+  auto ui_best_effort_runner =
+      GetUIThreadTaskRunner({base::TaskPriority::BEST_EFFORT});
 
   StrictMockTask best_effort;
 
   ui_best_effort_runner->PostTask(FROM_HERE, best_effort.Get());
   ui_best_effort_runner->PostDelayedTask(
       FROM_HERE, best_effort.Get(), base::TimeDelta::FromMilliseconds(100));
-  base::PostDelayedTask(
-      FROM_HERE, {BrowserThread::UI, base::TaskPriority::BEST_EFFORT},
-      best_effort.Get(), base::TimeDelta::FromMilliseconds(100));
-  base::PostTask(FROM_HERE,
-                 {BrowserThread::UI, base::TaskPriority::BEST_EFFORT},
+  GetUIThreadTaskRunner({base::TaskPriority::BEST_EFFORT})
+      ->PostDelayedTask(FROM_HERE, best_effort.Get(),
+                        base::TimeDelta::FromMilliseconds(100));
+  GetUIThreadTaskRunner({base::TaskPriority::BEST_EFFORT})
+      ->PostTask(FROM_HERE,
+
                  best_effort.Get());
   task_environment_.RunUntilIdle();
 

content/public/browser/browser_task_traits.h

@@ -80,7 +80,8 @@ class CONTENT_EXPORT BrowserTaskTraitsExtension {
           base::trait_helpers::AreValidTraits<ValidTrait, ArgTypes...>::value>>
   constexpr BrowserTaskTraitsExtension(ArgTypes... args)
       : browser_thread_(
-            base::trait_helpers::GetEnum<BrowserThread::ID>(args...)),
+            base::trait_helpers::GetEnum<BrowserThread::ID,
+                                         BrowserThread::ID_COUNT>(args...)),
         task_type_(
             base::trait_helpers::GetEnum<BrowserTaskType,
                                          BrowserTaskType::kDefault>(args...)),
@@ -105,7 +106,13 @@ class CONTENT_EXPORT BrowserTaskTraitsExtension {
         static_cast<bool>(extension.data[2]));
   }
 
-  constexpr BrowserThread::ID browser_thread() const { return browser_thread_; }
+  constexpr BrowserThread::ID browser_thread() const {
+    // TODO(1026641): Migrate to BrowserTaskTraits under which BrowserThread is
+    // not a trait. Until then, only code that knows traits have explicitly set
+    // the BrowserThread trait should check this field.
+    DCHECK_NE(browser_thread_, BrowserThread::ID_COUNT);
+    return browser_thread_;
+  }
 
   constexpr BrowserTaskType task_type() const { return task_type_; }
 
@@ -135,6 +142,40 @@ constexpr base::TaskTraitsExtensionStorage MakeTaskTraitsExtension(
       .Serialize();
 }
 
+class CONTENT_EXPORT BrowserTaskTraits : public base::TaskTraits {
+ public:
+  struct ValidTrait : public base::TaskTraits::ValidTrait {
+    ValidTrait(BrowserTaskType);
+    ValidTrait(NonNestable);
+
+    // TODO(1026641): Reconsider whether BrowserTaskTraits should really be
+    // supporting base::TaskPriority.
+    ValidTrait(base::TaskPriority);
+  };
+
+  // TODO(1026641): Get rid of BrowserTaskTraitsExtension and store its members
+  // (|task_type_| & |nestable_|) directly in BrowserTaskTraits.
+  template <
+      class... ArgTypes,
+      class CheckArgumentsAreValid = std::enable_if_t<
+          base::trait_helpers::AreValidTraits<ValidTrait, ArgTypes...>::value>>
+  constexpr BrowserTaskTraits(ArgTypes... args) : base::TaskTraits(args...) {}
+
+  BrowserTaskType task_type() {
+    return GetExtension<BrowserTaskTraitsExtension>().task_type();
+  }
+
+  // Returns true if tasks with these traits may run in a nested RunLoop.
+  bool nestable() const {
+    return GetExtension<BrowserTaskTraitsExtension>().nestable();
+  }
+};
+
+static_assert(sizeof(BrowserTaskTraits) == sizeof(base::TaskTraits),
+              "During the migration away from BrowserTasktraitsExtension, "
+              "BrowserTaskTraits must only use base::TaskTraits for storage "
+              "to prevent slicing.");
+
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_BROWSER_TASK_TRAITS_H_

content/public/browser/browser_task_traits_unittest.nc

@@ -14,11 +14,9 @@ namespace content {
 #if defined(NCTEST_BROWSER_TASK_TRAITS_MULTIPLE_THREADS)  // [r"The traits bag contains multiple traits of the same type."]
 constexpr base::TaskTraits traits = {BrowserThread::UI,
                                      BrowserThread::IO};
-#elif defined(NCTEST_BROWSER_TASK_TRAITS_MULTIPLE_TASK_TYPES)  // [r"The traits bag is missing a required trait."]
+#elif defined(NCTEST_BROWSER_TASK_TRAITS_MULTIPLE_TASK_TYPES)  // [r"The traits bag contains multiple traits of the same type."]
 constexpr base::TaskTraits traits = {BrowserTaskType::kNavigation,
                                      BrowserTaskType::kBootstrap};
-#elif defined(NCTEST_BROWSER_TASK_TRAITS_NO_THREAD_ID)  // [r"The traits bag is missing a required trait."]
-constexpr base::TaskTraits traits = {BrowserTaskType::kNavigation};
 #endif
 
 

content/public/browser/browser_thread.h

@@ -21,6 +21,11 @@
 
 namespace content {
 
+// TODO(1026641): Include browser_task_traits.h directly when the migration to
+// Get(UI|IO)ThreadTaskrunner() is complete and the cyclic dependency of
+// browser_task_traits.h on BrowserThread::ID is broken.
+class BrowserTaskTraits;
+
 class BrowserThreadImpl;
 
 // Use DCHECK_CURRENTLY_ON(BrowserThread::ID) to assert that a function can only
@@ -30,34 +35,45 @@ class BrowserThreadImpl;
    << ::content::BrowserThread::GetDCheckCurrentlyOnErrorMessage(   \
           thread_identifier))
 
-///////////////////////////////////////////////////////////////////////////////
-// BrowserThread
+// The main entry point to post tasks to the UI thread. Tasks posted with the
+// same |traits| will run in posting order (i.e. according to the
+// SequencedTaskRunner contract). Tasks posted with different |traits| can be
+// re-ordered. You may keep a reference to this task runner, it's always
+// thread-safe to post to it though it may start returning false at some point
+// during shutdown when it definitely is no longer accepting tasks.
 //
-// Utility functions for threads that are known by a browser-wide name.  For
-// example, there is one IO thread for the entire browser process, and various
-// pieces of code find it useful to retrieve a pointer to the IO thread's
-// message loop.
+// In unit tests, there must be a content::BrowserTaskEnvironment in scope for
+// this API to be available.
 //
-// See browser_task_traits.h for posting Tasks to a BrowserThread.
+// TODO(1026641): Make default traits |{}| the default param when it's possible
+// to include browser_task_traits.h in this file (see note above on the
+// BrowserTaskTraits fwd-decl).
+CONTENT_EXPORT scoped_refptr<base::SingleThreadTaskRunner>
+GetUIThreadTaskRunner(const BrowserTaskTraits& traits);
+
+// The BrowserThread::IO counterpart to GetUIThreadTaskRunner().
+CONTENT_EXPORT scoped_refptr<base::SingleThreadTaskRunner>
+GetIOThreadTaskRunner(const BrowserTaskTraits& traits);
+
+///////////////////////////////////////////////////////////////////////////////
+// BrowserThread
 //
-// This class automatically handles the lifetime of different threads. You
-// should never need to cache pointers to MessageLoops, since they're not thread
-// safe.
+// Utility functions for threads that are known by a browser-wide name.
 class CONTENT_EXPORT BrowserThread {
  public:
   // An enumeration of the well-known threads.
-  // NOTE: threads must be listed in the order of their life-time, with each
-  // thread outliving every other thread below it.
   enum ID {
-    // The main thread in the browser.
+    // The main thread in the browser. It stops running tasks during shutdown
+    // and is never joined.
     UI,
 
-    // This is the thread that processes non-blocking IO, i.e. IPC and network.
-    // Blocking I/O should happen in ThreadPool.
+    // This is the thread that processes non-blocking I/O, i.e. IPC and network.
+    // Blocking I/O should happen in base::ThreadPool. It is joined on shutdown
+    // (and thus any task posted to it may block shutdown).
     IO,
 
     // NOTE: do not add new threads here. Instead you should just use
-    // base::Create*TaskRunner to run tasks on the ThreadPool.
+    // base::ThreadPool::Create*TaskRunner to run tasks on the base::ThreadPool.
 
     // This identifier does not represent a thread.  Instead it counts the
     // number of well-known threads.  Insert new well-known threads before this
@@ -65,15 +81,12 @@ class CONTENT_EXPORT BrowserThread {
     ID_COUNT
   };
 
-  // NOTE: Task posting APIs have moved to post_task.h. See
-  // browser_task_traits.h.
-
   // Delete/ReleaseSoon() helpers allow future deletion of an owned object on
   // its associated thread. If you already have a task runner bound to a
   // BrowserThread you should use its SequencedTaskRunner::DeleteSoon() member
-  // method. If you don't, the helpers below avoid having to do
-  // base::CreateSingleThreadTaskRunner({BrowserThread::ID})->DeleteSoon(...)
-  // which is equivalent.
+  // method.
+  // TODO(1026641): Get rid of the last few callers to these in favor of an
+  // explicit call to content::GetUIThreadTaskRunner({})->DeleteSoon(...).
 
   template <class T>
   static bool DeleteSoon(ID identifier,
@@ -98,7 +111,7 @@ class CONTENT_EXPORT BrowserThread {
   }
 
   // Posts a |task| to run at BEST_EFFORT priority using an arbitrary
-  // |task_runner| for which we do not control the priority
+  // |task_runner| for which we do not control the priority.
   //
   // This is useful when a task needs to run on |task_runner| (for thread-safety
   // reasons) but should be delayed until after critical phases (e.g. startup).
@@ -196,8 +209,8 @@ class CONTENT_EXPORT BrowserThread {
 
  private:
   friend class BrowserThreadImpl;
+  BrowserThread() = default;
 
-  BrowserThread() {}
   DISALLOW_COPY_AND_ASSIGN(BrowserThread);
 };
 

docs/threading_and_tasks.md

@@ -109,7 +109,8 @@ Every Chrome process has
    * in the browser process (BrowserThread::UI): updates the UI
    * in renderer processes (Blink main thread): runs most of Blink
 * an IO thread
-   * in the browser process (BrowserThread::IO): handles IPCs and network requests
+   * in the browser process (BrowserThread::IO): handles IPCs and network
+     requests
    * in renderer processes: handles IPCs
 * a few more special-purpose threads
 * and a pool of general-purpose threads
@@ -222,8 +223,8 @@ class A {
 ```
 
 Unless a test needs to control precisely how tasks are executed, it is preferred
-to call `base::ThreadPool::PostTask*()` directly (ref. [Testing](#Testing) for less invasive
-ways of controlling tasks in tests).
+to call `base::ThreadPool::PostTask*()` directly (ref. [Testing](#Testing) for
+less invasive ways of controlling tasks in tests).
 
 ## Posting a Sequenced Task
 
@@ -336,10 +337,17 @@ posting order.
 ### Posting to the Main Thread or to the IO Thread in the Browser Process
 
 To post tasks to the main thread or to the IO thread, use
-`base::PostTask()` or get the appropriate SingleThreadTaskRunner using
-`base::CreateSingleThreadTaskRunner`, supplying a `BrowserThread::ID`
-as trait. For this, you'll also need to include
-[`content/public/browser/browser_task_traits.h`](https://cs.chromium.org/chromium/src/content/public/browser/browser_task_traits.h).
+`content::GetUIThreadTaskRunner({})` or `content::GetUIThreadTaskRunner({})`
+from
+[`content/public/browser/browser_thread.h`](https://cs.chromium.org/chromium/src/content/public/browser/browser_thread.h)
+
+You may provide additional BrowserTaskTraits as a parameter to those methods
+though this is generally still uncommon in BrowserThreads and should be reserved
+for advanced use cases.
+
+There's an ongoing migration ([task APIs v3]) away from the previous
+base-API-with-traits which you may still find throughout the codebase (it's
+equivalent):
 
 ```cpp
 base::PostTask(FROM_HERE, {content::BrowserThread::UI}, ...);
@@ -348,8 +356,10 @@ base::CreateSingleThreadTaskRunner({content::BrowserThread::IO})
     ->PostTask(FROM_HERE, ...);
 ```
 
-Note: This API will soon be updated to follow the API-as-a-destination design
-for [task APIs v3], stay tuned!
+Note: For the duration of the migration, you'll unfortunately need to continue
+manually including
+[`content/public/browser/browser_task_traits.h`](https://cs.chromium.org/chromium/src/content/public/browser/browser_task_traits.h).
+to use the browser_thread.h API.
 
 The main thread and the IO thread are already super busy. Therefore, prefer
 posting to a general purpose thread when possible (ref.
@@ -362,13 +372,14 @@ Note: It is not necessary to have an explicit post task to the IO thread to
 send/receive an IPC or send/receive data on the network.
 
 ### Posting to the Main Thread in a Renderer Process
-TODO
+TODO(blink-dev)
 
 ### Posting to a Custom SingleThreadTaskRunner
 
 If multiple tasks need to run on the same thread and that thread doesn’t have to
 be the main thread or the IO thread, post them to a
-`base::SingleThreadTaskRunner` created by `base::Threadpool::CreateSingleThreadTaskRunner`.
+`base::SingleThreadTaskRunner` created by
+`base::Threadpool::CreateSingleThreadTaskRunner`.
 
 ```cpp
 scoped_refptr<SingleThreadTaskRunner> single_thread_task_runner =
@@ -387,13 +398,14 @@ be necessary.
 
 *** note
 **IMPORTANT:** To post a task that needs mutual exclusion with the current
-sequence of tasks but doesn’t absolutely need to run on the current physical thread,
-use `base::SequencedTaskRunnerHandle::Get()` instead of
+sequence of tasks but doesn’t absolutely need to run on the current physical
+thread, use `base::SequencedTaskRunnerHandle::Get()` instead of
 `base::ThreadTaskRunnerHandle::Get()` (ref. [Posting to the Current
-Sequence](#Posting-to-the-Current-Virtual_Thread)). That will better document the
-requirements of the posted task and will avoid unnecessarily making your API
-physical thread-affine. In a single-thread task, `base::SequencedTaskRunnerHandle::Get()`
-is equivalent to `base::ThreadTaskRunnerHandle::Get()`.
+Sequence](#Posting-to-the-Current-Virtual_Thread)). That will better document
+the requirements of the posted task and will avoid unnecessarily making your API
+physical thread-affine. In a single-thread task,
+`base::SequencedTaskRunnerHandle::Get()` is equivalent to
+`base::ThreadTaskRunnerHandle::Get()`.
 ***
 
 If you must post a task to the current physical thread nonetheless, use
@@ -481,11 +493,6 @@ base::ThreadPool::PostTask(
 base::ThreadPool::PostTask(
     FROM_HERE, {base::TaskShutdownBehavior::BLOCK_SHUTDOWN},
     base::BindOnce(...));
-
-// This task will run on the Browser UI thread.
-base::ThreadPool::PostTask(
-    FROM_HERE, {content::BrowserThread::UI},
-    base::BindOnce(...));
 ```
 
 ## Keeping the Browser Responsive