Replace Sequence lock with Sequence::Transaction class

Bug: 889029
Change-Id: Ia8ab74329953568bda37a324cc3a9b477acf732e
Reviewed-on: https://chromium-review.googlesource.com/c/1327548
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606684}

base/task/task_scheduler/platform_native_worker_pool_win.cc

@@ -100,7 +100,8 @@ scoped_refptr<Sequence> PlatformNativeWorkerPoolWin::GetWork() {
 
 void PlatformNativeWorkerPoolWin::OnCanScheduleSequence(
     scoped_refptr<Sequence> sequence) {
-  const SequenceSortKey sequence_sort_key = sequence->GetSortKey();
+  const SequenceSortKey sequence_sort_key =
+      sequence->BeginTransaction()->GetSortKey();
   auto transaction(priority_queue_.BeginTransaction());
 
   transaction->Push(std::move(sequence), sequence_sort_key);

base/task/task_scheduler/priority_queue_unittest.cc

@@ -59,23 +59,27 @@ TEST(TaskSchedulerPriorityQueueTest, PushPopPeek) {
   // Create test sequences.
   scoped_refptr<Sequence> sequence_a =
       MakeRefCounted<Sequence>(TaskTraits(TaskPriority::USER_VISIBLE));
-  sequence_a->PushTask(Task(FROM_HERE, DoNothing(), TimeDelta()));
-  SequenceSortKey sort_key_a = sequence_a->GetSortKey();
+  sequence_a->BeginTransaction()->PushTask(
+      Task(FROM_HERE, DoNothing(), TimeDelta()));
+  SequenceSortKey sort_key_a = sequence_a->BeginTransaction()->GetSortKey();
 
   scoped_refptr<Sequence> sequence_b =
       MakeRefCounted<Sequence>(TaskTraits(TaskPriority::USER_BLOCKING));
-  sequence_b->PushTask(Task(FROM_HERE, DoNothing(), TimeDelta()));
-  SequenceSortKey sort_key_b = sequence_b->GetSortKey();
+  sequence_b->BeginTransaction()->PushTask(
+      Task(FROM_HERE, DoNothing(), TimeDelta()));
+  SequenceSortKey sort_key_b = sequence_b->BeginTransaction()->GetSortKey();
 
   scoped_refptr<Sequence> sequence_c =
       MakeRefCounted<Sequence>(TaskTraits(TaskPriority::USER_BLOCKING));
-  sequence_c->PushTask(Task(FROM_HERE, DoNothing(), TimeDelta()));
-  SequenceSortKey sort_key_c = sequence_c->GetSortKey();
+  sequence_c->BeginTransaction()->PushTask(
+      Task(FROM_HERE, DoNothing(), TimeDelta()));
+  SequenceSortKey sort_key_c = sequence_c->BeginTransaction()->GetSortKey();
 
   scoped_refptr<Sequence> sequence_d =
       MakeRefCounted<Sequence>(TaskTraits(TaskPriority::BEST_EFFORT));
-  sequence_d->PushTask(Task(FROM_HERE, DoNothing(), TimeDelta()));
-  SequenceSortKey sort_key_d = sequence_d->GetSortKey();
+  sequence_d->BeginTransaction()->PushTask(
+      Task(FROM_HERE, DoNothing(), TimeDelta()));
+  SequenceSortKey sort_key_d = sequence_d->BeginTransaction()->GetSortKey();
 
   // Create a PriorityQueue and a Transaction.
   PriorityQueue pq;

base/task/task_scheduler/scheduler_single_thread_task_runner_manager.cc

@@ -112,7 +112,8 @@ class SchedulerWorkerDelegate : public SchedulerWorker::Delegate {
 
   void ReEnqueueSequence(scoped_refptr<Sequence> sequence) override {
     DCHECK(sequence);
-    const SequenceSortKey sequence_sort_key = sequence->GetSortKey();
+    const SequenceSortKey sequence_sort_key =
+        sequence->BeginTransaction()->GetSortKey();
     std::unique_ptr<PriorityQueue::Transaction> transaction(
         priority_queue_.BeginTransaction());
     transaction->Push(std::move(sequence), sequence_sort_key);
@@ -223,8 +224,8 @@ class SchedulerWorkerCOMDelegate : public SchedulerWorkerDelegate {
                              TimeDelta());
       if (task_tracker_->WillPostTask(&pump_message_task,
                                       TaskShutdownBehavior::SKIP_ON_SHUTDOWN)) {
-        bool was_empty =
-            message_pump_sequence_->PushTask(std::move(pump_message_task));
+        bool was_empty = message_pump_sequence_->BeginTransaction()->PushTask(
+            std::move(pump_message_task));
         DCHECK(was_empty) << "GetWorkFromWindowsMessageQueue() does not expect "
                              "queueing of pump tasks.";
         return message_pump_sequence_;
@@ -330,7 +331,8 @@ class SchedulerSingleThreadTaskRunnerManager::SchedulerSingleThreadTaskRunner
   }
 
   void PostTaskNow(Task task) {
-    const bool sequence_was_empty = sequence_->PushTask(std::move(task));
+    const bool sequence_was_empty =
+        sequence_->BeginTransaction()->PushTask(std::move(task));
     if (sequence_was_empty) {
       if (outer_->task_tracker_->WillScheduleSequence(sequence_,
                                                       GetDelegate())) {

base/task/task_scheduler/scheduler_worker_pool.cc

@@ -59,7 +59,8 @@ void SchedulerWorkerPool::PostTaskWithSequenceNow(
   // in the past).
   DCHECK_LE(task.delayed_run_time, TimeTicks::Now());
 
-  const bool sequence_was_empty = sequence->PushTask(std::move(task));
+  const bool sequence_was_empty =
+      sequence->BeginTransaction()->PushTask(std::move(task));
   if (sequence_was_empty) {
     // Try to schedule |sequence| if it was empty before |task| was inserted
     // into it. Otherwise, one of these must be true:

base/task/task_scheduler/scheduler_worker_pool_impl.cc

@@ -280,7 +280,7 @@ void SchedulerWorkerPoolImpl::OnCanScheduleSequence(
 void SchedulerWorkerPoolImpl::PushSequenceToPriorityQueue(
     scoped_refptr<Sequence> sequence) {
   DCHECK(sequence);
-  const auto sequence_sort_key = sequence->GetSortKey();
+  const auto sequence_sort_key = sequence->BeginTransaction()->GetSortKey();
   shared_priority_queue_.BeginTransaction()->Push(std::move(sequence),
                                                   sequence_sort_key);
 }

base/task/task_scheduler/scheduler_worker_unittest.cc

@@ -188,7 +188,7 @@ class TaskSchedulerWorkerTest : public testing::TestWithParam<size_t> {
                   TimeDelta());
         EXPECT_TRUE(outer_->task_tracker_.WillPostTask(
             &task, sequence->traits().shutdown_behavior()));
-        sequence->PushTask(std::move(task));
+        sequence->BeginTransaction()->PushTask(std::move(task));
       }
 
       ExpectCallToDidRunTask();
@@ -221,8 +221,10 @@ class TaskSchedulerWorkerTest : public testing::TestWithParam<size_t> {
 
       // Verify that |sequence| contains TasksPerSequence() - 1 Tasks.
       for (size_t i = 0; i < outer_->TasksPerSequence() - 1; ++i) {
-        EXPECT_TRUE(sequence->TakeTask());
-        EXPECT_EQ(i == outer_->TasksPerSequence() - 2, sequence->Pop());
+        std::unique_ptr<Sequence::Transaction> transaction =
+            sequence->BeginTransaction();
+        EXPECT_TRUE(transaction->TakeTask());
+        EXPECT_EQ(i == outer_->TasksPerSequence() - 2, transaction->Pop());
       }
 
       // Add |sequence| to |re_enqueued_sequences_|.
@@ -455,7 +457,7 @@ class ControllableCleanupDelegate : public SchedulerWorkerDefaultDelegate {
         TimeDelta());
     EXPECT_TRUE(task_tracker_->WillPostTask(
         &task, sequence->traits().shutdown_behavior()));
-    sequence->PushTask(std::move(task));
+    sequence->BeginTransaction()->PushTask(std::move(task));
     sequence =
         task_tracker_->WillScheduleSequence(std::move(sequence), nullptr);
     EXPECT_TRUE(sequence);

base/task/task_scheduler/sequence.cc

@@ -8,71 +8,80 @@
 
 #include "base/critical_closure.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/time/time.h"
 
 namespace base {
 namespace internal {
 
-Sequence::Sequence(
-    const TaskTraits& traits,
-    scoped_refptr<SchedulerParallelTaskRunner> scheduler_parallel_task_runner)
-    : traits_(traits),
-      scheduler_parallel_task_runner_(scheduler_parallel_task_runner) {}
+Sequence::Transaction::Transaction(scoped_refptr<Sequence> sequence)
+    : sequence_(sequence) {
+  sequence_->lock_.Acquire();
+}
+
+Sequence::Transaction::~Transaction() {
+  sequence_->lock_.AssertAcquired();
+  sequence_->lock_.Release();
+}
 
-bool Sequence::PushTask(Task task) {
+bool Sequence::Transaction::PushTask(Task task) {
   // Use CHECK instead of DCHECK to crash earlier. See http://crbug.com/711167
   // for details.
   CHECK(task.task);
   DCHECK(task.sequenced_time.is_null());
   task.sequenced_time = base::TimeTicks::Now();
 
-  task.task =
-      traits_.shutdown_behavior() == TaskShutdownBehavior::BLOCK_SHUTDOWN
-          ? MakeCriticalClosure(std::move(task.task))
-          : std::move(task.task);
+  task.task = sequence_->traits_.shutdown_behavior() ==
+                      TaskShutdownBehavior::BLOCK_SHUTDOWN
+                  ? MakeCriticalClosure(std::move(task.task))
+                  : std::move(task.task);
 
-  AutoSchedulerLock auto_lock(lock_);
-  queue_.push(std::move(task));
+  sequence_->queue_.push(std::move(task));
 
   // Return true if the sequence was empty before the push.
-  return queue_.size() == 1;
+  return sequence_->queue_.size() == 1;
 }
 
-Optional<Task> Sequence::TakeTask() {
-  AutoSchedulerLock auto_lock(lock_);
-  DCHECK(!queue_.empty());
-  DCHECK(queue_.front().task);
+Optional<Task> Sequence::Transaction::TakeTask() {
+  DCHECK(!sequence_->queue_.empty());
+  DCHECK(sequence_->queue_.front().task);
 
-  return std::move(queue_.front());
+  return std::move(sequence_->queue_.front());
 }
 
-bool Sequence::Pop() {
-  AutoSchedulerLock auto_lock(lock_);
-  DCHECK(!queue_.empty());
-  DCHECK(!queue_.front().task);
-  queue_.pop();
-  return queue_.empty();
+bool Sequence::Transaction::Pop() {
+  DCHECK(!sequence_->queue_.empty());
+  DCHECK(!sequence_->queue_.front().task);
+  sequence_->queue_.pop();
+  return sequence_->queue_.empty();
 }
 
-SequenceSortKey Sequence::GetSortKey() const {
-  base::TimeTicks next_task_sequenced_time;
-
-  {
-    AutoSchedulerLock auto_lock(lock_);
-    DCHECK(!queue_.empty());
+SequenceSortKey Sequence::Transaction::GetSortKey() const {
+  DCHECK(!sequence_->queue_.empty());
 
-    // Save the sequenced time of the next task in the sequence.
-    next_task_sequenced_time = queue_.front().sequenced_time;
-  }
+  // Save the sequenced time of the next task in the sequence.
+  base::TimeTicks next_task_sequenced_time =
+      sequence_->queue_.front().sequenced_time;
 
-  return SequenceSortKey(traits_.priority(), next_task_sequenced_time);
+  return SequenceSortKey(sequence_->traits_.priority(),
+                         next_task_sequenced_time);
 }
 
+Sequence::Sequence(
+    const TaskTraits& traits,
+    scoped_refptr<SchedulerParallelTaskRunner> scheduler_parallel_task_runner)
+    : traits_(traits),
+      scheduler_parallel_task_runner_(scheduler_parallel_task_runner) {}
+
 Sequence::~Sequence() {
   if (scheduler_parallel_task_runner_) {
     scheduler_parallel_task_runner_->UnregisterSequence(this);
   }
 }
 
+std::unique_ptr<Sequence::Transaction> Sequence::BeginTransaction() {
+  return WrapUnique(new Transaction(this));
+}
+
 }  // namespace internal
 }  // namespace base

base/task/task_scheduler/sequence.h

@@ -43,6 +43,48 @@ namespace internal {
 // This class is thread-safe.
 class BASE_EXPORT Sequence : public RefCountedThreadSafe<Sequence> {
  public:
+  // A Transaction can perform multiple operations atomically on a
+  // Sequence. While a Transaction is alive, it is guaranteed that nothing
+  // else will access the Sequence; the Sequence's lock is held for the
+  // lifetime of the Transaction.
+  class BASE_EXPORT Transaction {
+   public:
+    ~Transaction();
+
+    // Adds |task| in a new slot at the end of the Sequence. Returns true if the
+    // Sequence was empty before this operation.
+    bool PushTask(Task task);
+
+    // Transfers ownership of the Task in the front slot of the Sequence to the
+    // caller. The front slot of the Sequence will be nullptr and remain until
+    // Pop(). Cannot be called on an empty Sequence or a Sequence whose front
+    // slot is already nullptr.
+    //
+    // Because this method cannot be called on an empty Sequence, the returned
+    // Optional<Task> is never nullptr. An Optional is used in preparation for
+    // the merge between TaskScheduler and TaskQueueManager (in Blink).
+    // https://crbug.com/783309
+    Optional<Task> TakeTask();
+
+    // 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;
+
+   private:
+    friend class Sequence;
+
+    explicit Transaction(scoped_refptr<Sequence> sequence);
+
+    const scoped_refptr<Sequence> sequence_;
+
+    DISALLOW_COPY_AND_ASSIGN(Transaction);
+  };
+
   // |traits| is metadata that applies to all Tasks in the Sequence.
   // |scheduler_parallel_task_runner| is a reference to the
   // SchedulerParallelTaskRunner that created this Sequence, if any.
@@ -50,29 +92,9 @@ class BASE_EXPORT Sequence : public RefCountedThreadSafe<Sequence> {
            scoped_refptr<SchedulerParallelTaskRunner>
                scheduler_parallel_task_runner = nullptr);
 
-  // Adds |task| in a new slot at the end of the Sequence. Returns true if the
-  // Sequence was empty before this operation.
-  bool PushTask(Task task);
-
-  // Transfers ownership of the Task in the front slot of the Sequence to the
-  // caller. The front slot of the Sequence will be nullptr and remain until
-  // Pop(). Cannot be called on an empty Sequence or a Sequence whose front slot
-  // is already nullptr.
-  //
-  // Because this method cannot be called on an empty Sequence, the returned
-  // Optional<Task> is never nullptr. An Optional is used in preparation for the
-  // merge between TaskScheduler and TaskQueueManager (in Blink).
-  // https://crbug.com/783309
-  Optional<Task> TakeTask();
-
-  // 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;
+  // Begins a Transaction. This method cannot be called on a thread which has an
+  // active Sequence::Transaction.
+  std::unique_ptr<Transaction> BeginTransaction();
 
   // Returns a token that uniquely identifies this Sequence.
   const SequenceToken& token() const { return token_; }

base/task/task_scheduler/sequence_unittest.cc

@@ -44,72 +44,74 @@ TEST(TaskSchedulerSequenceTest, PushTakeRemove) {
   testing::StrictMock<MockTask> mock_task_d;
   testing::StrictMock<MockTask> mock_task_e;
 
-  scoped_refptr<Sequence> sequence =
-      MakeRefCounted<Sequence>(TaskTraits(TaskPriority::BEST_EFFORT));
+  std::unique_ptr<Sequence::Transaction> transaction =
+      MakeRefCounted<Sequence>(TaskTraits(TaskPriority::BEST_EFFORT))
+          ->BeginTransaction();
 
   // Push task A in the sequence. PushTask() should return true since it's the
   // first task->
-  EXPECT_TRUE(sequence->PushTask(CreateTask(&mock_task_a)));
+  EXPECT_TRUE(transaction->PushTask(CreateTask(&mock_task_a)));
 
   // Push task B, C and D in the sequence. PushTask() should return false since
   // there is already a task in a sequence.
-  EXPECT_FALSE(sequence->PushTask(CreateTask(&mock_task_b)));
-  EXPECT_FALSE(sequence->PushTask(CreateTask(&mock_task_c)));
-  EXPECT_FALSE(sequence->PushTask(CreateTask(&mock_task_d)));
+  EXPECT_FALSE(transaction->PushTask(CreateTask(&mock_task_b)));
+  EXPECT_FALSE(transaction->PushTask(CreateTask(&mock_task_c)));
+  EXPECT_FALSE(transaction->PushTask(CreateTask(&mock_task_d)));
 
   // Take the task in front of the sequence. It should be task A.
-  Optional<Task> task = sequence->TakeTask();
+  Optional<Task> task = transaction->TakeTask();
   ExpectMockTask(&mock_task_a, &task.value());
   EXPECT_FALSE(task->sequenced_time.is_null());
 
   // Remove the empty slot. Task B should now be in front.
-  EXPECT_FALSE(sequence->Pop());
-  task = sequence->TakeTask();
+  EXPECT_FALSE(transaction->Pop());
+  task = transaction->TakeTask();
   ExpectMockTask(&mock_task_b, &task.value());
   EXPECT_FALSE(task->sequenced_time.is_null());
 
   // Remove the empty slot. Task C should now be in front.
-  EXPECT_FALSE(sequence->Pop());
-  task = sequence->TakeTask();
+  EXPECT_FALSE(transaction->Pop());
+  task = transaction->TakeTask();
   ExpectMockTask(&mock_task_c, &task.value());
   EXPECT_FALSE(task->sequenced_time.is_null());
 
   // Remove the empty slot.
-  EXPECT_FALSE(sequence->Pop());
+  EXPECT_FALSE(transaction->Pop());
 
   // Push task E in the sequence.
-  EXPECT_FALSE(sequence->PushTask(CreateTask(&mock_task_e)));
+  EXPECT_FALSE(transaction->PushTask(CreateTask(&mock_task_e)));
 
   // Task D should be in front.
-  task = sequence->TakeTask();
+  task = transaction->TakeTask();
   ExpectMockTask(&mock_task_d, &task.value());
   EXPECT_FALSE(task->sequenced_time.is_null());
 
   // Remove the empty slot. Task E should now be in front.
-  EXPECT_FALSE(sequence->Pop());
-  task = sequence->TakeTask();
+  EXPECT_FALSE(transaction->Pop());
+  task = transaction->TakeTask();
   ExpectMockTask(&mock_task_e, &task.value());
   EXPECT_FALSE(task->sequenced_time.is_null());
 
   // Remove the empty slot. The sequence should now be empty.
-  EXPECT_TRUE(sequence->Pop());
+  EXPECT_TRUE(transaction->Pop());
 }
 
 // Verifies the sort key of a BEST_EFFORT sequence that contains one task.
 TEST(TaskSchedulerSequenceTest, GetSortKeyBestEffort) {
   // Create a BEST_EFFORT sequence with a task.
   Task best_effort_task(FROM_HERE, DoNothing(), TimeDelta());
-  scoped_refptr<Sequence> best_effort_sequence =
-      MakeRefCounted<Sequence>(TaskTraits(TaskPriority::BEST_EFFORT));
-  best_effort_sequence->PushTask(std::move(best_effort_task));
+  std::unique_ptr<Sequence::Transaction> best_effort_sequence_transaction =
+      MakeRefCounted<Sequence>(TaskTraits(TaskPriority::BEST_EFFORT))
+          ->BeginTransaction();
+  best_effort_sequence_transaction->PushTask(std::move(best_effort_task));
 
   // Get the sort key.
   const SequenceSortKey best_effort_sort_key =
-      best_effort_sequence->GetSortKey();
+      best_effort_sequence_transaction->GetSortKey();
 
   // Take the task from the sequence, so that its sequenced time is available
   // for the check below.
-  auto take_best_effort_task = best_effort_sequence->TakeTask();
+  auto take_best_effort_task = best_effort_sequence_transaction->TakeTask();
 
   // Verify the sort key.
   EXPECT_EQ(TaskPriority::BEST_EFFORT, best_effort_sort_key.priority());
@@ -117,7 +119,7 @@ TEST(TaskSchedulerSequenceTest, GetSortKeyBestEffort) {
             best_effort_sort_key.next_task_sequenced_time());
 
   // Pop for correctness.
-  best_effort_sequence->Pop();
+  best_effort_sequence_transaction->Pop();
 }
 
 // Same as TaskSchedulerSequenceTest.GetSortKeyBestEffort, but with a
@@ -125,16 +127,18 @@ TEST(TaskSchedulerSequenceTest, GetSortKeyBestEffort) {
 TEST(TaskSchedulerSequenceTest, GetSortKeyForeground) {
   // Create a USER_VISIBLE sequence with a task.
   Task foreground_task(FROM_HERE, DoNothing(), TimeDelta());
-  scoped_refptr<Sequence> foreground_sequence =
-      MakeRefCounted<Sequence>(TaskTraits(TaskPriority::USER_VISIBLE));
-  foreground_sequence->PushTask(std::move(foreground_task));
+  std::unique_ptr<Sequence::Transaction> foreground_sequence_transaction =
+      MakeRefCounted<Sequence>(TaskTraits(TaskPriority::USER_VISIBLE))
+          ->BeginTransaction();
+  foreground_sequence_transaction->PushTask(std::move(foreground_task));
 
   // Get the sort key.
-  const SequenceSortKey foreground_sort_key = foreground_sequence->GetSortKey();
+  const SequenceSortKey foreground_sort_key =
+      foreground_sequence_transaction->GetSortKey();
 
   // Take the task from the sequence, so that its sequenced time is available
   // for the check below.
-  auto take_foreground_task = foreground_sequence->TakeTask();
+  auto take_foreground_task = foreground_sequence_transaction->TakeTask();
 
   // Verify the sort key.
   EXPECT_EQ(TaskPriority::USER_VISIBLE, foreground_sort_key.priority());
@@ -142,32 +146,35 @@ TEST(TaskSchedulerSequenceTest, GetSortKeyForeground) {
             foreground_sort_key.next_task_sequenced_time());
 
   // Pop for correctness.
-  foreground_sequence->Pop();
+  foreground_sequence_transaction->Pop();
 }
 
 // Verify that a DCHECK fires if Pop() is called on a sequence whose front slot
 // isn't empty.
 TEST(TaskSchedulerSequenceTest, PopNonEmptyFrontSlot) {
-  scoped_refptr<Sequence> sequence = MakeRefCounted<Sequence>(TaskTraits());
-  sequence->PushTask(Task(FROM_HERE, DoNothing(), TimeDelta()));
+  std::unique_ptr<Sequence::Transaction> transaction =
+      MakeRefCounted<Sequence>(TaskTraits())->BeginTransaction();
+  transaction->PushTask(Task(FROM_HERE, DoNothing(), TimeDelta()));
 
-  EXPECT_DCHECK_DEATH({ sequence->Pop(); });
+  EXPECT_DCHECK_DEATH({ transaction->Pop(); });
 }
 
 // Verify that a DCHECK fires if TakeTask() is called on a sequence whose front
 // slot is empty.
 TEST(TaskSchedulerSequenceTest, TakeEmptyFrontSlot) {
-  scoped_refptr<Sequence> sequence = MakeRefCounted<Sequence>(TaskTraits());
-  sequence->PushTask(Task(FROM_HERE, DoNothing(), TimeDelta()));
+  std::unique_ptr<Sequence::Transaction> transaction =
+      MakeRefCounted<Sequence>(TaskTraits())->BeginTransaction();
+  transaction->PushTask(Task(FROM_HERE, DoNothing(), TimeDelta()));
 
-  EXPECT_TRUE(sequence->TakeTask());
-  EXPECT_DCHECK_DEATH({ sequence->TakeTask(); });
+  EXPECT_TRUE(transaction->TakeTask());
+  EXPECT_DCHECK_DEATH({ transaction->TakeTask(); });
 }
 
 // Verify that a DCHECK fires if TakeTask() is called on an empty sequence.
 TEST(TaskSchedulerSequenceTest, TakeEmptySequence) {
-  scoped_refptr<Sequence> sequence = MakeRefCounted<Sequence>(TaskTraits());
-  EXPECT_DCHECK_DEATH({ sequence->TakeTask(); });
+  std::unique_ptr<Sequence::Transaction> transaction =
+      MakeRefCounted<Sequence>(TaskTraits())->BeginTransaction();
+  EXPECT_DCHECK_DEATH({ transaction->TakeTask(); });
 }
 
 }  // namespace internal

base/task/task_scheduler/task_tracker.cc

@@ -457,7 +457,7 @@ scoped_refptr<Sequence> TaskTracker::WillScheduleSequence(
     scoped_refptr<Sequence> sequence,
     CanScheduleSequenceObserver* observer) {
   DCHECK(sequence);
-  const SequenceSortKey sort_key = sequence->GetSortKey();
+  const SequenceSortKey sort_key = sequence->BeginTransaction()->GetSortKey();
   const int priority_index = static_cast<int>(sort_key.priority());
 
   AutoSchedulerLock auto_lock(preemption_state_[priority_index].lock);
@@ -483,7 +483,7 @@ scoped_refptr<Sequence> TaskTracker::RunAndPopNextTask(
   DCHECK(sequence);
 
   // Run the next task in |sequence|.
-  Optional<Task> task = sequence->TakeTask();
+  Optional<Task> task = sequence->BeginTransaction()->TakeTask();
   // TODO(fdoray): Support TakeTask() returning null. https://crbug.com/783309
   DCHECK(task);
 
@@ -502,7 +502,7 @@ scoped_refptr<Sequence> TaskTracker::RunAndPopNextTask(
   if (task->delayed_run_time.is_null())
     DecrementNumIncompleteUndelayedTasks();
 
-  const bool sequence_is_empty_after_pop = sequence->Pop();
+  const bool sequence_is_empty_after_pop = sequence->BeginTransaction()->Pop();
   const TaskPriority priority = sequence->traits().priority();
 
   // Never reschedule a Sequence emptied by Pop(). The contract is such that
@@ -870,9 +870,10 @@ scoped_refptr<Sequence> TaskTracker::ManageSequencesAfterRunningTask(
     CanScheduleSequenceObserver* observer,
     TaskPriority task_priority) {
   const TimeTicks next_task_sequenced_time =
-      just_ran_sequence
-          ? just_ran_sequence->GetSortKey().next_task_sequenced_time()
-          : TimeTicks();
+      just_ran_sequence ? just_ran_sequence->BeginTransaction()
+                              ->GetSortKey()
+                              .next_task_sequenced_time()
+                        : TimeTicks();
   PreemptedSequence sequence_to_schedule;
   int priority_index = static_cast<int>(task_priority);
 

base/task/task_scheduler/task_tracker_unittest.cc

@@ -898,7 +898,7 @@ TEST_F(TaskSchedulerTaskTrackerTest, CurrentSequenceToken) {
   Task task(FROM_HERE, Bind(&ExpectSequenceToken, sequence_token), TimeDelta());
   tracker_.WillPostTask(&task, sequence->traits().shutdown_behavior());
 
-  sequence->PushTask(std::move(task));
+  sequence->BeginTransaction()->PushTask(std::move(task));
 
   EXPECT_FALSE(SequenceToken::GetForCurrentThread().IsValid());
   sequence = tracker_.WillScheduleSequence(std::move(sequence),
@@ -1079,7 +1079,7 @@ TEST_F(TaskSchedulerTaskTrackerTest,
 
   scoped_refptr<Sequence> sequence =
       test::CreateSequenceWithTask(std::move(task_1), default_traits);
-  sequence->PushTask(std::move(task_2));
+  sequence->BeginTransaction()->PushTask(std::move(task_2));
   EXPECT_EQ(sequence, tracker_.WillScheduleSequence(sequence, nullptr));
 
   EXPECT_EQ(sequence, tracker_.RunAndPopNextTask(sequence, nullptr));
@@ -1227,7 +1227,7 @@ TEST_F(TaskSchedulerTaskTrackerTest,
   TaskTraits best_effort_traits = TaskTraits(TaskPriority::BEST_EFFORT);
   EXPECT_TRUE(
       tracker_.WillPostTask(&task_b_2, best_effort_traits.shutdown_behavior()));
-  sequence_b->PushTask(std::move(task_b_2));
+  sequence_b->BeginTransaction()->PushTask(std::move(task_b_2));
   testing::StrictMock<MockCanScheduleSequenceObserver> observer_b_2;
   EXPECT_EQ(nullptr, tracker_.WillScheduleSequence(sequence_b, &observer_b_2));
   // The TaskPriority of the Sequence is unchanged by posting new tasks to it.
@@ -1335,7 +1335,7 @@ TEST_F(TaskSchedulerTaskTrackerTest,
                 TimeDelta());
   EXPECT_TRUE(
       tracker.WillPostTask(&task_a_2, best_effort_traits.shutdown_behavior()));
-  sequence_a->PushTask(std::move(task_a_2));
+  sequence_a->BeginTransaction()->PushTask(std::move(task_a_2));
 
   // Run the first task in |sequence_a|. RunAndPopNextTask() should return
   // nullptr since |sequence_a| can't be rescheduled immediately.

base/task/task_scheduler/test_utils.cc

@@ -21,7 +21,7 @@ MockSchedulerWorkerObserver::~MockSchedulerWorkerObserver() = default;
 scoped_refptr<Sequence> CreateSequenceWithTask(Task task,
                                                const TaskTraits& traits) {
   scoped_refptr<Sequence> sequence = MakeRefCounted<Sequence>(traits);
-  sequence->PushTask(std::move(task));
+  sequence->BeginTransaction()->PushTask(std::move(task));
   return sequence;
 }