TaskScheduler: Replace the SchedulerServiceThread with a base::Thread.

base/task_scheduler/scheduler_worker_pool_impl_unittest.cc

 // Copyright 2016 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_worker_pool_impl.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <unordered_set>
@@ skipping 13 matching lines @@
 #include "base/synchronization/lock.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task_runner.h"
 #include "base/task_scheduler/delayed_task_manager.h"
 #include "base/task_scheduler/scheduler_worker_pool_params.h"
 #include "base/task_scheduler/sequence.h"
 #include "base/task_scheduler/sequence_sort_key.h"
 #include "base/task_scheduler/task_tracker.h"
 #include "base/task_scheduler/test_task_factory.h"
 #include "base/test/gtest_util.h"
+#include "base/test/test_simple_task_runner.h"
+#include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/simple_thread.h"
+#include "base/threading/thread.h"
 #include "base/threading/thread_checker_impl.h"
 #include "base/threading/thread_local_storage.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 namespace internal {
 namespace {
 
 constexpr size_t kNumWorkersInWorkerPool = 4;
 constexpr size_t kNumThreadsPostingTasks = 4;
 constexpr size_t kNumTasksPostedPerThread = 150;
 // This can't be lower because Windows' WaitableEvent wakes up too early when a
 // small timeout is used. This results in many spurious wake ups before a worker
 // is allowed to detach.
 constexpr TimeDelta kReclaimTimeForDetachTests =
     TimeDelta::FromMilliseconds(500);
 constexpr TimeDelta kExtraTimeToWaitForDetach =
     TimeDelta::FromSeconds(1);
 
 using IORestriction = SchedulerWorkerPoolParams::IORestriction;
 
-class TestDelayedTaskManager : public DelayedTaskManager {
- public:
-  TestDelayedTaskManager() : DelayedTaskManager(Bind(&DoNothing)) {}
-
-  void SetCurrentTime(TimeTicks now) { now_ = now; }
-
-  // DelayedTaskManager:
-  TimeTicks Now() const override { return now_; }
-
- private:
-  TimeTicks now_ = TimeTicks::Now();
-
-  DISALLOW_COPY_AND_ASSIGN(TestDelayedTaskManager);
-};
-
 class TaskSchedulerWorkerPoolImplTest
     : public testing::TestWithParam<ExecutionMode> {
  protected:
-  TaskSchedulerWorkerPoolImplTest() = default;
+  TaskSchedulerWorkerPoolImplTest()
+      : service_thread_("TaskSchedulerServiceThread") {}
 
   void SetUp() override {
     InitializeWorkerPool(TimeDelta::Max(), kNumWorkersInWorkerPool);
   }
 
   void TearDown() override {
+    service_thread_.Stop();
     worker_pool_->WaitForAllWorkersIdleForTesting();
     worker_pool_->JoinForTesting();
   }
 
   void InitializeWorkerPool(const TimeDelta& suggested_reclaim_time,
                             size_t num_workers) {
+    ASSERT_FALSE(worker_pool_);
+    ASSERT_FALSE(delayed_task_manager_);
+    service_thread_.Start();
+    delayed_task_manager_ =
+        base::MakeUnique<DelayedTaskManager>(service_thread_.task_runner());
     worker_pool_ = SchedulerWorkerPoolImpl::Create(
         SchedulerWorkerPoolParams("TestWorkerPool", ThreadPriority::NORMAL,
                                   IORestriction::ALLOWED, num_workers,
                                   suggested_reclaim_time),
         Bind(&TaskSchedulerWorkerPoolImplTest::ReEnqueueSequenceCallback,
              Unretained(this)),
-        &task_tracker_, &delayed_task_manager_);
+        &task_tracker_, delayed_task_manager_.get());
     ASSERT_TRUE(worker_pool_);
   }
 
   std::unique_ptr<SchedulerWorkerPoolImpl> worker_pool_;
 
   TaskTracker task_tracker_;
-  TestDelayedTaskManager delayed_task_manager_;
+  Thread service_thread_;
+  std::unique_ptr<DelayedTaskManager> delayed_task_manager_;
 
  private:
   void ReEnqueueSequenceCallback(scoped_refptr<Sequence> sequence) {
     // In production code, this callback would be implemented by the
     // TaskScheduler which would first determine which PriorityQueue the
     // sequence must be re-enqueued.
     const SequenceSortKey sort_key(sequence->GetSortKey());
     worker_pool_->ReEnqueueSequence(std::move(sequence), sort_key);
   }
 
@@ skipping 185 matching lines @@
 }
 
 // Verify that a Task can't be posted after shutdown.
 TEST_P(TaskSchedulerWorkerPoolImplTest, PostTaskAfterShutdown) {
   auto task_runner =
       worker_pool_->CreateTaskRunnerWithTraits(TaskTraits(), GetParam());
   task_tracker_.Shutdown();
   EXPECT_FALSE(task_runner->PostTask(FROM_HERE, Bind(&ShouldNotRunCallback)));
 }
 
