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

base/task_scheduler/sequence_unittest.cc

Index: base/task_scheduler/sequence_unittest.cc
diff --git a/base/task_scheduler/sequence_unittest.cc b/base/task_scheduler/sequence_unittest.cc
index 6a15299e1e588b4ff57ad9b51a6c86df296b7880..53015145ea64c1f0f54b9b80e97a8f94fe6f230b 100644
--- a/base/task_scheduler/sequence_unittest.cc
+++ b/base/task_scheduler/sequence_unittest.cc
@@ -4,7 +4,11 @@
 
 #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"
 
@@ -13,6 +17,32 @@ 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()
@@ -185,5 +215,21 @@ TEST_F(TaskSchedulerSequenceTest, GetSortKey) {
             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