[base/task] Introduce base::ThreadPool:: for Task APIs v3.

Design doc : https://docs.google.com/document/d/1tssusPykvx3g0gvbvU4HxGyn3MjJlIylnsH13-Tv6s4/edit

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

Bug: 1026641
Change-Id: I11c0633779cd2cfe75e29d3318b953c86e32bbec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1977964
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735609}

base/BUILD.gn

@@ -640,6 +640,8 @@ jumbo_component("base") {
     "task/task_traits.cc",
     "task/task_traits.h",
     "task/task_traits_extension.h",
+    "task/thread_pool.cc",
+    "task/thread_pool.h",
     "task/thread_pool/delayed_task_manager.cc",
     "task/thread_pool/delayed_task_manager.h",
     "task/thread_pool/environment_config.cc",

base/task/post_task.h

@@ -28,25 +28,47 @@ namespace base {
 
 // This is the interface to post tasks.
 //
+// Note: A migration is in-progress away from this API and in favor of explicit
+// API-as-a-destination. thread_pool.h is now preferred to the
+// base::ThreadPool() to post to the thread pool
+//
 // To post a simple one-off task with default traits:
 //     PostTask(FROM_HERE, BindOnce(...));
+// modern equivalent:
+//     ThreadPool::PostTask(FROM_HERE, BindOnce(...));
 //
 // To post a high priority one-off task to respond to a user interaction:
 //     PostTask(
 //         FROM_HERE,
 //         {ThreadPool(), TaskPriority::USER_BLOCKING},
 //         BindOnce(...));
+// modern equivalent:
+//     ThreadPool::PostTask(
+//         FROM_HERE,
+//         {TaskPriority::USER_BLOCKING},
+//         BindOnce(...));
 //
 // To post tasks that must run in sequence with default traits:
 //     scoped_refptr<SequencedTaskRunner> task_runner =
 //         CreateSequencedTaskRunner({ThreadPool()});
 //     task_runner->PostTask(FROM_HERE, BindOnce(...));
 //     task_runner->PostTask(FROM_HERE, BindOnce(...));
+// modern equivalent:
+//     scoped_refptr<SequencedTaskRunner> task_runner =
+//         ThreadPool::CreateSequencedTaskRunner({});
+//     task_runner->PostTask(FROM_HERE, BindOnce(...));
+//     task_runner->PostTask(FROM_HERE, BindOnce(...));
 //
 // To post tasks that may block, must run in sequence and can be skipped on
 // shutdown:
 //     scoped_refptr<SequencedTaskRunner> task_runner =
-//         CreateSequencedTaskRunner(
+//         CreateSequencedTaskRunner({ThreadPool(), MayBlock(),
+//                                   TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
+//     task_runner->PostTask(FROM_HERE, BindOnce(...));
+//     task_runner->PostTask(FROM_HERE, BindOnce(...));
+// modern equivalent:
+//     scoped_refptr<SequencedTaskRunner> task_runner =
+//         ThreadPool::CreateSequencedTaskRunner(
 //             {MayBlock(), TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
 //     task_runner->PostTask(FROM_HERE, BindOnce(...));
 //     task_runner->PostTask(FROM_HERE, BindOnce(...));
@@ -62,8 +84,7 @@ namespace base {
 // Tasks posted with only traits defined in base/task/task_traits.h run on
 // threads owned by the registered ThreadPoolInstance (i.e. not on the main
 // thread). An embedder (e.g. Chrome) can define additional traits to make tasks
-// run on threads of their choosing. TODO(https://crbug.com/863341): Make this a
-// reality.
+// run on threads of their choosing.
 //
 // Tasks posted with the same traits will be scheduled in the order they were
 // posted. IMPORTANT: Please note however that, unless the traits imply a
@@ -171,6 +192,8 @@ BASE_EXPORT scoped_refptr<SequencedTaskRunner> CreateSequencedTaskRunner(
 //
 // |traits| requirements:
 // - base::ThreadPool() must be specified.
+//     Note: Prefer the explicit (thread_pool.h) version of this API while we
+//     migrate this one to it.
 // - Extension traits (e.g. BrowserThread) cannot be specified.
 // - base::ThreadPolicy must be specified if the priority of the task runner
 //   will ever be increased from BEST_EFFORT.

base/task/task_traits.h

@@ -18,6 +18,11 @@
 #include "base/traits_bag.h"
 #include "build/build_config.h"
 
+// TODO(gab): This is backwards, thread_pool.h should include task_traits.h
+// but it this is necessary to have it in this direction during the migration
+// from old code that used base::ThreadPool as a trait.
+#include "base/task/thread_pool.h"
+
 namespace base {
 
 class PostTaskAndroid;
@@ -180,11 +185,6 @@ struct MayBlock {};
 // In doubt, consult with //base/task/OWNERS.
 struct WithBaseSyncPrimitives {};
 
-// Tasks and task runners with this trait will run in the thread pool,
-// concurrently with tasks on other task runners. If you need mutual exclusion
-// between tasks, see base::PostTask::CreateSequencedTaskRunner.
-struct ThreadPool {};
-
 // Describes metadata for a single task or a group of tasks.
 class BASE_EXPORT TaskTraits {
  public:
@@ -212,14 +212,14 @@ class BASE_EXPORT TaskTraits {
   // WithBaseSyncPrimitives in any order to the constructor.
   //
   // E.g.
-  // constexpr base::TaskTraits default_traits = {base::ThreadPool()};
+  // constexpr base::TaskTraits default_traits = {};
   // constexpr base::TaskTraits user_visible_traits = {
-  //     base::ThreadPool(), base::TaskPriority::USER_VISIBLE};
+  //     base::TaskPriority::USER_VISIBLE};
   // constexpr base::TaskTraits user_visible_may_block_traits = {
-  //     base::ThreadPool(), base::TaskPriority::USER_VISIBLE, base::MayBlock()
+  //     base::TaskPriority::USER_VISIBLE, base::MayBlock()
   // };
   // constexpr base::TaskTraits other_user_visible_may_block_traits = {
-  //     base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_VISIBLE
+  //     base::MayBlock(), base::TaskPriority::USER_VISIBLE
   // };
   template <class... ArgTypes,
             class CheckArgumentsAreValid = std::enable_if_t<

base/task/task_traits_extension_unittest.cc

@@ -10,12 +10,20 @@
 namespace base {
 
 TEST(TaskTraitsExtensionTest, NoExtension) {
-  constexpr TaskTraits traits = {ThreadPool()};
+  constexpr TaskTraits traits = {};
 
   EXPECT_EQ(traits.extension_id(),
             TaskTraitsExtensionStorage::kInvalidExtensionId);
 }
 
+TEST(TaskTraitsExtensionTest, ThreadPoolIsntAnExtension) {
+  constexpr TaskTraits traits = {base::ThreadPool()};
+
+  EXPECT_TRUE(traits.use_thread_pool());
+  EXPECT_EQ(traits.extension_id(),
+            TaskTraitsExtensionStorage::kInvalidExtensionId);
+}
+
 TEST(TaskTraitsExtensionTest, CreateWithOneExtensionTrait) {
   constexpr TaskTraits traits = {TestExtensionEnumTrait::kB};
 

base/task/task_traits_unittest.cc

@@ -9,7 +9,7 @@
 namespace base {
 
 TEST(TaskTraitsTest, Default) {
-  constexpr TaskTraits traits = {ThreadPool()};
+  constexpr TaskTraits traits = {};
   EXPECT_FALSE(traits.priority_set_explicitly());
   EXPECT_EQ(TaskPriority::USER_BLOCKING, traits.priority());
   EXPECT_FALSE(traits.shutdown_behavior_set_explicitly());
@@ -21,7 +21,7 @@ TEST(TaskTraitsTest, Default) {
 }
 
 TEST(TaskTraitsTest, TaskPriority) {
-  constexpr TaskTraits traits = {ThreadPool(), TaskPriority::BEST_EFFORT};
+  constexpr TaskTraits traits = {TaskPriority::BEST_EFFORT};
   EXPECT_TRUE(traits.priority_set_explicitly());
   EXPECT_EQ(TaskPriority::BEST_EFFORT, traits.priority());
   EXPECT_FALSE(traits.shutdown_behavior_set_explicitly());
@@ -46,8 +46,7 @@ TEST(TaskTraitsTest, TaskShutdownBehavior) {
 }
 
 TEST(TaskTraitsTest, ThreadPolicy) {
-  constexpr TaskTraits traits = {ThreadPool(),
-                                 ThreadPolicy::MUST_USE_FOREGROUND};
+  constexpr TaskTraits traits = {ThreadPolicy::MUST_USE_FOREGROUND};
   EXPECT_FALSE(traits.priority_set_explicitly());
   EXPECT_EQ(TaskPriority::USER_BLOCKING, traits.priority());
   EXPECT_FALSE(traits.shutdown_behavior_set_explicitly());
@@ -59,7 +58,7 @@ TEST(TaskTraitsTest, ThreadPolicy) {
 }
 
 TEST(TaskTraitsTest, MayBlock) {
-  constexpr TaskTraits traits = {ThreadPool(), MayBlock()};
+  constexpr TaskTraits traits = {MayBlock()};
   EXPECT_FALSE(traits.priority_set_explicitly());
   EXPECT_EQ(TaskPriority::USER_BLOCKING, traits.priority());
   EXPECT_FALSE(traits.shutdown_behavior_set_explicitly());
@@ -71,7 +70,7 @@ TEST(TaskTraitsTest, MayBlock) {
 }
 
 TEST(TaskTraitsTest, WithBaseSyncPrimitives) {
-  constexpr TaskTraits traits = {ThreadPool(), WithBaseSyncPrimitives()};
+  constexpr TaskTraits traits = {WithBaseSyncPrimitives()};
   EXPECT_FALSE(traits.priority_set_explicitly());
   EXPECT_EQ(TaskPriority::USER_BLOCKING, traits.priority());
   EXPECT_FALSE(traits.shutdown_behavior_set_explicitly());
@@ -84,7 +83,7 @@ TEST(TaskTraitsTest, WithBaseSyncPrimitives) {
 
 TEST(TaskTraitsTest, UpdatePriority) {
   {
-    TaskTraits traits = {ThreadPool()};
+    TaskTraits traits = {};
     EXPECT_FALSE(traits.priority_set_explicitly());
     traits.UpdatePriority(TaskPriority::BEST_EFFORT);
     EXPECT_EQ(TaskPriority::BEST_EFFORT, traits.priority());
@@ -92,7 +91,7 @@ TEST(TaskTraitsTest, UpdatePriority) {
   }
 
   {
-    TaskTraits traits = {ThreadPool(), TaskPriority::USER_VISIBLE};
+    TaskTraits traits = {TaskPriority::USER_VISIBLE};
     EXPECT_TRUE(traits.priority_set_explicitly());
     traits.UpdatePriority(TaskPriority::BEST_EFFORT);
     EXPECT_EQ(TaskPriority::BEST_EFFORT, traits.priority());
@@ -102,14 +101,14 @@ TEST(TaskTraitsTest, UpdatePriority) {
 
 TEST(TaskTraitsTest, InheritPriority) {
   {
-    TaskTraits traits = {ThreadPool()};
+    TaskTraits traits = {};
     traits.InheritPriority(TaskPriority::BEST_EFFORT);
     EXPECT_EQ(TaskPriority::BEST_EFFORT, traits.priority());
     EXPECT_FALSE(traits.priority_set_explicitly());
   }
 
   {
-    TaskTraits traits = {ThreadPool(), TaskPriority::USER_VISIBLE};
+    TaskTraits traits = {TaskPriority::USER_VISIBLE};
     traits.InheritPriority(TaskPriority::BEST_EFFORT);
     EXPECT_EQ(TaskPriority::USER_VISIBLE, traits.priority());
     EXPECT_TRUE(traits.priority_set_explicitly());
@@ -117,12 +116,9 @@ TEST(TaskTraitsTest, InheritPriority) {
 }
 
 TEST(TaskTraitsTest, MultipleTraits) {
-  constexpr TaskTraits traits = {ThreadPool(),
-                                 TaskPriority::BEST_EFFORT,
-                                 TaskShutdownBehavior::BLOCK_SHUTDOWN,
-                                 ThreadPolicy::MUST_USE_FOREGROUND,
-                                 MayBlock(),
-                                 WithBaseSyncPrimitives()};
+  constexpr TaskTraits traits = {
+      TaskPriority::BEST_EFFORT, TaskShutdownBehavior::BLOCK_SHUTDOWN,
+      ThreadPolicy::MUST_USE_FOREGROUND, MayBlock(), WithBaseSyncPrimitives()};
   EXPECT_TRUE(traits.priority_set_explicitly());
   EXPECT_EQ(TaskPriority::BEST_EFFORT, traits.priority());
   EXPECT_TRUE(traits.shutdown_behavior_set_explicitly());
@@ -134,12 +130,9 @@ TEST(TaskTraitsTest, MultipleTraits) {
 }
 
 TEST(TaskTraitsTest, Copy) {
-  constexpr TaskTraits traits = {ThreadPool(),
-                                 TaskPriority::BEST_EFFORT,
-                                 TaskShutdownBehavior::BLOCK_SHUTDOWN,
-                                 ThreadPolicy::MUST_USE_FOREGROUND,
-                                 MayBlock(),
-                                 WithBaseSyncPrimitives()};
+  constexpr TaskTraits traits = {
+      TaskPriority::BEST_EFFORT, TaskShutdownBehavior::BLOCK_SHUTDOWN,
+      ThreadPolicy::MUST_USE_FOREGROUND, MayBlock(), WithBaseSyncPrimitives()};
   constexpr TaskTraits traits_copy(traits);
 
   EXPECT_EQ(traits, traits_copy);

base/task/thread_pool.cc

+// Copyright 2019 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/task/thread_pool.h"
+
+#include "base/logging.h"
+#include "base/task/scoped_set_task_priority_for_current_thread.h"
+#include "base/task/task_traits.h"
+#include "base/task/thread_pool/thread_pool_impl.h"
+#include "base/task/thread_pool/thread_pool_instance.h"
+#include "base/threading/post_task_and_reply_impl.h"
+
+namespace base {
+
+namespace {
+
+class PostTaskAndReplyWithTraitsTaskRunner
+    : public internal::PostTaskAndReplyImpl {
+ public:
+  explicit PostTaskAndReplyWithTraitsTaskRunner(const TaskTraits& traits)
+      : traits_(traits) {}
+
+ private:
+  bool PostTask(const Location& from_here, OnceClosure task) override {
+    ThreadPool::PostTask(from_here, traits_, std::move(task));
+    return true;
+  }
+
+  const TaskTraits traits_;
+};
+
+// Returns TaskTraits based on |traits|. If TaskPriority hasn't been set
+// explicitly in |traits|, the returned TaskTraits will inherit the current
+// TaskPriority.
+TaskTraits GetTaskTraitsWithExplicitPriority(TaskTraits traits) {
+  traits.InheritPriority(internal::GetTaskPriorityForCurrentThread());
+  return traits;
+}
+
+internal::ThreadPoolImpl* GetThreadPoolImpl() {
+  auto* instance = ThreadPoolInstance::Get();
+  DCHECK(instance)
+      << "Ref. Prerequisite section of base/task/thread_pool.h.\n"
+         "Hint: if this is in a unit test, you're likely merely missing a "
+         "base::test::TaskEnvironment member in your fixture (or your fixture "
+         "is using a base::test::SingleThreadTaskEnvironment and now needs a "
+         "full base::test::TaskEnvironment).\n";
+  return static_cast<internal::ThreadPoolImpl*>(instance);
+}
+
+}  // namespace
+
+// static
+bool ThreadPool::PostTask(const Location& from_here, OnceClosure task) {
+  return ThreadPool::PostDelayedTask(from_here, std::move(task), TimeDelta());
+}
+
+// static
+bool ThreadPool::PostDelayedTask(const Location& from_here,
+                                 OnceClosure task,
+                                 TimeDelta delay) {
+  return ThreadPool::PostDelayedTask(from_here, {}, std::move(task), delay);
+}
+
+// static
+bool ThreadPool::PostTaskAndReply(const Location& from_here,
+                                  OnceClosure task,
+                                  OnceClosure reply) {
+  return ThreadPool::PostTaskAndReply(from_here, {}, std::move(task),
+                                      std::move(reply));
+}
+
+// static
+bool ThreadPool::PostTask(const Location& from_here,
+                          const TaskTraits& traits,
+                          OnceClosure task) {
+  return ThreadPool::PostDelayedTask(from_here, traits, std::move(task),
+                                     TimeDelta());
+}
+
+// static
+bool ThreadPool::PostDelayedTask(const Location& from_here,
+                                 const TaskTraits& traits,
+                                 OnceClosure task,
+                                 TimeDelta delay) {
+  const TaskTraits adjusted_traits = GetTaskTraitsWithExplicitPriority(traits);
+  return GetThreadPoolImpl()->PostDelayedTask(from_here, adjusted_traits,
+                                              std::move(task), delay);
+}
+
+// static
+bool ThreadPool::PostTaskAndReply(const Location& from_here,
+                                  const TaskTraits& traits,
+                                  OnceClosure task,
+                                  OnceClosure reply) {
+  return PostTaskAndReplyWithTraitsTaskRunner(traits).PostTaskAndReply(
+      from_here, std::move(task), std::move(reply));
+}
+
+// static
+scoped_refptr<TaskRunner> ThreadPool::CreateTaskRunner(
+    const TaskTraits& traits) {
+  return GetThreadPoolImpl()->CreateTaskRunner(traits);
+}
+
+// static
+scoped_refptr<SequencedTaskRunner> ThreadPool::CreateSequencedTaskRunner(
+    const TaskTraits& traits) {
+  return GetThreadPoolImpl()->CreateSequencedTaskRunner(traits);
+}
+
+// static
+scoped_refptr<UpdateableSequencedTaskRunner>
+ThreadPool::CreateUpdateableSequencedTaskRunner(const TaskTraits& traits) {
+  const TaskTraits adjusted_traits = GetTaskTraitsWithExplicitPriority(traits);
+  return GetThreadPoolImpl()->CreateUpdateableSequencedTaskRunner(
+      adjusted_traits);
+}
+
+// static
+scoped_refptr<SingleThreadTaskRunner> ThreadPool::CreateSingleThreadTaskRunner(
+    const TaskTraits& traits,
+    SingleThreadTaskRunnerThreadMode thread_mode) {
+  return GetThreadPoolImpl()->CreateSingleThreadTaskRunner(traits, thread_mode);
+}
+
+#if defined(OS_WIN)
+// static
+scoped_refptr<SingleThreadTaskRunner> ThreadPool::CreateCOMSTATaskRunner(
+    const TaskTraits& traits,
+    SingleThreadTaskRunnerThreadMode thread_mode) {
+  return GetThreadPoolImpl()->CreateCOMSTATaskRunner(traits, thread_mode);
+}
+#endif  // defined(OS_WIN)
+
+}  // namespace base

base/task/thread_pool/thread_pool_impl.cc

@@ -231,7 +231,7 @@ bool ThreadPoolImpl::PostDelayedTask(const Location& from_here,
                                      OnceClosure task,
                                      TimeDelta delay) {
   // Post |task| as part of a one-off single-task Sequence.
-  const TaskTraits new_traits = SetUserBlockingPriorityIfNeeded(traits);
+  const TaskTraits new_traits = VerifyAndAjustIncomingTraits(traits);
   return PostTaskWithSequence(
       Task(from_here, std::move(task), delay),
       MakeRefCounted<Sequence>(new_traits, nullptr,
@@ -240,13 +240,13 @@ bool ThreadPoolImpl::PostDelayedTask(const Location& from_here,
 
 scoped_refptr<TaskRunner> ThreadPoolImpl::CreateTaskRunner(
     const TaskTraits& traits) {
-  const TaskTraits new_traits = SetUserBlockingPriorityIfNeeded(traits);
+  const TaskTraits new_traits = VerifyAndAjustIncomingTraits(traits);
   return MakeRefCounted<PooledParallelTaskRunner>(new_traits, this);
 }
 
 scoped_refptr<SequencedTaskRunner> ThreadPoolImpl::CreateSequencedTaskRunner(
     const TaskTraits& traits) {
-  const TaskTraits new_traits = SetUserBlockingPriorityIfNeeded(traits);
+  const TaskTraits new_traits = VerifyAndAjustIncomingTraits(traits);
   return MakeRefCounted<PooledSequencedTaskRunner>(new_traits, this);
 }
 
@@ -255,7 +255,7 @@ ThreadPoolImpl::CreateSingleThreadTaskRunner(
     const TaskTraits& traits,
     SingleThreadTaskRunnerThreadMode thread_mode) {
   return single_thread_task_runner_manager_.CreateSingleThreadTaskRunner(
-      SetUserBlockingPriorityIfNeeded(traits), thread_mode);
+      VerifyAndAjustIncomingTraits(traits), thread_mode);
 }
 
 #if defined(OS_WIN)
@@ -263,13 +263,13 @@ scoped_refptr<SingleThreadTaskRunner> ThreadPoolImpl::CreateCOMSTATaskRunner(
     const TaskTraits& traits,
     SingleThreadTaskRunnerThreadMode thread_mode) {
   return single_thread_task_runner_manager_.CreateCOMSTATaskRunner(
-      SetUserBlockingPriorityIfNeeded(traits), thread_mode);
+      VerifyAndAjustIncomingTraits(traits), thread_mode);
 }
 #endif  // defined(OS_WIN)
 
 scoped_refptr<UpdateableSequencedTaskRunner>
 ThreadPoolImpl::CreateUpdateableSequencedTaskRunner(const TaskTraits& traits) {
-  const TaskTraits new_traits = SetUserBlockingPriorityIfNeeded(traits);
+  const TaskTraits new_traits = VerifyAndAjustIncomingTraits(traits);
   return MakeRefCounted<PooledSequencedTaskRunner>(new_traits, this);
 }
 
@@ -526,8 +526,11 @@ void ThreadPoolImpl::UpdateCanRunPolicy() {
   single_thread_task_runner_manager_.DidUpdateCanRunPolicy();
 }
 
-TaskTraits ThreadPoolImpl::SetUserBlockingPriorityIfNeeded(
+TaskTraits ThreadPoolImpl::VerifyAndAjustIncomingTraits(
     TaskTraits traits) const {
+  DCHECK_EQ(traits.extension_id(),
+            TaskTraitsExtensionStorage::kInvalidExtensionId)
+      << "Extension traits cannot be used with the ThreadPool API.";
   if (all_tasks_user_blocking_.IsSet())
     traits.UpdatePriority(TaskPriority::USER_BLOCKING);
   return traits;

base/task/thread_pool/thread_pool_impl.h

@@ -124,9 +124,10 @@ class BASE_EXPORT ThreadPoolImpl : public ThreadPoolInstance,
   // the CanRunPolicy in TaskTracker and wakes up workers as appropriate.
   void UpdateCanRunPolicy();
 
-  // Returns |traits|, with priority set to TaskPriority::USER_BLOCKING if
+  // Verifies that |traits| do not have properties that are banned in ThreadPool
+  // and returns |traits|, with priority set to TaskPriority::USER_BLOCKING if
   // |all_tasks_user_blocking_| is set.
-  TaskTraits SetUserBlockingPriorityIfNeeded(TaskTraits traits) const;
+  TaskTraits VerifyAndAjustIncomingTraits(TaskTraits traits) const;
 
   void ReportHeartbeatMetrics() const;