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

base/task_scheduler/delayed_task_manager.cc

Index: base/task_scheduler/delayed_task_manager.cc
diff --git a/base/task_scheduler/delayed_task_manager.cc b/base/task_scheduler/delayed_task_manager.cc
index d648b9d4628fab6324b9e8474daffa607692f0d8..1cc928b0f0d080d926813c97037e2a68da7710b7 100644
--- a/base/task_scheduler/delayed_task_manager.cc
+++ b/base/task_scheduler/delayed_task_manager.cc
@@ -6,49 +6,18 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/task_runner.h"
 #include "base/task_scheduler/scheduler_worker_pool.h"
 
 namespace base {
 namespace internal {
 
-struct DelayedTaskManager::DelayedTask {
-  DelayedTask(std::unique_ptr<Task> task,
-              scoped_refptr<Sequence> sequence,
-              SchedulerWorker* worker,
-              SchedulerWorkerPool* worker_pool,
-              uint64_t index)
-      : task(std::move(task)),
-        sequence(std::move(sequence)),
-        worker(worker),
-        worker_pool(worker_pool),
-        index(index) {}
-
-  DelayedTask(DelayedTask&& other) = default;
-
-  ~DelayedTask() = default;
-
-  DelayedTask& operator=(DelayedTask&& other) = default;
-
-  // |task| will be posted to |worker_pool| with |sequence| and |worker|
-  // when it becomes ripe for execution.
-  std::unique_ptr<Task> task;
-  scoped_refptr<Sequence> sequence;
-  SchedulerWorker* worker;
-  SchedulerWorkerPool* worker_pool;
-
-  // Ensures that tasks that have the same |delayed_run_time| are sorted
-  // according to the order in which they were added to the DelayedTaskManager.
-  uint64_t index;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(DelayedTask);
-};
-
 DelayedTaskManager::DelayedTaskManager(
-    const Closure& on_delayed_run_time_updated)
-    : on_delayed_run_time_updated_(on_delayed_run_time_updated) {
-  DCHECK(!on_delayed_run_time_updated_.is_null());
+    scoped_refptr<TaskRunner> service_thread_task_runner)
+    : service_thread_task_runner_(std::move(service_thread_task_runner)) {
+  DCHECK(service_thread_task_runner_);
 }
 
 DelayedTaskManager::~DelayedTaskManager() = default;
@@ -61,92 +30,16 @@ void DelayedTaskManager::AddDelayedTask(std::unique_ptr<Task> task,
   DCHECK(sequence);
   DCHECK(worker_pool);
 
-  const TimeTicks new_task_delayed_run_time = task->delayed_run_time;
-  TimeTicks current_delayed_run_time;
-
-  {
-    AutoSchedulerLock auto_lock(lock_);
-
-    if (!delayed_tasks_.empty())
-      current_delayed_run_time = delayed_tasks_.top().task->delayed_run_time;
-
-    delayed_tasks_.emplace(std::move(task), std::move(sequence), worker,
-                           worker_pool, ++delayed_task_index_);
-  }
-
-  if (current_delayed_run_time.is_null() ||
-      new_task_delayed_run_time < current_delayed_run_time) {
-    on_delayed_run_time_updated_.Run();
-  }
-}
-
-void DelayedTaskManager::PostReadyTasks() {
-  const TimeTicks now = Now();
-
-  // Move delayed tasks that are ready for execution into |ready_tasks|. Don't
-  // post them right away to avoid imposing an unecessary lock dependency on
-  // PostTaskNowHelper.
-  std::vector<DelayedTask> ready_tasks;
-
-  {
-    AutoSchedulerLock auto_lock(lock_);
-    while (!delayed_tasks_.empty() &&
-           delayed_tasks_.top().task->delayed_run_time <= now) {
-      // The const_cast for std::move is okay since we're immediately popping
-      // the task from |delayed_tasks_|. See DelayedTaskComparator::operator()
-      // for minor debug-check implications.
-      ready_tasks.push_back(
-          std::move(const_cast<DelayedTask&>(delayed_tasks_.top())));
-      delayed_tasks_.pop();
-    }
-  }
-
-  // Post delayed tasks that are ready for execution.
-  for (auto& delayed_task : ready_tasks) {
-    delayed_task.worker_pool->PostTaskWithSequenceNow(
-        std::move(delayed_task.task), std::move(delayed_task.sequence),
-        delayed_task.worker);
-  }
-}
-
-TimeTicks DelayedTaskManager::GetDelayedRunTime() const {
-  AutoSchedulerLock auto_lock(lock_);
-
-  if (delayed_tasks_.empty())
-    return TimeTicks();
-
-  return delayed_tasks_.top().task->delayed_run_time;
-}
-
-// In std::priority_queue, the largest element is on top. Therefore, this
-// comparator returns true if the delayed run time of |right| is earlier than
-// the delayed run time of |left|.
-bool DelayedTaskManager::DelayedTaskComparator::operator()(
-    const DelayedTask& left,
-    const DelayedTask& right) const {
-#ifndef NDEBUG
-  // Due to STL consistency checks in Windows and const_cast'ing right before
-  // popping the DelayedTask, a null task can be passed to this comparator in
-  // Debug builds. To satisfy these consistency checks, this comparator
-  // considers null tasks to be the larger than anything.
-  DCHECK(left.task || right.task);
-  if (!left.task)
-    return false;
-  if (!right.task)
-    return true;
-#else
-  DCHECK(left.task);
-  DCHECK(right.task);
-#endif  // NDEBUG
-  if (left.task->delayed_run_time > right.task->delayed_run_time)
-    return true;
-  if (left.task->delayed_run_time < right.task->delayed_run_time)
-    return false;
-  return left.index > right.index;
-}
+  const TimeDelta delay = task->delay;
+  DCHECK(!delay.is_zero());
 
-TimeTicks DelayedTaskManager::Now() const {
-  return TimeTicks::Now();
+  // TODO(fdoray): Use |task->delayed_run_time| on the service thread
+  // MessageLoop rather than recomputing it from |delay|.
+  service_thread_task_runner_->PostDelayedTask(
+      FROM_HERE, Bind(&SchedulerWorkerPool::PostTaskWithSequenceNow,
+                      Unretained(worker_pool), Passed(std::move(task)),

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

std::move not required inside Passed() IIRC

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

It's required. See other call sites
https://cs.chromium.org/chromium/src/base/bind_helpers.h?sq=package:chromium&...

c:\src\chromium\src\base\task_scheduler\delayed_task_manager.cc(40): error
C2672: 'Passed': no matching overloaded function found
c:\src\chromium\src\base\task_scheduler\delayed_task_manager.cc(40): error
C2784: 'base::internal::PassedWrapper<T> base::Passed(T *)': could not deduce
template argument for 'T *' from
'std::unique_ptr<base::internal::Task,std::default_delete<_Ty>>'
        with
        [
            _Ty=base::internal::Task
        ]
c:\src\chromium\src\base\bind_helpers.h(444): note: see declaration of
'base::Passed'
c:\src\chromium\src\base\task_scheduler\delayed_task_manager.cc(40): error
C2783: 'base::internal::PassedWrapper<T> base::Passed(T &&)': could not deduce
template argument for '__formal'
c:\src\chromium\src\base\bind_helpers.h(440): note: see declaration of
'base::Passed'
c:\src\chromium\src\base\task_scheduler\delayed_task_manager.cc(42): error
C2664: 'bool base::TaskRunner::PostDelayedTask(const tracked_objects::Location
&,const base::Closure &,base::TimeDelta)': cannot convert argument 2 from
'base::Callback<R (base::internal::SchedulerWorker
*),base::internal::CopyMode::Copyable,base::internal::RepeatMode::Repeating>' to
'const base::Closure &'
        with
        [
            R=void
        ]
c:\src\chromium\src\base\task_scheduler\delayed_task_manager.cc(42): note:
Reason: cannot convert from 'base::Callback<R (base::internal::SchedulerWorker
*),base::internal::CopyMode::Copyable,base::internal::RepeatMode::Repeating>' to
'const base::Closure'
        with
        [
            R=void
        ]
c:\src\chromium\src\base\task_scheduler\delayed_task_manager.cc(42): note: No
user-defined-conversion operator available that can perform this conversion, or
the operator cannot be called

[gab, 2016/10/18 20:59:03]

Ah interesting, in retrospect it makes sense. base::Passed was previously
adapted to work with scoped_ptr (which was a hack with MOVE_ONLY_CPP03 macro or
something) but I guess it can't work around it for std::unique_ptr (i.e. can't
fake an rvalue). This mostly makes base::Passed outdated I think but wtv that's
beyond this CL and it's still required in today's Bind world.

+                      std::move(sequence), Unretained(worker)),
+      delay);
 }
 
 }  // namespace internal