sequence_manager: Fix deadlock when posting delayed tasks from non-main thread

If a delayed task is posted from a non-main thread and task queue time
recording is enabled, we attempt to grab the "any thread" lock twice,
leading to a deadlock. This patch reduces the scope of the lock to avoid
the problem.

Original fix from Alexander Timin <altimin@chromium.org>.

Bug: 891670
Change-Id: I5af0c2016a12b061c523b8b51c16ff23a8f3a41f
Reviewed-on: https://chromium-review.googlesource.com/c/1344144Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610000}

base/task/sequence_manager/sequence_manager_impl_unittest.cc

@@ -4044,6 +4044,33 @@ TEST_P(SequenceManagerTest, GetPendingTaskCountForTesting) {
   EXPECT_EQ(0u, manager_->GetPendingTaskCountForTesting());
 }
 
+TEST_P(SequenceManagerTest, PostDelayedTaskFromOtherThread) {
+  scoped_refptr<TestTaskQueue> main_tq = CreateTaskQueue();
+  scoped_refptr<TaskRunner> task_runner =
+      main_tq->CreateTaskRunner(kTaskTypeNone);
+  manager_->SetAddQueueTimeToTasks(true);
+
+  Thread thread("test thread");
+  thread.StartAndWaitForTesting();
+
+  WaitableEvent task_posted(WaitableEvent::ResetPolicy::MANUAL,
+                            WaitableEvent::InitialState::NOT_SIGNALED);
+  thread.task_runner()->PostTask(
+      FROM_HERE, BindOnce(
+                     [](scoped_refptr<TaskRunner> task_runner,
+                        WaitableEvent* task_posted) {
+                       task_runner->PostDelayedTask(
+                           FROM_HERE, BindOnce(&NopTask),
+                           base::TimeDelta::FromMilliseconds(10));
+                       task_posted->Signal();
+                     },
+                     std::move(task_runner), &task_posted));
+  task_posted.Wait();
+  test_task_runner_->FastForwardUntilNoTasksRemain();
+  RunLoop().RunUntilIdle();
+  thread.Stop();
+}
+
 }  // namespace sequence_manager_impl_unittest
 }  // namespace internal
 }  // namespace sequence_manager

base/task/sequence_manager/task_queue_impl.cc

@@ -261,16 +261,19 @@ void TaskQueueImpl::PostDelayedTaskImpl(PostedTask task,
     // be common. This pathway is less optimal than perhaps it could be
     // because it causes two main thread tasks to be run.  Should this
     // assumption prove to be false in future, we may need to revisit this.
-    AutoLock lock(any_thread_lock_);
     EnqueueOrder sequence_number = sequence_manager_->GetNextSequenceNumber();
 
-    TimeTicks time_domain_now = any_thread().time_domain->Now();
+    TimeTicks time_domain_now;
+    {
+      AutoLock lock(any_thread_lock_);
+      time_domain_now = any_thread().time_domain->Now();
+    }
     TimeTicks time_domain_delayed_run_time = time_domain_now + task.delay;
     if (sequence_manager_->GetAddQueueTimeToTasks()) {
       task.queue_time = time_domain_now;
     }
 
-    PushOntoDelayedIncomingQueueLocked(
+    PushOntoDelayedIncomingQueue(
         Task(std::move(task), time_domain_delayed_run_time, sequence_number,
              EnqueueOrder(), resolution));
   }
@@ -290,7 +293,7 @@ void TaskQueueImpl::PushOntoDelayedIncomingQueueFromMainThread(
   TraceQueueSize();
 }
 
-void TaskQueueImpl::PushOntoDelayedIncomingQueueLocked(Task pending_task) {
+void TaskQueueImpl::PushOntoDelayedIncomingQueue(Task pending_task) {
   sequence_manager_->WillQueueTask(&pending_task);
 
   // TODO(altimin): Add a copy method to Task to capture metadata here.

base/task/sequence_manager/task_queue_impl.h

@@ -339,7 +339,7 @@ class BASE_EXPORT TaskQueueImpl {
 
   // Push the task onto the |delayed_incoming_queue|.  Slow path from other
   // threads.
-  void PushOntoDelayedIncomingQueueLocked(Task pending_task);
+  void PushOntoDelayedIncomingQueue(Task pending_task);
 
   void ScheduleDelayedWorkTask(Task pending_task);