Implement Shared SingleThreadTaskRunners in the Task Scheduler

base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc

Index: base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc
diff --git a/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc b/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc
index 4415f5fd808adbd83124cac4686b062d8549e649..f620683bf58418d728d060f9b27ec7b449cebca2 100644
--- a/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc
+++ b/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc
@@ -14,6 +14,7 @@
 #include "base/task_scheduler/scheduler_worker_pool_params.h"
 #include "base/task_scheduler/task_tracker.h"
 #include "base/task_scheduler/task_traits.h"
+#include "base/test/gtest_util.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/simple_thread.h"
@@ -82,6 +83,10 @@ void CaptureThreadPriority(ThreadPriority* thread_priority) {
   *thread_priority = PlatformThread::GetCurrentThreadPriority();
 }
 
+void CaptureThreadName(std::string* thread_name) {
+  *thread_name = PlatformThread::GetName();
+}
+
 void ShouldNotRun() {
   ADD_FAILURE() << "Ran a task that shouldn't run.";
 }
@@ -92,13 +97,13 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, DifferentThreadsUsed) {
   scoped_refptr<SingleThreadTaskRunner> task_runner_1 =
       single_thread_task_runner_manager_
           ->CreateSingleThreadTaskRunnerWithTraits(
-              "A", ThreadPriority::NORMAL,
-              {TaskShutdownBehavior::BLOCK_SHUTDOWN});
+              "A", {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+              SingleThreadTaskRunnerThreadMode::DEDICATED);
   scoped_refptr<SingleThreadTaskRunner> task_runner_2 =
       single_thread_task_runner_manager_
           ->CreateSingleThreadTaskRunnerWithTraits(
-              "B", ThreadPriority::NORMAL,
-              {TaskShutdownBehavior::BLOCK_SHUTDOWN});
+              "B", {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+              SingleThreadTaskRunnerThreadMode::DEDICATED);
 
   PlatformThreadRef thread_ref_1;
   task_runner_1->PostTask(FROM_HERE,
@@ -114,19 +119,106 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, DifferentThreadsUsed) {
   EXPECT_NE(thread_ref_1, thread_ref_2);
 }
 
-TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PrioritySetCorrectly) {
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, SameThreadUsed) {
+  scoped_refptr<SingleThreadTaskRunner> task_runner_1 =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              "A", {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+              SingleThreadTaskRunnerThreadMode::SHARED);
+  scoped_refptr<SingleThreadTaskRunner> task_runner_2 =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              "B", {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+              SingleThreadTaskRunnerThreadMode::SHARED);
+
+  PlatformThreadRef thread_ref_1;
+  task_runner_1->PostTask(FROM_HERE,
+                          BindOnce(&CaptureThreadRef, &thread_ref_1));
+  PlatformThreadRef thread_ref_2;
+  task_runner_2->PostTask(FROM_HERE,
+                          BindOnce(&CaptureThreadRef, &thread_ref_2));
+
+  task_tracker_.Shutdown();
+
+  ASSERT_FALSE(thread_ref_1.is_null());
+  ASSERT_FALSE(thread_ref_2.is_null());
+  EXPECT_EQ(thread_ref_1, thread_ref_2);
+}
+
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest,
+       RunsTasksOnCurrentThread) {

[gab, 2017/05/25 18:47:00]

This test should work in both modes, parametrize as a
TaskSchedulerSingleThreadTaskRunnerManagerCommonTest as well?

[robliao, 2017/05/30 20:01:07]

Since we do check the thread handle for the COM case,  it's not guaranteed that
this test will succeed when the mode is SHARED. In fact, the EXPECT_FALSEs below
become EXPECT_TRUEs.

+  scoped_refptr<SingleThreadTaskRunner> task_runner_1 =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              "A", {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+              SingleThreadTaskRunnerThreadMode::DEDICATED);
+  scoped_refptr<SingleThreadTaskRunner> task_runner_2 =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              "B", {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+              SingleThreadTaskRunnerThreadMode::DEDICATED);
+
+  EXPECT_FALSE(task_runner_1->RunsTasksOnCurrentThread());
+  EXPECT_FALSE(task_runner_2->RunsTasksOnCurrentThread());
+
+  task_runner_1->PostTask(
+      FROM_HERE, BindOnce(
+                     [](scoped_refptr<SingleThreadTaskRunner> task_runner_1,
+                        scoped_refptr<SingleThreadTaskRunner> task_runner_2) {
+                       EXPECT_TRUE(task_runner_1->RunsTasksOnCurrentThread());
+                       EXPECT_FALSE(task_runner_2->RunsTasksOnCurrentThread());
+                     },
+                     task_runner_1, task_runner_2));
+
+  task_runner_2->PostTask(
+      FROM_HERE, BindOnce(
+                     [](scoped_refptr<SingleThreadTaskRunner> task_runner_1,
+                        scoped_refptr<SingleThreadTaskRunner> task_runner_2) {
+                       EXPECT_FALSE(task_runner_1->RunsTasksOnCurrentThread());
+                       EXPECT_TRUE(task_runner_2->RunsTasksOnCurrentThread());
+                     },
+                     task_runner_1, task_runner_2));
+
+  task_tracker_.Shutdown();
+}
+
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest,
+       SharedWithBaseSyncPrimitivesDCHECKs) {
+  EXPECT_DCHECK_DEATH({
+    single_thread_task_runner_manager_->CreateSingleThreadTaskRunnerWithTraits(
+        "A", {WithBaseSyncPrimitives()},
+        SingleThreadTaskRunnerThreadMode::SHARED);
+  });
+}
+
+namespace {
+
+class TaskSchedulerSingleThreadTaskRunnerManagerCommonTest
+    : public TaskSchedulerSingleThreadTaskRunnerManagerTest,
+      public ::testing::WithParamInterface<SingleThreadTaskRunnerThreadMode> {
+ public:
+  TaskSchedulerSingleThreadTaskRunnerManagerCommonTest() = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(
+      TaskSchedulerSingleThreadTaskRunnerManagerCommonTest);
+};
+
+}  // namespace
+
+TEST_P(TaskSchedulerSingleThreadTaskRunnerManagerCommonTest,
+       PrioritySetCorrectly) {
   // Why are events used here instead of the task tracker?
   // Shutting down can cause priorities to get raised. This means we have to use
   // events to determine when a task is run.
   scoped_refptr<SingleThreadTaskRunner> task_runner_background =
       single_thread_task_runner_manager_
-          ->CreateSingleThreadTaskRunnerWithTraits("Background",
-                                                   ThreadPriority::BACKGROUND,
-                                                   {TaskPriority::BACKGROUND});
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              "Background", {TaskPriority::BACKGROUND}, GetParam());
   scoped_refptr<SingleThreadTaskRunner> task_runner_normal =
       single_thread_task_runner_manager_
           ->CreateSingleThreadTaskRunnerWithTraits(
-              "Normal", ThreadPriority::NORMAL, {TaskPriority::USER_VISIBLE});
+              "Normal", {TaskPriority::USER_VISIBLE}, GetParam());
 
   ThreadPriority thread_priority_background;
   task_runner_background->PostTask(
@@ -158,16 +250,62 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PrioritySetCorrectly) {
   EXPECT_EQ(ThreadPriority::NORMAL, thread_priority_normal);
 }
 
-TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PostTaskAfterShutdown) {
+TEST_P(TaskSchedulerSingleThreadTaskRunnerManagerCommonTest, ThreadNamesSet) {
+  constexpr TaskTraits foo_traits = {TaskPriority::BACKGROUND,
+                                     TaskShutdownBehavior::BLOCK_SHUTDOWN};
+  scoped_refptr<SingleThreadTaskRunner> foo_task_runner =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits("MyName", foo_traits,
+                                                   GetParam());
+  std::string foo_captured_name;
+  foo_task_runner->PostTask(FROM_HERE,
+                            BindOnce(&CaptureThreadName, &foo_captured_name));
+
+  constexpr TaskTraits user_blocking_traits = {
+      TaskPriority::USER_BLOCKING, MayBlock(),
+      TaskShutdownBehavior::BLOCK_SHUTDOWN};
+  scoped_refptr<SingleThreadTaskRunner> user_blocking_task_runner =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits("A", user_blocking_traits,
+                                                   GetParam());
+
+  std::string user_blocking_captured_name;
+  user_blocking_task_runner->PostTask(
+      FROM_HERE, BindOnce(&CaptureThreadName, &user_blocking_captured_name));
+
+  task_tracker_.Shutdown();
+
+  EXPECT_NE(std::string::npos, foo_captured_name.find("MyName"));
+  EXPECT_NE(std::string::npos,
+            foo_captured_name.find(
+                kEnvironmentParams[GetEnvironmentIndexForTraits(foo_traits)]
+                    .name_suffix));
+  EXPECT_NE(
+      std::string::npos,
+      user_blocking_captured_name.find(
+          kEnvironmentParams[GetEnvironmentIndexForTraits(user_blocking_traits)]
+              .name_suffix));
+
+  if (GetParam() == SingleThreadTaskRunnerThreadMode::DEDICATED) {
+    EXPECT_EQ(std::string::npos, foo_captured_name.find("Shared"));
+    EXPECT_EQ(std::string::npos, user_blocking_captured_name.find("Shared"));
+  } else {
+    EXPECT_NE(std::string::npos, foo_captured_name.find("Shared"));
+    EXPECT_NE(std::string::npos, user_blocking_captured_name.find("Shared"));
+  }
+}
+
+TEST_P(TaskSchedulerSingleThreadTaskRunnerManagerCommonTest,
+       PostTaskAfterShutdown) {
   auto task_runner = single_thread_task_runner_manager_
                          ->CreateSingleThreadTaskRunnerWithTraits(
-                             "A", ThreadPriority::NORMAL, TaskTraits());
+                             "A", TaskTraits(), GetParam());
   task_tracker_.Shutdown();
   EXPECT_FALSE(task_runner->PostTask(FROM_HERE, BindOnce(&ShouldNotRun)));
 }
 
 // Verify that a Task runs shortly after its delay expires.
-TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PostDelayedTask) {
+TEST_P(TaskSchedulerSingleThreadTaskRunnerManagerCommonTest, PostDelayedTask) {
   TimeTicks start_time = TimeTicks::Now();
 
   // Post a task with a short delay.
@@ -175,7 +313,7 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PostDelayedTask) {
                          WaitableEvent::InitialState::NOT_SIGNALED);
   auto task_runner = single_thread_task_runner_manager_
                          ->CreateSingleThreadTaskRunnerWithTraits(
-                             "A", ThreadPriority::NORMAL, TaskTraits());
+                             "A", TaskTraits(), GetParam());
   EXPECT_TRUE(task_runner->PostDelayedTask(
       FROM_HERE, BindOnce(&WaitableEvent::Signal, Unretained(&task_ran)),
       TestTimeouts::tiny_timeout()));
@@ -191,42 +329,11 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PostDelayedTask) {
             TimeDelta::FromMilliseconds(250) + TestTimeouts::tiny_timeout());
 }
 
-TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest,
-       RunsTasksOnCurrentThread) {

[gab, 2017/05/25 18:47:01]

Should stay here as a Common test, no?

[robliao, 2017/05/30 20:01:07]

See above.

-  scoped_refptr<SingleThreadTaskRunner> task_runner_1 =
-      single_thread_task_runner_manager_
-          ->CreateSingleThreadTaskRunnerWithTraits(
-              "A", ThreadPriority::NORMAL,
-              {TaskShutdownBehavior::BLOCK_SHUTDOWN});
-  scoped_refptr<SingleThreadTaskRunner> task_runner_2 =
-      single_thread_task_runner_manager_
-          ->CreateSingleThreadTaskRunnerWithTraits(
-              "B", ThreadPriority::NORMAL,
-              {TaskShutdownBehavior::BLOCK_SHUTDOWN});
-
-  EXPECT_FALSE(task_runner_1->RunsTasksOnCurrentThread());
-  EXPECT_FALSE(task_runner_2->RunsTasksOnCurrentThread());
-
-  task_runner_1->PostTask(
-      FROM_HERE, BindOnce(
-                     [](scoped_refptr<SingleThreadTaskRunner> task_runner_1,
-                        scoped_refptr<SingleThreadTaskRunner> task_runner_2) {
-                       EXPECT_TRUE(task_runner_1->RunsTasksOnCurrentThread());
-                       EXPECT_FALSE(task_runner_2->RunsTasksOnCurrentThread());
-                     },
-                     task_runner_1, task_runner_2));
-
-  task_runner_2->PostTask(
-      FROM_HERE, BindOnce(
-                     [](scoped_refptr<SingleThreadTaskRunner> task_runner_1,
-                        scoped_refptr<SingleThreadTaskRunner> task_runner_2) {
-                       EXPECT_FALSE(task_runner_1->RunsTasksOnCurrentThread());
-                       EXPECT_TRUE(task_runner_2->RunsTasksOnCurrentThread());
-                     },
-                     task_runner_1, task_runner_2));
-
-  task_tracker_.Shutdown();
-}
+INSTANTIATE_TEST_CASE_P(
+    AllModes,
+    TaskSchedulerSingleThreadTaskRunnerManagerCommonTest,
+    ::testing::Values(SingleThreadTaskRunnerThreadMode::SHARED,
+                      SingleThreadTaskRunnerThreadMode::DEDICATED));
 
 namespace {
 
@@ -282,10 +389,10 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerJoinTest, ConcurrentJoin) {
                               WaitableEvent::InitialState::NOT_SIGNALED);
 
   {
-    auto task_runner =
-        single_thread_task_runner_manager_
-            ->CreateSingleThreadTaskRunnerWithTraits(
-                "A", ThreadPriority::NORMAL, {WithBaseSyncPrimitives()});
+    auto task_runner = single_thread_task_runner_manager_
+                           ->CreateSingleThreadTaskRunnerWithTraits(
+                               "A", {WithBaseSyncPrimitives()},
+                               SingleThreadTaskRunnerThreadMode::DEDICATED);
     EXPECT_TRUE(task_runner->PostTask(
         FROM_HERE,
         BindOnce(&WaitableEvent::Signal, Unretained(&task_running))));
@@ -312,10 +419,10 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerJoinTest,
                               WaitableEvent::InitialState::NOT_SIGNALED);
 
   {
-    auto task_runner =
-        single_thread_task_runner_manager_
-            ->CreateSingleThreadTaskRunnerWithTraits(
-                "A", ThreadPriority::NORMAL, {WithBaseSyncPrimitives()});
+    auto task_runner = single_thread_task_runner_manager_
+                           ->CreateSingleThreadTaskRunnerWithTraits(
+                               "A", {WithBaseSyncPrimitives()},
+                               SingleThreadTaskRunnerThreadMode::DEDICATED);
     EXPECT_TRUE(task_runner->PostTask(
         FROM_HERE,
         BindOnce(&WaitableEvent::Signal, Unretained(&task_running))));
@@ -335,10 +442,11 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerJoinTest,
 
 #if defined(OS_WIN)
 
-TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, COMSTAInitialized) {
+TEST_P(TaskSchedulerSingleThreadTaskRunnerManagerCommonTest,
+       COMSTAInitialized) {
   scoped_refptr<SingleThreadTaskRunner> com_task_runner =
       single_thread_task_runner_manager_->CreateCOMSTATaskRunnerWithTraits(
-          "A", ThreadPriority::NORMAL, {TaskShutdownBehavior::BLOCK_SHUTDOWN});
+          "A", {TaskShutdownBehavior::BLOCK_SHUTDOWN}, GetParam());
 
   com_task_runner->PostTask(
       FROM_HERE, BindOnce([]() {
@@ -352,6 +460,30 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, COMSTAInitialized) {
   task_tracker_.Shutdown();
 }
 
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, COMSTASameThreadUsed) {

[gab, 2017/05/25 18:47:01]

Seems like this is testing an implementation detail. I don't think we want to
test this for SHARED (we do for DEDICATED).

[robliao, 2017/05/30 20:01:07]

This is currently the only reasonable way to verify reuse. If/when the
implementation changes, then we can update this test.

+  scoped_refptr<SingleThreadTaskRunner> task_runner_1 =
+      single_thread_task_runner_manager_->CreateCOMSTATaskRunnerWithTraits(
+          "A", {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+          SingleThreadTaskRunnerThreadMode::SHARED);
+  scoped_refptr<SingleThreadTaskRunner> task_runner_2 =
+      single_thread_task_runner_manager_->CreateCOMSTATaskRunnerWithTraits(
+          "B", {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+          SingleThreadTaskRunnerThreadMode::SHARED);
+
+  PlatformThreadRef thread_ref_1;
+  task_runner_1->PostTask(FROM_HERE,
+                          BindOnce(&CaptureThreadRef, &thread_ref_1));
+  PlatformThreadRef thread_ref_2;
+  task_runner_2->PostTask(FROM_HERE,
+                          BindOnce(&CaptureThreadRef, &thread_ref_2));
+
+  task_tracker_.Shutdown();
+
+  ASSERT_FALSE(thread_ref_1.is_null());
+  ASSERT_FALSE(thread_ref_2.is_null());
+  EXPECT_EQ(thread_ref_1, thread_ref_2);
+}
+
 namespace {
 
 const wchar_t* const kTestWindowClassName =
@@ -400,7 +532,8 @@ class TaskSchedulerSingleThreadTaskRunnerManagerTestWin
 TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTestWin, PumpsMessages) {
   scoped_refptr<SingleThreadTaskRunner> com_task_runner =
       single_thread_task_runner_manager_->CreateCOMSTATaskRunnerWithTraits(
-          "A", ThreadPriority::NORMAL, {TaskShutdownBehavior::BLOCK_SHUTDOWN});
+          "A", {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+          SingleThreadTaskRunnerThreadMode::DEDICATED);

[gab, 2017/05/25 18:47:01]

Why isn't this also Common?

[robliao, 2017/05/30 20:01:07]

This uses TaskSchedulerSingleThreadTaskRunnerTestWin. Creating a common for this
seemed overkill.

   HWND hwnd = nullptr;
   // HWNDs process messages on the thread that created them, so we have to
   // create them within the context of the task runner to properly simulate a
@@ -451,8 +584,8 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerStartTest,
   WaitableEvent task_running(WaitableEvent::ResetPolicy::MANUAL,
                              WaitableEvent::InitialState::NOT_SIGNALED);
   single_thread_task_runner_manager_
-      ->CreateSingleThreadTaskRunnerWithTraits("A", ThreadPriority::NORMAL,
-                                               TaskTraits())
+      ->CreateSingleThreadTaskRunnerWithTraits(
+          "A", TaskTraits(), SingleThreadTaskRunnerThreadMode::DEDICATED)
       ->PostTask(
           FROM_HERE,
           BindOnce(