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

base/task_scheduler/sequence.cc

Index: base/task_scheduler/sequence.cc
diff --git a/base/task_scheduler/sequence.cc b/base/task_scheduler/sequence.cc
index 4ecb60568cb88a6c013a3fb23d94c629fa22135e..c4619d4be4a1d34e64f9b7b8d63566e4b8c1d01d 100644
--- a/base/task_scheduler/sequence.cc
+++ b/base/task_scheduler/sequence.cc
@@ -36,6 +36,11 @@ const Task* Sequence::PeekTask() const {
 }
 
 bool Sequence::PopTask() {
+  // Delete the popped task outside the scope of |lock_|. That prevents a double

[gab, 2016/07/20 20:34:51]

s/That/This/

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

Done.

+  // acquisition of |lock_| if the task's destructor tries to post a task to
+  // this Sequence and reduces contention.
+  std::unique_ptr<Task> delete_outside_lock_scope;
+
   AutoSchedulerLock auto_lock(lock_);

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

This would be more robust and clearer if AutoSchedulerLock and associated
statements were in its own block. You'd need to store the value of
queue_.empty() in a temporary. 

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

Done.

   DCHECK(!queue_.empty());
 
@@ -44,6 +49,7 @@ bool Sequence::PopTask() {
   DCHECK_GT(num_tasks_per_priority_[priority_index], 0U);
   --num_tasks_per_priority_[priority_index];
 
+  delete_outside_lock_scope = std::move(queue_.front());
   queue_.pop();
   return queue_.empty();
 }