TaskScheduler [6] SchedulerWorkerThread

base/task_scheduler/scheduler_worker_thread.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_thread.h"
+
+#include <stddef.h>
+
+#include <utility>
+
+#include "base/logging.h"
+#include "base/task_scheduler/task_tracker.h"
+
+namespace base {
+namespace internal {
+
+scoped_ptr<SchedulerWorkerThread>
+SchedulerWorkerThread::CreateSchedulerWorkerThread(
+    ThreadPriority thread_priority,
+    const Closure& main_entry_callback,
+    const GetWorkCallback& get_work_callback,
+    const RanTaskFromSequenceCallback& ran_task_from_sequence_callback,
+    TaskTracker* task_tracker) {
+  scoped_ptr<SchedulerWorkerThread> worker_thread(new SchedulerWorkerThread(
+      thread_priority, main_entry_callback, get_work_callback,
+      ran_task_from_sequence_callback, task_tracker));
+
+  if (worker_thread->thread_handle_.is_null())
+    return nullptr;
+  return worker_thread;
+}
+
+SchedulerWorkerThread::~SchedulerWorkerThread() {
+  DCHECK(ShouldExitForTesting());
+}
+
+void SchedulerWorkerThread::WakeUp() {
+  wake_up_event_.Signal();

[gab, 2016/04/05 23:35:20]

Should this DCHECK(!wake_up_event_.IsSignaled());? Or is it okay to call WakeUp
on an active thread? With the current impl, calling it while its awake would
mean it would immediately wake up (consume the signal) the next time it tries to
sleep.

Also, please update the API to reflect the decision made here.

Edit: just read comment on line 89. We should either disallow multiple WakeUps
or make the API clear that they're allowed. First reading about it being a
possibility in a comment at the end of ThreadMain is too late.

[fdoray, 2016/04/07 16:07:43]

We support calls to WakeUp on an active thread. It will be called every time
that a Sequence is inserted into the single-threaded PQ. Updated the method's
description.

+}
+
+void SchedulerWorkerThread::JoinForTesting() {
+  {
+    AutoSchedulerLock auto_lock(should_exit_for_testing_lock_);
+    should_exit_for_testing_ = true;
+  }
+  WakeUp();
+  PlatformThread::Join(thread_handle_);
+}
+
+SchedulerWorkerThread::SchedulerWorkerThread(
+    ThreadPriority thread_priority,
+    const Closure& main_entry_callback,
+    const GetWorkCallback& get_work_callback,
+    const RanTaskFromSequenceCallback& ran_task_from_sequence_callback,
+    TaskTracker* task_tracker)
+    : wake_up_event_(false, false),
+      main_entry_callback_(main_entry_callback),
+      get_work_callback_(get_work_callback),
+      ran_task_from_sequence_callback_(ran_task_from_sequence_callback),
+      task_tracker_(task_tracker) {
+  DCHECK(!main_entry_callback_.is_null());
+  DCHECK(!get_work_callback_.is_null());
+  DCHECK(!ran_task_from_sequence_callback_.is_null());
+  DCHECK(task_tracker_);
+
+  static const size_t kDefaultStackSize = 0;

[gab, 2016/04/05 23:35:20]

No need for "static" on non-compound local const types (i.e. only for
arrays/structs local consts).

[fdoray, 2016/04/07 16:07:43]

Done.

+  PlatformThread::CreateWithPriority(kDefaultStackSize, this, &thread_handle_,
+                                     thread_priority);
+}
+
+void SchedulerWorkerThread::ThreadMain() {
+  main_entry_callback_.Run();
+
+  // A SchedulerWorkerThread starts out sleeping.

[gab, 2016/04/05 23:35:20]

This should be mentioned in the API (header).

[fdoray, 2016/04/07 16:07:42]

Done.

+  wake_up_event_.Wait();
+
+  while (!task_tracker_->shutdown_completed() && !ShouldExitForTesting()) {
+    // Get the sequence containing the next task to execute.
+    scoped_refptr<Sequence> sequence = get_work_callback_.Run(this);
+
+    if (!sequence) {
+      wake_up_event_.Wait();
+      continue;
+    }
+
+    task_tracker_->RunTask(sequence->PeekTask());
+    ran_task_from_sequence_callback_.Run(this, std::move(sequence));
+
+    // Calling WakeUp() guarantees that this SchedulerWorkerThread will run
+    // Tasks from Sequences returned by |get_work_callback_| until the callback
+    // returns nullptr. Resetting |wake_up_event_| here doesn't break this
+    // invariant and avoids a useless loop iteration before going to sleep if
+    // WakeUp() is called while this SchedulerWorkerThread is awake.
+    wake_up_event_.Reset();
+  }
+}
+
+bool SchedulerWorkerThread::ShouldExitForTesting() const {
+  AutoSchedulerLock auto_lock(should_exit_for_testing_lock_);
+  return should_exit_for_testing_;
+}
+
+}  // namespace internal
+}  // namespace base