base: Add a ThreadPool task trait

This patch introduces a ThreadPool task trait for marking tasks that
should run on the thread pool as opposed to a named browser thread such
as the main thread. Previously any tasks that did *not* indicate a named
thread would automatically run on the thread pool, which was convenient
but perhaps surprising, since these types of tasks can run concurrently
which may not be expected.

After a future patch, tasks will need to explicitly opt into either
thread pool or named thread execution; there is no automatic default.

Change-Id: I2d9cfcd8779b4af5a6ad7baece52bc6acbb7b737
Bug: 968047
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632471
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664413}

base/android/java/src/org/chromium/base/task/PostTask.java

@@ -86,8 +86,8 @@ public class PostTask {
                 getTaskExecutorForTraits(taskTraits).postDelayedTask(taskTraits, task, delay);
             } else {
                 nativePostDelayedTask(taskTraits.mPrioritySetExplicitly, taskTraits.mPriority,
-                        taskTraits.mMayBlock, taskTraits.mExtensionId, taskTraits.mExtensionData,
-                        task, delay);
+                        taskTraits.mMayBlock, taskTraits.mUseThreadPool, taskTraits.mExtensionId,
+                        taskTraits.mExtensionData, task, delay);
             }
         }
     }
@@ -256,5 +256,6 @@ public class PostTask {
     }
 
     private static native void nativePostDelayedTask(boolean prioritySetExplicitly, int priority,
-            boolean mayBlock, byte extensionId, byte[] extensionData, Runnable task, long delay);
+            boolean mayBlock, boolean useThreadPool, byte extensionId, byte[] extensionData,
+            Runnable task, long delay);
 }

base/android/java/src/org/chromium/base/task/TaskRunnerImpl.java

@@ -158,7 +158,8 @@ public class TaskRunnerImpl implements TaskRunner {
         if (mNativeTaskRunnerAndroid == 0) {
             mNativeTaskRunnerAndroid = nativeInit(mTaskRunnerType,
                     mTaskTraits.mPrioritySetExplicitly, mTaskTraits.mPriority,
-                    mTaskTraits.mMayBlock, mTaskTraits.mExtensionId, mTaskTraits.mExtensionData);
+                    mTaskTraits.mMayBlock, mTaskTraits.mUseThreadPool, mTaskTraits.mExtensionId,
+                    mTaskTraits.mExtensionData);
         }
     }
 
@@ -183,8 +184,8 @@ public class TaskRunnerImpl implements TaskRunner {
 
     // NB due to Proguard obfuscation it's easiest to pass the traits via arguments.
     private native long nativeInit(@TaskRunnerType int taskRunnerType,
-            boolean prioritySetExplicitly, int priority, boolean mayBlock, byte extensionId,
-            byte[] extensionData);
+            boolean prioritySetExplicitly, int priority, boolean mayBlock, boolean useThreadPool,
+            byte extensionId, byte[] extensionData);
     private native void nativeDestroy(long nativeTaskRunnerAndroid);
     private native void nativePostDelayedTask(
             long nativeTaskRunnerAndroid, Runnable task, long delay);

base/android/java/src/org/chromium/base/task/TaskTraits.java

@@ -63,11 +63,25 @@ public class TaskTraits {
         CHOREOGRAPHER_FRAME.mIsChoreographerFrame = true;
     }
 
+    // For tasks that should run on the thread pool instead of the main thread.
+    // Note that currently also tasks which lack this trait will execute on the
+    // thread pool unless a trait for a named thread is given.
+    // TODO(skyostil@): Make it required to state the thread affinity for all
+    // tasks.
+    public static final TaskTraits THREAD_POOL = new TaskTraits().threadPool();
+    public static final TaskTraits THREAD_POOL_USER_BLOCKING =
+            THREAD_POOL.taskPriority(TaskPriority.USER_BLOCKING);
+    public static final TaskTraits THREAD_POOL_USER_VISIBLE =
+            THREAD_POOL.taskPriority(TaskPriority.USER_VISIBLE);
+    public static final TaskTraits THREAD_POOL_BEST_EFFORT =
+            THREAD_POOL.taskPriority(TaskPriority.BEST_EFFORT);
+
     // For convenience of the JNI code, we use primitive types only.
     // Note shutdown behavior is not supported on android.
     boolean mPrioritySetExplicitly;
     int mPriority;
     boolean mMayBlock;
+    boolean mUseThreadPool;
     byte mExtensionId;
     byte mExtensionData[];
     boolean mIsChoreographerFrame;
@@ -81,6 +95,7 @@ public class TaskTraits {
         mPrioritySetExplicitly = other.mPrioritySetExplicitly;
         mPriority = other.mPriority;
         mMayBlock = other.mMayBlock;
+        mUseThreadPool = other.mUseThreadPool;
         mExtensionId = other.mExtensionId;
         mExtensionData = other.mExtensionData;
     }
@@ -105,6 +120,12 @@ public class TaskTraits {
         return taskTraits;
     }
 
