TaskScheduler: Don't delete Tasks in the scope of a Sequence's lock.

base/task_scheduler/sequence_unittest.cc

 // 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/sequence.h"
 
+#include <utility>
+
+#include "base/bind.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 namespace internal {
 
 namespace {
 
+// A class that pushes a Task to a Sequence in its destructor.
+class PushTaskInDestructor {
+ public:
+  explicit PushTaskInDestructor(scoped_refptr<Sequence> sequence)
+      : sequence_(std::move(sequence)) {}
+  PushTaskInDestructor(PushTaskInDestructor&&) = default;
+
+  ~PushTaskInDestructor() {
+    // |sequence_| may be nullptr in a temporary instance of this class.
+    if (sequence_) {
+      EXPECT_FALSE(sequence_->PeekTask());
+      sequence_->PushTask(WrapUnique(
+          new Task(FROM_HERE, Closure(), TaskTraits(), TimeDelta())));
+    }
+  }
+
+  PushTaskInDestructor& operator=(PushTaskInDestructor&&) = default;

[robliao, 2016/07/20 22:39:00]

Group this assignment operator with the constructor above.
https://google.github.io/styleguide/cppguide.html#Declaration_Order

[fdoray, 2016/07/21 13:36:03]

Done.

+
+ private:
+  scoped_refptr<Sequence> sequence_;
+
+  DISALLOW_COPY_AND_ASSIGN(PushTaskInDestructor);
+};
+
+void DoNothing(const PushTaskInDestructor&) {}
+
 class TaskSchedulerSequenceTest : public testing::Test {
  public:
   TaskSchedulerSequenceTest()
       : task_a_owned_(
             new Task(FROM_HERE,
                      Closure(),
                      TaskTraits().WithPriority(TaskPriority::BACKGROUND),
                      TimeDelta())),
         task_b_owned_(
             new Task(FROM_HERE,
@@ skipping 152 matching lines @@
       SequenceSortKey(TaskPriority::USER_BLOCKING, task_d_->sequenced_time),
       sequence->GetSortKey());
 
   // Pop task D. The highest priority is now from task E (BACKGROUND). The
   // task in front of the sequence is now task E.
   sequence->PopTask();
   EXPECT_EQ(SequenceSortKey(TaskPriority::BACKGROUND, task_e_->sequenced_time),
             sequence->GetSortKey());
 }
 
+TEST_F(TaskSchedulerSequenceTest, CanPushTaskInTaskDestructor) {
+  scoped_refptr<Sequence> sequence(new Sequence);
+  sequence->PushTask(WrapUnique(
+      new Task(FROM_HERE, Bind(&DoNothing, PushTaskInDestructor(sequence)),
+               TaskTraits(), TimeDelta())));
+
+  // PushTask() is invoked on |sequence| when the popped Task is destroyed. If
+  // PopTask() destroys the Task outside the scope of its lock as expected, no
+  // deadlock will occur when PushTask() tries to acquire the Sequence's lock.
+  sequence->PopTask();
+
+  // Verify that |sequence| contains exactly one Task.
+  EXPECT_TRUE(sequence->PeekTask());
+  EXPECT_TRUE(sequence->PopTask());
+}
+
 }  // namespace internal
 }  // namespace base