TaskScheduler [5/9] Task Tracker

base/task_scheduler/task_tracker.cc

Index: base/task_scheduler/task_tracker.cc
diff --git a/base/task_scheduler/task_tracker.cc b/base/task_scheduler/task_tracker.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3069113c6126fbbb047464b49f3cd032c8e6800c
--- /dev/null
+++ b/base/task_scheduler/task_tracker.cc
@@ -0,0 +1,120 @@
+// 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/task_tracker.h"
+
+#include "base/callback.h"
+#include "base/debug/task_annotator.h"
+
+namespace base {
+namespace internal {
+
+namespace {
+const char kQueueFunctionName[] = "base::PostTask";
+}  // namespace
+
+TaskTracker::TaskTracker()
+    : cv_(lock_.CreateConditionVariable()),
+      num_tasks_blocking_shutdown_(0),
+      is_shutting_down_(false),
+      shutdown_completed_(false) {}
+
+TaskTracker::~TaskTracker() = default;
+
+void TaskTracker::Shutdown() {
+  AutoSchedulerLock auto_lock(lock_);
+
+  DCHECK(!shutdown_completed_ && !is_shutting_down_)
+      << "TaskTracker::Shutdown() should only be invoked once.";
+  is_shutting_down_ = true;
+
+  // Wait until the number of tasks blocking shutdown is zero.
+  while (num_tasks_blocking_shutdown_ != 0)
+    cv_->Wait();
+
+  is_shutting_down_ = false;
+  shutdown_completed_ = true;
+}
+
+void TaskTracker::PostTask(
+    const Callback<void(scoped_ptr<Task>)>& post_task_callback,
+    scoped_ptr<Task> task) {
+  DCHECK(!post_task_callback.is_null());
+  DCHECK(task.get());
+
+  if (!RequestPostTask(task->traits.shutdown_behavior()))
+    return;
+
+  debug::TaskAnnotator task_annotator;
+  task_annotator.DidQueueTask(kQueueFunctionName, *task.get());

[gab, 2016/03/01 22:18:47]

Not your fault but noticing that "DidQueueTask" is a weird name as it
intuitively feels it should be called *before* queueing the task (or it may run
before it's called) and this is in fact what existing impls do... so it should
really be "WillPostTask"...

[fdoray, 2016/03/02 00:38:41]

Acknowledged.

+
+  post_task_callback.Run(std::move(task));
+}
+
+void TaskTracker::RunTask(const Task* task) {
+  DCHECK(task);
+
+  const TaskShutdownBehavior shutdown_behavior =
+      task->traits.shutdown_behavior();
+  if (!RequestScheduleTask(shutdown_behavior))
+    return;
+
+  debug::TaskAnnotator task_annotator;
+  task_annotator.RunTask(kQueueFunctionName, *task);
+
+  DidExecuteTask(shutdown_behavior);
+}
+
+bool TaskTracker::RequestPostTask(TaskShutdownBehavior shutdown_behavior) {

[robliao, 2016/03/01 22:29:41]

Now that these are private, we can give them names that reflect when they're
called.

RequestPostTask -> BeforeTaskPosted
RequestScheduleTask -> BeforeTaskExecuted
DidExecuteTask -> AfterTaskExecuted

[fdoray, 2016/03/02 00:38:41]

Done.

+  AutoSchedulerLock auto_lock(lock_);
+
+  if (shutdown_completed_)
+    return false;
+
+  if (shutdown_behavior == TaskShutdownBehavior::BLOCK_SHUTDOWN) {
+    ++num_tasks_blocking_shutdown_;
+    return true;
+  }
+
+  return !is_shutting_down_;
+}
+
+bool TaskTracker::RequestScheduleTask(TaskShutdownBehavior shutdown_behavior) {

[gab, 2016/03/01 22:18:47]

"RequestRunTask" to match nomenclature in rest of this class?

[fdoray, 2016/03/02 00:38:41]

robliao@ suggested different method names. 
RequestPostTask -> BeforeTaskPosted
RequestScheduleTask -> BeforeTaskExecuted
DidExecuteTask -> AfterTaskExecuted

+  AutoSchedulerLock auto_lock(lock_);
+
+  if (shutdown_completed_) {
+    DCHECK_NE(TaskShutdownBehavior::BLOCK_SHUTDOWN, shutdown_behavior);
+    return false;
+  }
+
+  switch (shutdown_behavior) {
+    case TaskShutdownBehavior::BLOCK_SHUTDOWN:
+      return true;
+
+    case TaskShutdownBehavior::SKIP_ON_SHUTDOWN:
+      if (is_shutting_down_)
+        return false;
+      ++num_tasks_blocking_shutdown_;

[robliao, 2016/03/01 22:29:41]

It's worth commented that we want Shutdown to block on these tasks during
execution and we increment here since we don't want to include them in the
overall blocking count unless they're currently executing.

[fdoray, 2016/03/02 00:38:40]

Done.

+      return true;
+
+    case TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN:
+      return !is_shutting_down_;
+  }
+
+  return false;

[robliao, 2016/03/01 22:29:41]

Might as well add a NOTREACHED() here.

[fdoray, 2016/03/02 00:38:41]

Done.

+}
+
+void TaskTracker::DidExecuteTask(TaskShutdownBehavior shutdown_behavior) {
+  if (shutdown_behavior == TaskShutdownBehavior::BLOCK_SHUTDOWN ||
+      shutdown_behavior == TaskShutdownBehavior::SKIP_ON_SHUTDOWN) {
+    AutoSchedulerLock auto_lock(lock_);
+    DCHECK_GT(num_tasks_blocking_shutdown_, 0U);
+    --num_tasks_blocking_shutdown_;
+    if (num_tasks_blocking_shutdown_ == 0 && is_shutting_down_)
+      cv_->Signal();
+  }
+}
+
+}  // namespace internal
+}  // namespace base