TaskScheduler [3/9] Task and Sequence

base/task_scheduler/task.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_TASK_H_
+#define BASE_TASK_SCHEDULER_TASK_H_
+
+#include "base/base_export.h"
+#include "base/callback_forward.h"
+#include "base/location.h"
+#include "base/pending_task.h"
+#include "base/task_scheduler/task_traits.h"
+#include "base/time/time.h"
+
+namespace base {
+namespace internal {
+
+// A task is a unit of work inside the task scheduler.

[gab, 2016/02/18 03:00:46]

Add to the comment that support for tracing is inherited from PendingTask.

[fdoray, 2016/02/18 14:56:13]

Done.

+struct BASE_EXPORT Task : public PendingTask {
+  Task();
+  Task(const tracked_objects::Location& posted_from,
+       const Closure& task,
+       const TaskTraits& traits,
+       const TimeTicks& post_time);

[gab, 2016/02/18 03:00:46]

Comments on constructor to highlight differences.

[fdoray, 2016/02/18 14:56:13]

Default constructor is not needed.

+  ~Task();
+
+  // The traits describing where, when and how the task should run.

[gab, 2016/02/18 03:00:46]

// The TaskTraits of this task.

(what those are is better described in task_traits.h and anybody that cares will
know to look there through the type)

[fdoray, 2016/02/18 14:56:13]

Done.

+  TaskTraits traits;
+
+  // The time at which the task was posted. For a delayed task, this is the time
+  // at which the task was inserted in its sequence after its delay expired; not
+  // the time at which it was initially sent to a task scheduler.
+  TimeTicks post_time;

[fdoray, 2016/02/18 01:46:11]

Is this clearer?

// The time at which the task was inserted in its sequence. For an undelayed
// task, this happens at post time. For a delayed task, happens after the task's
// delay has expired.
TimeTicks inserted_in_sequence_time;

[gab, 2016/02/18 03:00:46]

Actually, for a delayed task isn't this the time at which it was intended to
run? I guess our Sequence doesn't support inserting anywhere but at the back and
expects sorted post times so we can't really do this.

Adding an extra delay is fine for now for BACKGROUND work but won't be later for
precise timer-based work in the renderers; just a thought...

So I guess I'm okay with slight tweaks:

// The time at which the task was inserted in its sequence. For an undelayed
// task, this happens at post time. For a delayed task, this happens some 
// time after the task's delay has expired.
TimeTicks sequenced_time;

Also, how about |sequenced_time|, i.e. 4 phases: post_time <= sequenced_time <=
scheduled_time <= execution_time ? I guess "scheduled" and "execution" are
almost the same (i.e to me scheduled is picked up by a thread and execution
comes almost right away at that point) -- whatever, beyond this discussion but
just mentioning this because I initially thought about |scheduled_time| for this
variable but realized "scheduled" was later.

[fdoray, 2016/02/18 14:56:13]

Done. There's no need to add |scheduled_time| to this struct, right?

[gab, 2016/02/18 20:22:34]

Right, sorry for making this comment so long and confusing... I was thinking
about a name out loud and spewed my reasoning here..!

+};
+
+}  // namespace internal
+}  // namespace base
+
+#endif  // BASE_TASK_SCHEDULER_TASK_H_