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

base/task_scheduler/sequence.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_SEQUENCE_H_
 #define BASE_TASK_SCHEDULER_SEQUENCE_H_
 
 #include <stddef.h>
 
 #include <memory>
 #include <queue>
 
 #include "base/base_export.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/sequence_token.h"
 #include "base/task_scheduler/scheduler_lock.h"
 #include "base/task_scheduler/sequence_sort_key.h"
 #include "base/task_scheduler/task.h"
 #include "base/task_scheduler/task_traits.h"
 
 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.

[robliao, 2016/10/07 20:57:11]

A Sequence holds slots each containing up to a single task that must be executed
in posting order.

[fdoray, 2016/10/11 12:29:19]

Done.

+//
+// PushTask() adds a task in a new slot at the end of the Sequence. TakeTask()

[robliao, 2016/10/07 20:57:12]

This duplicates the method comments below. I would remove this.

[fdoray, 2016/10/11 12:29:19]

Done.

+// transfers ownership of the task in the front slot of the Sequence to the
+// caller, leaving the slot empty. Pop() removes the empty slot in front of the
+// Sequence.
+//
+// In comments below, an "empty Sequence" is a Sequence with no slot.
 //
 // Note: there is a known refcounted-ownership cycle in the Scheduler
 // architecture: Sequence -> Task -> TaskRunner -> Sequence -> ...
 // This is okay so long as the other owners of Sequence (PriorityQueue and
 // SchedulerWorker in alternation and
 // SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::GetWork()
 // temporarily) keep running it (and taking Tasks from it as a result). A
 // dangling reference cycle would only occur should they release their reference
 // to it while it's not empty. In other words, it is only correct for them to
 // release it after PopTask() returns false to indicate it was made empty by
 // that call (in which case the next PushTask() will return true to indicate to
 // the caller that the Sequence should be re-enqueued for execution).
 //
 // This class is thread-safe.
 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 was empty before this operation.
   bool PushTask(std::unique_ptr<Task> task);
 
-  // Returns the task in front of the sequence's queue, if any.
-  const Task* PeekTask() const;
+  // Transfers ownership of the Task in the front slot of the Sequence to the
+  // caller. The front slot of the Sequence will be empty until Pop() is

[robliao, 2016/10/07 20:57:12]

Might be clearer to say the "front slot of the Sequence will be nullptr and
remain until Pop()"

[fdoray, 2016/10/11 12:29:19]

Done.

+  // invoked. Cannot be called on an empty Sequence or a Sequence whose front
+  // slot is already empty.
+  std::unique_ptr<Task> TakeTask();
 
-  // 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 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 PeekTaskTraits() const;
 
-  // Returns a SequenceSortKey representing the priority of the sequence. Cannot
-  // be called on an empty sequence.
+  // Removes the front slot of the Sequence. The front slot must have been
+  // emptied by TakeTask() before this is called. Cannot be called on an empty
+  // Sequence. Returns true if the Sequence is empty after this operation.
+  bool Pop();
+
+  // 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.
   const SequenceToken& token() const { return token_; }
 
  private:
   friend class RefCountedThreadSafe<Sequence>;
   ~Sequence();
 
   const SequenceToken token_ = SequenceToken::Create();
 
   // Synchronizes access to all members.
   mutable SchedulerLock lock_;
 
   // 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] =
       {};
 
   DISALLOW_COPY_AND_ASSIGN(Sequence);
 };
 
 }  // namespace internal
 }  // namespace base
 
 #endif  // BASE_TASK_SCHEDULER_SEQUENCE_H_