base: Don't avoid scheduling immediate work inside DoDelayedWork

Some message pumps -- most notably the Mac ones -- don't guarantee that
returning true from DoDelayedWork causes a future call to DoWork to be
scheduled. This patch ensures the optimization to prevent a redundant
call to ScheduleWork from a running task isn't applied for delayed
tasks. This is a speculative fix to some of the Mac hangs that we are
observing with the SequenceManager.

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

base/task/sequence_manager/thread_controller_with_message_pump_impl.cc

@@ -4,6 +4,7 @@
 
 #include "base/task/sequence_manager/thread_controller_with_message_pump_impl.h"
 
+#include "base/auto_reset.h"
 #include "base/message_loop/message_pump.h"
 #include "base/time/tick_clock.h"
 #include "base/trace_event/trace_event.h"
@@ -121,9 +122,13 @@ void ThreadControllerWithMessagePumpImpl::ScheduleWork() {
   if (RunsTasksInCurrentSequence()) {
     // Don't post a DoWork if there's an immediate DoWork in flight or if we're
     // inside a top level DoWork. We can rely on a continuation being posted as
-    // needed.
-    if (main_thread_only().immediate_do_work_posted || InTopLevelDoWork())
+    // needed. We need to avoid this inside DoDelayedWork, however, because
+    // returning true there doesn't guarantee work to get scheduled.
+    // TODO(skyostil@): Simplify this once DoWork/DoDelayedWork get merged.
+    if (main_thread_only().immediate_do_work_posted ||
+        (InTopLevelDoWork() && !main_thread_only().doing_delayed_work)) {
       return;
+    }
     main_thread_only().immediate_do_work_posted = true;
   }
   pump_->ScheduleWork();
@@ -216,6 +221,7 @@ bool ThreadControllerWithMessagePumpImpl::DoWork() {
 
 bool ThreadControllerWithMessagePumpImpl::DoDelayedWork(
     TimeTicks* next_run_time) {
+  AutoReset<bool> delayed_scope(&main_thread_only().doing_delayed_work, true);
   return DoWorkImpl(next_run_time);
 }
 

base/task/sequence_manager/thread_controller_with_message_pump_impl.h

@@ -112,6 +112,10 @@ class BASE_EXPORT ThreadControllerWithMessagePumpImpl
     // Used to prevent redundant calls to ScheduleWork / ScheduleDelayedWork.
     bool immediate_do_work_posted = false;
 
+    // Whether we're currently executing delayed work (as opposed to immediate
+    // work).
+    bool doing_delayed_work = false;
+
     // Number of tasks processed in a single DoWork invocation.
     int work_batch_size = 1;
 

base/task/sequence_manager/thread_controller_with_message_pump_impl_unittest.cc

@@ -350,6 +350,25 @@ TEST_F(ThreadControllerWithMessagePumpTest,
   testing::Mock::VerifyAndClearExpectations(message_pump_);
 }
 
+TEST_F(ThreadControllerWithMessagePumpTest, ScheduleWorkFromDelayedTask) {
+  ThreadTaskRunnerHandle handle(MakeRefCounted<FakeTaskRunner>());
+
+  EXPECT_CALL(*message_pump_, Run(_))
+      .WillOnce(Invoke([](MessagePump::Delegate* delegate) {
+        base::TimeTicks run_time;
+        delegate->DoDelayedWork(&run_time);
+      }));
+  EXPECT_CALL(*message_pump_, ScheduleWork());
+
+  task_source_.AddTask(PendingTask(FROM_HERE, base::BindLambdaForTesting([&]() {
+                                     thread_controller_.ScheduleWork();
+                                   }),
+                                   TimeTicks()));
+  RunLoop().Run();
+
+  testing::Mock::VerifyAndClearExpectations(message_pump_);
+}
+
 TEST_F(ThreadControllerWithMessagePumpTest, SetDefaultTaskRunner) {
   scoped_refptr<SingleThreadTaskRunner> task_runner1 =
       MakeRefCounted<FakeTaskRunner>();