TaskScheduler [6] SchedulerWorkerThread

base/task_scheduler/worker_thread.h

+// 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.
+
+#ifndef BASE_TASK_SCHEDULER_WORKER_THREAD_H_
+#define BASE_TASK_SCHEDULER_WORKER_THREAD_H_
+
+#include "base/base_export.h"
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/single_thread_task_runner.h"
+#include "base/synchronization/condition_variable.h"
+#include "base/task_scheduler/priority_queue.h"
+#include "base/task_scheduler/scheduler_lock.h"
+#include "base/task_scheduler/sequence.h"
+#include "base/task_scheduler/task_traits.h"
+#include "base/threading/platform_thread.h"
+
+namespace base {
+namespace internal {
+
+class TaskTracker;
+
+// A thread that runs tasks from a shared and a single-threaded priority queue.
+// This class is thread-safe.
+class BASE_EXPORT WorkerThread : public PlatformThread::Delegate {
+ public:
+  // Callback invoked by |worker_thread| to reinsert |sequence| in the

[gab, 2016/03/21 19:11:55]

The callback contract can't really dictate who calls it. So maybe instead:

// Callback invoked to reinsert |sequence| in the appropriate PriorityQueue
after one of its tasks was executed on |worker_thread|.

And actually this comment should probably not mention "priority queue" either as
nothing forces the destination to be a PQ.

Two options:
1) Make the comment more generic.
2) Define the type on PriorityQueue instead of here.

[fdoray, 2016/03/24 19:21:10]

Done.

+  // appropriate priority queue it has executed one of its tasks.

[gab, 2016/03/21 19:11:55]

missing "after" before "it"?

[fdoray, 2016/03/24 19:21:10]

Done.

+  using ReinsertSequenceCallback =
+      Callback<void(scoped_refptr<Sequence> sequence,
+                    const WorkerThread* worker_thread)>;
+
+  // Callback invoked when |worker_thread| becomes idle.
+  using BecomesIdleCallback = Callback<void(WorkerThread* worker_thread)>;
+
+  // Creates a WorkerThread with priority |thread_priority| that runs tasks from
+  // a single-thread priority queue and from |shared_priority_queue|.

[gab, 2016/03/21 19:11:54]

s/a single-thread priority queue/its single-thread priority queue/

[fdoray, 2016/03/24 19:21:10]

Done.

+  // |reinsert_sequence_callback| is invoked to reinsert a sequence in the
+  // appropriate priority queue after one of its tasks has been executed.
+  // |task_tracker| is used to handle shutdown behavior of tasks.
+  static scoped_ptr<WorkerThread> CreateWorkerThread(
+      ThreadPriority thread_priority,
+      PriorityQueue* shared_priority_queue,
+      const ReinsertSequenceCallback& reinsert_sequence_callback,
+      const BecomesIdleCallback& becomes_idle_callback,
+      TaskTracker* task_tracker);
+
+  // A WorkerThread must only be destroyed after JoinForTesting() has returned.

[gab, 2016/03/21 19:11:55]

Hmmm, isn't the WorkerThread destroyed in the product where JoinForTesting() is
an illegal call?

Or I guess we decided to never delete scheduler objects on product shutdown? If
that's the reason, this should be highlighted in this comment.

[fdoray, 2016/03/24 19:21:10]

Done.

+  ~WorkerThread() override;
+
+  // Wakes up the WorkerThread. It is guaranteed that the WorkerThread will stay
+  // awake until both of its PriorityQueues become empty after this is called.
+  // Returns false if the WorkerThread was already awake.
+  bool WakeUp();
+
+  // Returns a SingleThreadTaskRunner whose PostTask invocations will result in
+  // scheduling tasks on this WorkerThread with |traits|.

[gab, 2016/03/21 19:11:54]

s/tasks on this WorkerThread with |traits|/tasks with |traits| on this
WorkerThread/

(i.e. makes it clearer that |traits| apply to tasks not WorkerThread)

[fdoray, 2016/03/24 19:21:10]

Done.

+  scoped_refptr<SingleThreadTaskRunner> CreateTaskRunnerWithTraits(
+      const TaskTraits& traits);
+
+  // Joins the WorkerThread. If a task is already running, it will be allowed
+  // to complete its execution. This can only be called once.
+  void JoinForTesting();
+
+ private:
+  WorkerThread(ThreadPriority thread_priority,
+               PriorityQueue* shared_priority_queue,
+               const ReinsertSequenceCallback& reinsert_sequence_callback,
+               const BecomesIdleCallback& becomes_idle_callback,
+               TaskTracker* task_tracker);
+
+  // Extracts the sequence with the highest priority from
+  // |shared_priority_queue_| or |single_thread_priority_queue_|.
+  // |single_thread| is set to true if the returned task comes from
+  // |single_thread_priority_queue_|.
+  scoped_refptr<Sequence> GetWork(bool* single_thread);

[gab, 2016/03/21 19:11:54]

s/single_thread/is_single_thread/

[fdoray, 2016/03/24 19:21:10]

Done.

+
+  // Waits until |is_awake_| becomes true.
+  void WaitUntilWakeUp();

[gab, 2016/03/21 19:11:55]

WaitUntilWokenUp or WaitForWakeUp?

[fdoray, 2016/03/24 19:21:10]

Done (method gone).

+
+  // PlatformThread::Delegate:
+  void ThreadMain() override;
+
+  // Platform thread managed by this WorkerThread.
+  PlatformThreadHandle thread_handle_;
+
+  // Synchronizes access to |wake_up_cv_|, |is_awake_| and
+  // |should_exit_for_testing_|.
+  SchedulerLock lock_;
+
+  // Condition variable signaled when |is_awake_| becomes true.
+  mutable scoped_ptr<ConditionVariable> wake_up_cv_;
+
+  // True from the time WakeUp() is called to the time the WorkerThread doesn't

[gab, 2016/03/21 19:11:55]

s/the WorkerThread/this WorkerThread/

(same below)

[fdoray, 2016/03/24 19:21:10]

Done.

+  // have any remaining work to do.
+  bool is_awake_;
+
+  // True once JoinForTesting() has been called.
+  bool should_exit_for_testing_;
+
+  // The single-threaded priority queue from which the WorkerThread gets work.
+  PriorityQueue single_thread_priority_queue_;
+
+  // The shared priority queue from which the WorkerThread gets work.
+  PriorityQueue* const shared_priority_queue_;
+
+  const ReinsertSequenceCallback reinsert_sequence_callback_;
+  const BecomesIdleCallback becomes_idle_callback_;
+  TaskTracker* const task_tracker_;
+
+  DISALLOW_COPY_AND_ASSIGN(WorkerThread);
+};
+
+}  // namespace internal
+}  // namespace base
+
+#endif  // BASE_TASK_SCHEDULER_WORKER_THREAD_H_