Introduce SchedulerSingleThreadTaskRunnerManager

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
new file mode 100644
index 0000000000000000000000000000000000000000..d78c6d0826af2f9c5dd5fe2e4c3256031bd8ce0a
--- /dev/null
+++ b/base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc
@@ -0,0 +1,243 @@
+// Copyright 2017 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_scheduler/scheduler_single_thread_task_runner_manager.h"
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/task_scheduler/delayed_task_manager.h"
+#include "base/task_scheduler/post_task.h"
+#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/scoped_task_scheduler.h"
+#include "base/test/test_timeouts.h"
+#include "base/threading/thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+namespace base {

[gab, 2017/02/21 21:39:50]

nit empty line above

[robliao, 2017/02/22 01:04:19]

Done.

+namespace internal {
+
+namespace {
+
+enum WorkerPoolType {
+  BACKGROUND_WORKER_POOL = 0,
+  FOREGROUND_WORKER_POOL,
+};
+
+static size_t GetThreadPoolIndexForTraits(const TaskTraits& traits) {
+  return traits.priority() == TaskPriority::BACKGROUND ? BACKGROUND_WORKER_POOL
+                                                       : FOREGROUND_WORKER_POOL;
+}
+
+class TaskSchedulerSingleThreadTaskRunnerManagerTest : public testing::Test {
+ public:
+  TaskSchedulerSingleThreadTaskRunnerManagerTest()
+      : service_thread_("TaskSchedulerServiceThread") {}
+
+  void SetUp() override {
+    ASSERT_FALSE(delayed_task_manager_);

[gab, 2017/02/21 21:39:50]

It's not typical to ASSERT_FALSE on the fixture's ptrs at the top of SetUp()

[robliao, 2017/02/22 01:04:19]

Done.

+    service_thread_.Start();
+
+    using StandbyThreadPolicy = SchedulerWorkerPoolParams::StandbyThreadPolicy;
+
+    std::vector<SchedulerWorkerPoolParams> params_vector;
+
+    ASSERT_EQ(BACKGROUND_WORKER_POOL, params_vector.size());
+    params_vector.emplace_back("Background", ThreadPriority::BACKGROUND,
+                               StandbyThreadPolicy::LAZY, 1U, TimeDelta::Max());
+
+    ASSERT_EQ(FOREGROUND_WORKER_POOL, params_vector.size());
+    params_vector.emplace_back("Foreground", ThreadPriority::NORMAL,
+                               StandbyThreadPolicy::LAZY, 1U, TimeDelta::Max());
+
+    delayed_task_manager_ =
+        MakeUnique<DelayedTaskManager>(service_thread_.task_runner());
+    single_thread_task_runner_manager_ =
+        MakeUnique<SchedulerSingleThreadTaskRunnerManager>(
+            params_vector, Bind(&GetThreadPoolIndexForTraits), &task_tracker_,
+            delayed_task_manager_.get());
+  }
+
+  void TearDown() override {
+    single_thread_task_runner_manager_->JoinForTesting();
+    single_thread_task_runner_manager_.reset();
+    delayed_task_manager_.reset();
+    service_thread_.Stop();
+  }
+
+ protected:
+  std::unique_ptr<SchedulerSingleThreadTaskRunnerManager>
+      single_thread_task_runner_manager_;
+  TaskTracker task_tracker_;
+
+ private:
+  Thread service_thread_;
+  std::unique_ptr<DelayedTaskManager> delayed_task_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(TaskSchedulerSingleThreadTaskRunnerManagerTest);
+};
+
+void CaptureThreadId(PlatformThreadId* thread_id) {
+  ASSERT_TRUE(thread_id);
+  *thread_id = PlatformThread::CurrentId();
+}
+
+void CaptureThreadPriority(ThreadPriority* thread_priority) {
+  ASSERT_TRUE(thread_priority);
+  *thread_priority = PlatformThread::GetCurrentThreadPriority();
+}
+
+void ShouldNotRunCallback() {

[gab, 2017/02/21 21:39:50]

ShouldNotRun() -- i.e. I'd drop "Callback", it's only a Callback when it's bound
and at that point it's implicit.

[robliao, 2017/02/22 01:04:18]

This was another holdover
https://cs.chromium.org/chromium/src/base/task_scheduler/scheduler_worker_poo...

Removed here and originating file updated at
https://codereview.chromium.org/2710853002

+  ADD_FAILURE() << "Ran a task that shouldn't run.";
+}
+
+}  // namespace
+
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, DifferentThreadsUsed) {
+  scoped_refptr<SingleThreadTaskRunner> task_runner_1 =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              TaskTraits().WithShutdownBehavior(
+                  TaskShutdownBehavior::BLOCK_SHUTDOWN));
+  scoped_refptr<SingleThreadTaskRunner> task_runner_2 =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              TaskTraits().WithShutdownBehavior(
+                  TaskShutdownBehavior::BLOCK_SHUTDOWN));
+
+  PlatformThreadId thread_id_1 = kInvalidThreadId;
+  task_runner_1->PostTask(FROM_HERE, Bind(&CaptureThreadId, &thread_id_1));
+  PlatformThreadId thread_id_2 = kInvalidThreadId;
+  task_runner_2->PostTask(FROM_HERE, Bind(&CaptureThreadId, &thread_id_2));
+
+  task_tracker_.Shutdown();
+
+  ASSERT_NE(kInvalidThreadId, thread_id_1);
+  ASSERT_NE(kInvalidThreadId, thread_id_2);
+  EXPECT_NE(thread_id_1, thread_id_2);
+}
+
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, 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(
+              TaskTraits().WithPriority(TaskPriority::BACKGROUND));
+  scoped_refptr<SingleThreadTaskRunner> task_runner_user_visible =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              TaskTraits().WithPriority(TaskPriority::USER_VISIBLE));
+  scoped_refptr<SingleThreadTaskRunner> task_runner_user_blocking =
+      single_thread_task_runner_manager_
+          ->CreateSingleThreadTaskRunnerWithTraits(
+              TaskTraits()
+                  .WithPriority(TaskPriority::USER_BLOCKING)
+                  .WithShutdownBehavior(TaskShutdownBehavior::BLOCK_SHUTDOWN));
+
+  ThreadPriority thread_priority_background;
+  task_runner_background->PostTask(
+      FROM_HERE, Bind(&CaptureThreadPriority, &thread_priority_background));
+  WaitableEvent waitable_event_background(
+      WaitableEvent::ResetPolicy::MANUAL,
+      WaitableEvent::InitialState::NOT_SIGNALED);
+  task_runner_background->PostTask(
+      FROM_HERE,
+      Bind(&WaitableEvent::Signal, Unretained(&waitable_event_background)));
+
+  ThreadPriority thread_priority_user_visible;
+  task_runner_user_visible->PostTask(
+      FROM_HERE, Bind(&CaptureThreadPriority, &thread_priority_user_visible));
+  WaitableEvent waitable_event_user_visible(
+      WaitableEvent::ResetPolicy::MANUAL,
+      WaitableEvent::InitialState::NOT_SIGNALED);
+  task_runner_user_visible->PostTask(
+      FROM_HERE,
+      Bind(&WaitableEvent::Signal, Unretained(&waitable_event_user_visible)));
+
+  ThreadPriority thread_priority_user_blocking;
+  task_runner_user_blocking->PostTask(
+      FROM_HERE, Bind(&CaptureThreadPriority, &thread_priority_user_blocking));
+  WaitableEvent waitable_event_user_blocking(
+      WaitableEvent::ResetPolicy::MANUAL,
+      WaitableEvent::InitialState::NOT_SIGNALED);
+  task_runner_user_blocking->PostTask(
+      FROM_HERE,
+      Bind(&WaitableEvent::Signal, Unretained(&waitable_event_user_blocking)));
+
+  waitable_event_background.Wait();
+  waitable_event_user_visible.Wait();
+  waitable_event_user_blocking.Wait();
+
+  if (Lock::HandlesMultipleThreadPriorities())
+    EXPECT_EQ(ThreadPriority::BACKGROUND, thread_priority_background);
+  else
+    EXPECT_EQ(ThreadPriority::NORMAL, thread_priority_background);
+  EXPECT_EQ(ThreadPriority::NORMAL, thread_priority_user_visible);
+  EXPECT_EQ(ThreadPriority::NORMAL, thread_priority_user_blocking);
+}
+
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PostTaskAfterShutdown) {
+  auto task_runner = single_thread_task_runner_manager_
+                         ->CreateSingleThreadTaskRunnerWithTraits(TaskTraits());
+  task_tracker_.Shutdown();
+  EXPECT_FALSE(task_runner->PostTask(FROM_HERE, Bind(&ShouldNotRunCallback)));
+}
+
+// Verify that a Task runs shortly after its delay expires.
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest, PostDelayedTask) {
+  TimeTicks start_time = TimeTicks::Now();
+
+  // Post a task with a short delay.
+  WaitableEvent task_ran(WaitableEvent::ResetPolicy::MANUAL,
+                         WaitableEvent::InitialState::NOT_SIGNALED);
+  auto task_runner = single_thread_task_runner_manager_
+                         ->CreateSingleThreadTaskRunnerWithTraits(TaskTraits());
+  EXPECT_TRUE(task_runner->PostDelayedTask(
+      FROM_HERE, Bind(&WaitableEvent::Signal, Unretained(&task_ran)),
+      TestTimeouts::tiny_timeout()));
+
+  // Wait until the task runs.
+  task_ran.Wait();
+
+  // Expect the task to run after its delay expires, but not more than 250 ms
+  // after that.
+  const TimeDelta actual_delay = TimeTicks::Now() - start_time;
+  EXPECT_GE(actual_delay, TestTimeouts::tiny_timeout());
+  EXPECT_LT(actual_delay,
+            TimeDelta::FromMilliseconds(250) + TestTimeouts::tiny_timeout());

[gab, 2017/02/21 21:39:50]

Flip as 

TestTimeouts::tiny_timeout() + TimeDelta::FromMilliseconds(250)

(i.e. 250ms *after* timeout to me sounds like "timeout + 250ms")

[robliao, 2017/02/22 01:04:19]

The important part here is the 250ms. This should be read like
250ms + _epsilon.
_eplison + 250ms doesn't flow as nicely.

This is also consistent with our SchedulerWorkerPoolImpl tests.
https://cs.chromium.org/chromium/src/base/task_scheduler/scheduler_worker_poo...

+}
+
+// Verify that the RunsTasksOnCurrentThread() method of a SEQUENCED TaskRunner
+// returns false when called from a task that isn't part of the sequence. Note:
+// Tests that use TestTaskFactory already verify that RunsTasksOnCurrentThread()
+// returns true when appropriate so this method complements it to get full
+// coverage of that method.
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest,
+       SequencedRunsTasksOnCurrentThread) {
+  test::ScopedTaskScheduler scoped_task_scheduler;
+  auto task_runner = single_thread_task_runner_manager_
+                         ->CreateSingleThreadTaskRunnerWithTraits(TaskTraits());
+  scoped_refptr<SequencedTaskRunner> sequenced_task_runner(

[gab, 2017/02/21 21:39:50]

Perhaps arbitrary but I prefer = instead of () to init when not directly
invoking a constructor (I don't find scoped_refptr's constructor counts).

[robliao, 2017/02/22 01:04:18]

Done one better and promoted to auto.
Originating file updated at https://codereview.chromium.org/2710853002

+      CreateSequencedTaskRunnerWithTraits(TaskTraits()));
+
+  WaitableEvent task_ran(WaitableEvent::ResetPolicy::MANUAL,
+                         WaitableEvent::InitialState::NOT_SIGNALED);
+  task_runner->PostTask(
+      FROM_HERE,
+      Bind(
+          [](scoped_refptr<TaskRunner> sequenced_task_runner,
+             WaitableEvent* task_ran) {
+            EXPECT_FALSE(sequenced_task_runner->RunsTasksOnCurrentThread());
+            task_ran->Signal();
+          },
+          sequenced_task_runner, Unretained(&task_ran)));
+  task_ran.Wait();
+}
+
+}  // namespace internal
+}  // namespace base