TaskScheduler: Change Sequence::PeekTask to Sequence::TakeTask.

base/task_scheduler/sequence.h

Index: base/task_scheduler/sequence.h
diff --git a/base/task_scheduler/sequence.h b/base/task_scheduler/sequence.h
index 8717336080e623dfde5ce8e5acc108ebcc9e61c0..b4a0aa45f763e31b67da7adfd0ea5c7e0aafb689 100644
--- a/base/task_scheduler/sequence.h
+++ b/base/task_scheduler/sequence.h
@@ -22,7 +22,7 @@
 namespace base {
 namespace internal {
 
-// A sequence holds tasks that must be executed in posting order.
+// A Sequence holds tasks that must be executed in posting order.
 //
 // Note: there is a known refcounted-ownership cycle in the Scheduler
 // architecture: Sequence -> Task -> TaskRunner -> Sequence -> ...
@@ -41,20 +41,29 @@ class BASE_EXPORT Sequence : public RefCountedThreadSafe<Sequence> {
  public:
   Sequence();
 
-  // Adds |task| at the end of the sequence's queue. Returns true if the
-  // sequence was empty before this operation.
+  // Adds |task| in a new slot at the end of the Sequence. Returns true if the
+  // Sequence contained no slot before this operation.

[gab, 2016/10/07 19:51:08]

I still prefer to refer to the Sequence as being "empty" instead of "containing
no slot" (otherwise wording of meta-comment of this class also has to be
reworded)

To me it's clear that empty means that the underlying vector is empty. Could
even clarify that in meta comment and then just use "empty" elsewhere.

[fdoray, 2016/10/07 20:30:30]

Done.

   bool PushTask(std::unique_ptr<Task> task);
 
-  // Returns the task in front of the sequence's queue, if any.
-  const Task* PeekTask() const;
-
-  // Removes the task in front of the sequence's queue. Returns true if the
-  // sequence is empty after this operation. Cannot be called on an empty
-  // sequence.
-  bool PopTask();
-
-  // Returns a SequenceSortKey representing the priority of the sequence. Cannot
-  // be called on an empty sequence.
+  // Transfers ownership of the Task in the front slot of the Sequence to the
+  // caller. If this returns nullptr, the front slot of the Sequence was empty
+  // or the Sequence contained no slot before this operation. If this returns a
+  // Task, the front slot of the Sequence is empty immediately after this

[gab, 2016/10/07 19:51:08]

, the front slot of the Sequence will be empty until RemoveFrontSlot() is
invoked.

[fdoray, 2016/10/07 20:30:30]

Done.

+  // operation.
+  std::unique_ptr<Task> TakeTask();
+
+  // Returns the TaskTraits of the Task in front of the Sequence. Cannot be
+  // called on an empty Sequence or on a Sequence whose front slot is empty.
+  TaskTraits GetFrontTaskTraits() const;

[gab, 2016/10/07 19:51:08]

PeekTaskTraits()?

[fdoray, 2016/10/07 20:30:30]

Done.

+
+  // Removes the slot in front of the Sequence. The front slot must have been
+  // emptied by TakeTask() before this is called. Cannot be called on a Sequence
+  // that contains no slot. Returns true if the Sequence contains no slot
+  // immediately after this operation.
+  bool RemoveFrontSlot();

[gab, 2016/10/07 19:51:08]

I prefer Pop() here, documentation can highlight what it means and why it's void
(well bool).

[fdoray, 2016/10/07 20:30:30]

Done.

+
+  // Returns a SequenceSortKey representing the priority of the Sequence. Cannot
+  // be called on an empty Sequence.
   SequenceSortKey GetSortKey() const;
 
   // Returns a token that uniquely identifies this Sequence.
@@ -72,7 +81,7 @@ class BASE_EXPORT Sequence : public RefCountedThreadSafe<Sequence> {
   // Queue of tasks to execute.
   std::queue<std::unique_ptr<Task>> queue_;
 
-  // Number of tasks contained in the sequence for each priority.
+  // Number of tasks contained in the Sequence for each priority.
   size_t num_tasks_per_priority_[static_cast<int>(TaskPriority::HIGHEST) + 1] =
       {};