+    public TaskTraits threadPool() {
+        TaskTraits taskTraits = new TaskTraits(this);
+        taskTraits.mUseThreadPool = true;
+        return taskTraits;
+    }
+
     /**
      * @return true if this task is using some TaskTraits extension.
      */
@@ -158,6 +179,7 @@ public class TaskTraits {
         hash = 37 * hash + (mPrioritySetExplicitly ? 0 : 1);
         hash = 37 * hash + mPriority;
         hash = 37 * hash + (mMayBlock ? 0 : 1);
+        hash = 37 * hash + (mUseThreadPool ? 0 : 1);
         hash = 37 * hash + (int) mExtensionId;
         hash = 37 * hash + Arrays.hashCode(mExtensionData);
         hash = 37 * hash + (mIsChoreographerFrame ? 0 : 1);

base/android/task_scheduler/post_task_android.cc

@@ -48,13 +48,14 @@ TaskTraits PostTaskAndroid::CreateTaskTraits(
     jboolean priority_set_explicitly,
     jint priority,
     jboolean may_block,
+    jboolean use_thread_pool,
     jbyte extension_id,
     const base::android::JavaParamRef<jbyteArray>& extension_data) {
   return TaskTraits(priority_set_explicitly,
                     static_cast<TaskPriority>(priority),
                     /* shutdown_behavior_set_explicitly */ false,
                     TaskShutdownBehavior::SKIP_ON_SHUTDOWN, may_block,
-                    /* with_base_sync_primitives */ false,
+                    /* with_base_sync_primitives */ false, use_thread_pool,
                     TaskTraitsExtensionStorage(
                         extension_id, GetExtensionData(env, extension_data)));
 }
@@ -64,6 +65,7 @@ void JNI_PostTask_PostDelayedTask(
     jboolean priority_set_explicitly,
     jint priority,
     jboolean may_block,
+    jboolean use_thread_pool,
     jbyte extension_id,
     const base::android::JavaParamRef<jbyteArray>& extension_data,
     const base::android::JavaParamRef<jobject>& task,
@@ -73,8 +75,8 @@ void JNI_PostTask_PostDelayedTask(
   PostDelayedTaskWithTraits(
       FROM_HERE,
       PostTaskAndroid::CreateTaskTraits(env, priority_set_explicitly, priority,
-                                        may_block, extension_id,
-                                        extension_data),
+                                        may_block, use_thread_pool,
+                                        extension_id, extension_data),
       BindOnce(&PostTaskAndroid::RunJavaTask,
                base::android::ScopedJavaGlobalRef<jobject>(task)),
       TimeDelta::FromMilliseconds(delay));

base/android/task_scheduler/post_task_android.h

@@ -27,6 +27,7 @@ class BASE_EXPORT PostTaskAndroid {
       jboolean priority_set_explicitly,
       jint priority,
       jboolean may_block,
+      jboolean use_thread_pool,
       jbyte extension_id,
       const base::android::JavaParamRef<jbyteArray>& extension_data);
 

base/android/task_scheduler/task_runner_android.cc

@@ -20,11 +20,12 @@ jlong JNI_TaskRunnerImpl_Init(
     jboolean priority_set_explicitly,
     jint priority,
     jboolean may_block,
+    jboolean thread_pool,
     jbyte extension_id,
     const base::android::JavaParamRef<jbyteArray>& extension_data) {
   TaskTraits task_traits = PostTaskAndroid::CreateTaskTraits(
-      env, priority_set_explicitly, priority, may_block, extension_id,
-      extension_data);
+      env, priority_set_explicitly, priority, may_block, thread_pool,
+      extension_id, extension_data);
   scoped_refptr<TaskRunner> task_runner;
   switch (static_cast<TaskRunnerType>(task_runner_type)) {
     case TaskRunnerType::BASE:

base/task/post_task.cc

@@ -46,9 +46,10 @@ TaskExecutor* GetTaskExecutorForTraits(const TaskTraits& traits) {
       << "Ref. Prerequisite section of post_task.h.\n\n"
          "Hint: if this is in a unit test, you're likely merely missing a "
          "base::test::ScopedTaskEnvironment member in your fixture.\n";
-  return executor ? executor
-                  : static_cast<internal::ThreadPoolImpl*>(
-                        ThreadPoolInstance::Get());
+  // TODO(skyostil): Make thread affinity a required trait.
+  if (!executor || traits.use_thread_pool())
+    return static_cast<internal::ThreadPoolImpl*>(ThreadPoolInstance::Get());
+  return executor;
 }
 
 }  // namespace

base/task/post_task_unittest.cc

@@ -106,6 +106,9 @@ TEST_F(PostTaskTestWithExecutor, PostTaskToThreadPool) {
   EXPECT_TRUE(PostTaskWithTraits(FROM_HERE, {MayBlock()}, DoNothing()));
   EXPECT_FALSE(executor_.runner()->HasPendingTask());
 
+  EXPECT_TRUE(PostTaskWithTraits(FROM_HERE, {ThreadPool()}, DoNothing()));
+  EXPECT_FALSE(executor_.runner()->HasPendingTask());
+
   // Task runners without extension should not be the executor's.
   auto task_runner = CreateTaskRunnerWithTraits({});
   EXPECT_NE(executor_.runner(), task_runner);
@@ -117,6 +120,19 @@ TEST_F(PostTaskTestWithExecutor, PostTaskToThreadPool) {
   auto comsta_task_runner = CreateCOMSTATaskRunnerWithTraits({});
   EXPECT_NE(executor_.runner(), comsta_task_runner);
 #endif  // defined(OS_WIN)
+
+  // Thread pool task runners should not be the executor's.
+  task_runner = CreateTaskRunnerWithTraits({ThreadPool()});
+  EXPECT_NE(executor_.runner(), task_runner);
+  sequenced_task_runner = CreateSequencedTaskRunnerWithTraits({ThreadPool()});
+  EXPECT_NE(executor_.runner(), sequenced_task_runner);
+  single_thread_task_runner =
+      CreateSingleThreadTaskRunnerWithTraits({ThreadPool()});
+  EXPECT_NE(executor_.runner(), single_thread_task_runner);
+#if defined(OS_WIN)
+  comsta_task_runner = CreateCOMSTATaskRunnerWithTraits({ThreadPool()});
+  EXPECT_NE(executor_.runner(), comsta_task_runner);
+#endif  // defined(OS_WIN)
 }
 
 TEST_F(PostTaskTestWithExecutor, PostTaskToTaskExecutor) {

base/task/task_traits.h

@@ -147,6 +147,11 @@ 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:
@@ -157,6 +162,7 @@ class BASE_EXPORT TaskTraits {
     ValidTrait(ThreadPolicy);
     ValidTrait(MayBlock);
     ValidTrait(WithBaseSyncPrimitives);
+    ValidTrait(ThreadPool);
   };
 
   // Invoking this constructor without arguments produces TaskTraits that are
@@ -204,14 +210,15 @@ class BASE_EXPORT TaskTraits {
             trait_helpers::HasTrait<TaskShutdownBehavior>(args...)),
         may_block_(trait_helpers::HasTrait<MayBlock>(args...)),
         with_base_sync_primitives_(
-            trait_helpers::HasTrait<WithBaseSyncPrimitives>(args...)) {}
+            trait_helpers::HasTrait<WithBaseSyncPrimitives>(args...)),
+        use_thread_pool_(trait_helpers::HasTrait<ThreadPool>(args...)) {}
 
   constexpr TaskTraits(const TaskTraits& other) = default;
   TaskTraits& operator=(const TaskTraits& other) = default;
 
   // TODO(eseckler): Default the comparison operator once C++20 arrives.
   bool operator==(const TaskTraits& other) const {
-    static_assert(sizeof(TaskTraits) == 16,
+    static_assert(sizeof(TaskTraits) == 17,
                   "Update comparison operator when TaskTraits change");
     return extension_ == other.extension_ && priority_ == other.priority_ &&
            shutdown_behavior_ == other.shutdown_behavior_ &&
@@ -220,7 +227,8 @@ class BASE_EXPORT TaskTraits {
            shutdown_behavior_set_explicitly_ ==
                other.shutdown_behavior_set_explicitly_ &&
            may_block_ == other.may_block_ &&
-           with_base_sync_primitives_ == other.with_base_sync_primitives_;
+           with_base_sync_primitives_ == other.with_base_sync_primitives_ &&
+           use_thread_pool_ == other.use_thread_pool_;
   }
 
   // Sets the priority of tasks with these traits to |priority|.
@@ -265,6 +273,9 @@ class BASE_EXPORT TaskTraits {
     return with_base_sync_primitives_;
   }
 
+  // Returns true if tasks with these traits execute on the thread pool.
+  constexpr bool use_thread_pool() const { return use_thread_pool_; }
+
   uint8_t extension_id() const { return extension_.extension_id; }
 
   // Access the extension data by parsing it into the provided extension type.
@@ -285,6 +296,7 @@ class BASE_EXPORT TaskTraits {
              TaskShutdownBehavior shutdown_behavior,
              bool may_block,
              bool with_base_sync_primitives,
+             bool use_thread_pool,
              TaskTraitsExtensionStorage extension)
       : extension_(extension),
         priority_(priority),
@@ -293,8 +305,9 @@ class BASE_EXPORT TaskTraits {
         priority_set_explicitly_(priority_set_explicitly),
         shutdown_behavior_set_explicitly_(shutdown_behavior_set_explicitly),
         may_block_(may_block),
-        with_base_sync_primitives_(with_base_sync_primitives) {
-    static_assert(sizeof(TaskTraits) == 16, "Keep this constructor up to date");
+        with_base_sync_primitives_(with_base_sync_primitives),
+        use_thread_pool_(use_thread_pool) {
+    static_assert(sizeof(TaskTraits) == 17, "Keep this constructor up to date");
   }
 
   // Ordered for packing.
@@ -306,6 +319,7 @@ class BASE_EXPORT TaskTraits {
   bool shutdown_behavior_set_explicitly_;
   bool may_block_;
   bool with_base_sync_primitives_;
+  bool use_thread_pool_;
 };
 
 // Returns string literals for the enums defined in this file. These methods