-// Verify that a Task posted with a delay is added to the DelayedTaskManager and
-// doesn't run before its delay expires.
+// Verify that a Task doesn't run before its delay expires.
+TEST_P(TaskSchedulerWorkerPoolImplTest, PostDelayedTaskNeverRuns) {

[gab, 2016/10/17 19:09:18]

I'm not convinced that this test is required.
TaskSchedulerWorkerPoolImplTest.PostDelayedTask already tests that the
functionality is hooked up and this thus feels like it's merely re-testing
DelayedTaskManager and base::Thread?

[fdoray, 2016/10/18 20:10:49]

Removed the test.

+  // Post a task with a very long delay.
+  EXPECT_TRUE(worker_pool_->CreateTaskRunnerWithTraits(TaskTraits(), GetParam())
+                  ->PostDelayedTask(FROM_HERE, Bind([]() {
+                                      ADD_FAILURE()
+                                          << "The delayed task should not run.";
+                                    }),
+                                    TimeDelta::FromDays(30)));
+
+  // Wait a few milliseconds. The test will fail if the delayed task runs.
+  PlatformThread::Sleep(TestTimeouts::tiny_timeout());
+}
+
+// Verify that a Task runs shortly after its delay expires.
 TEST_P(TaskSchedulerWorkerPoolImplTest, PostDelayedTask) {
-  EXPECT_TRUE(delayed_task_manager_.GetDelayedRunTime().is_null());
+  TimeTicks start_time = TimeTicks::Now();
 
-  // Post a delayed task.
+  // Post a task with a short delay.
   WaitableEvent task_ran(WaitableEvent::ResetPolicy::MANUAL,

[fdoray, 2016/10/18 20:10:49]

I think this test is useful because it verifies that the delay isn't "lost" (you
did "loose" delays when you implemented the redirection of SWP to
TaskScheduler).

                          WaitableEvent::InitialState::NOT_SIGNALED);
   EXPECT_TRUE(worker_pool_->CreateTaskRunnerWithTraits(TaskTraits(), GetParam())
                   ->PostDelayedTask(FROM_HERE, Bind(&WaitableEvent::Signal,
                                                     Unretained(&task_ran)),
-                                    TimeDelta::FromSeconds(10)));
+                                    TestTimeouts::tiny_timeout()));
 
-  // The task should have been added to the DelayedTaskManager.
-  EXPECT_FALSE(delayed_task_manager_.GetDelayedRunTime().is_null());
+  // Wait until the task runs.
+  task_ran.Wait();
 
-  // The task shouldn't run.
-  EXPECT_FALSE(task_ran.IsSignaled());
-
-  // Fast-forward time and post tasks that are ripe for execution.
-  delayed_task_manager_.SetCurrentTime(
-      delayed_task_manager_.GetDelayedRunTime());
-  delayed_task_manager_.PostReadyTasks();
-
-  // The task should run.
-  task_ran.Wait();
+  // Expect the task to run less than 250 ms after its delay expires.
+  EXPECT_LT(TimeTicks::Now() - start_time,
+            TimeDelta::FromMilliseconds(250) + TestTimeouts::tiny_timeout());

[gab, 2016/10/17 19:09:18]

Also EXPECT_GT(..., tiny_timeout)

[fdoray, 2016/10/18 20:10:49]

Done.

 }
 
 // Verify that the RunsTasksOnCurrentThread() method of a SEQUENCED TaskRunner
 // returns false when called from a task that isn't part of the sequence.
 TEST_P(TaskSchedulerWorkerPoolImplTest, SequencedRunsTasksOnCurrentThread) {
   scoped_refptr<TaskRunner> task_runner(
       worker_pool_->CreateTaskRunnerWithTraits(TaskTraits(), GetParam()));
   scoped_refptr<TaskRunner> sequenced_task_runner(
       worker_pool_->CreateTaskRunnerWithTraits(TaskTraits(),
                                                ExecutionMode::SEQUENCED));
@@ skipping 51 matching lines @@
   TaskSchedulerWorkerPoolImplIORestrictionTest() = default;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TaskSchedulerWorkerPoolImplIORestrictionTest);
 };
 
 }  // namespace
 
 TEST_P(TaskSchedulerWorkerPoolImplIORestrictionTest, IORestriction) {
   TaskTracker task_tracker;
-  DelayedTaskManager delayed_task_manager(Bind(&DoNothing));
+  DelayedTaskManager delayed_task_manager(
+      make_scoped_refptr(new TestSimpleTaskRunner));
 
   auto worker_pool = SchedulerWorkerPoolImpl::Create(
       SchedulerWorkerPoolParams("TestWorkerPoolWithParam",
                                 ThreadPriority::NORMAL, GetParam(), 1U,
                                 TimeDelta::Max()),
       Bind(&NotReachedReEnqueueSequenceCallback), &task_tracker,
       &delayed_task_manager);
   ASSERT_TRUE(worker_pool);
 
   WaitableEvent task_ran(WaitableEvent::ResetPolicy::MANUAL,
@@ skipping 333 matching lines @@
   worker_pool_.reset();
 
   // Verify that counts were recorded to the histogram as expected.
   EXPECT_EQ(0, histogram->SnapshotSamples()->GetCount(0));
   EXPECT_EQ(1, histogram->SnapshotSamples()->GetCount(3));
   EXPECT_EQ(0, histogram->SnapshotSamples()->GetCount(10));
 }
 
 }  // namespace internal
 }  // namespace base