Don't call SetNextDelayedDoWork if run_time is in the past (< lazy_now)

Underlying infrastructure might check fail if SetNextDelayedDoWork is
called for a run_time in the past. Which should be expected given that
we have a ScheduleWork method for that case.

ThreadControllerWithMessagePumpImpl checkfails on this.

Bug: 891670
Change-Id: I60f5142eba3ff71babe976c749cec4a504408eed
Reviewed-on: https://chromium-review.googlesource.com/c/1350960
Commit-Queue: Carlos Caballero <carlscab@google.com>
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611394}

base/task/sequence_manager/sequence_manager_impl.cc

@@ -172,20 +172,19 @@ SequenceManagerImpl::CreateOnCurrentThread() {
 
 // static
 std::unique_ptr<SequenceManagerImpl> SequenceManagerImpl::CreateUnbound(
-    MessageLoopBase* message_loop_base) {
+    MessageLoopBase* message_loop_base,
+    const TickClock* clock) {
   return WrapUnique(new SequenceManagerImpl(
-      ThreadControllerImpl::Create(message_loop_base,
-                                   DefaultTickClock::GetInstance()),
+      ThreadControllerImpl::Create(message_loop_base, clock),
       MessageLoop::Type::TYPE_DEFAULT));
 }
 
 // static
 std::unique_ptr<SequenceManagerImpl> SequenceManagerImpl::CreateUnboundWithPump(
-    MessageLoop::Type type) {
+    MessageLoop::Type type,
+    const TickClock* clock) {
   return WrapUnique(new SequenceManagerImpl(
-      ThreadControllerWithMessagePumpImpl::CreateUnbound(
-          DefaultTickClock::GetInstance()),
-      type));
+      ThreadControllerWithMessagePumpImpl::CreateUnbound(clock), type));
 }
 
 void SequenceManagerImpl::BindToMessageLoop(

base/task/sequence_manager/sequence_manager_impl.h

@@ -34,6 +34,7 @@
 #include "base/task/sequence_manager/task_queue_selector.h"
 #include "base/task/sequence_manager/thread_controller.h"
 #include "base/threading/thread_checker.h"
+#include "base/time/default_tick_clock.h"
 #include "build/build_config.h"
 
 namespace base {
@@ -96,10 +97,12 @@ class BASE_EXPORT SequenceManagerImpl
   //
   // This function should be called only once per MessageLoop.
   static std::unique_ptr<SequenceManagerImpl> CreateUnbound(
-      MessageLoopBase* message_loop_base);
+      MessageLoopBase* message_loop_base,
+      const TickClock* clock = DefaultTickClock::GetInstance());
 
   static std::unique_ptr<SequenceManagerImpl> CreateUnboundWithPump(
-      MessageLoop::Type type);
+      MessageLoop::Type type,
+      const TickClock* clock = DefaultTickClock::GetInstance());
 
   // SequenceManager implementation:
   void BindToCurrentThread() override;

base/task/sequence_manager/time_domain.cc

@@ -104,16 +104,20 @@ void TimeDomain::SetNextWakeUpForQueue(
   // SequenceManager's time. Right now it's not an issue since
   // VirtualTimeDomain doesn't invoke SequenceManager itself.
 
-  if (new_wake_up) {
-    if (new_wake_up != previous_wake_up) {
-      // Update the wake-up.
-      SetNextDelayedDoWork(lazy_now, new_wake_up.value());
-    }
+  if (new_wake_up == previous_wake_up) {
+    // Nothing to be done
+    return;
+  }
+
+  if (!new_wake_up) {
+    // No new wake-up to be set, cancel the previous one.
+    new_wake_up = TimeTicks::Max();
+  }
+
+  if (*new_wake_up <= lazy_now->Now()) {
+    RequestDoWork();
   } else {
-    if (previous_wake_up) {
-      // No new wake-up to be set, cancel the previous one.
-      SetNextDelayedDoWork(lazy_now, TimeTicks::Max());
-    }
+    SetNextDelayedDoWork(lazy_now, *new_wake_up);
   }
 }
 

base/task/sequence_manager/time_domain_unittest.cc

@@ -5,11 +5,13 @@
 #include "base/task/sequence_manager/time_domain.h"
 
 #include <memory>
+
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/task/sequence_manager/sequence_manager_impl.h"
 #include "base/task/sequence_manager/task_queue_impl.h"
 #include "base/task/sequence_manager/work_queue.h"
+#include "base/test/mock_callback.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -383,5 +385,40 @@ TEST_F(TimeDomainTest, HighResolutionWakeUps) {
   q2.UnregisterTaskQueue();
 }
 
+TEST_F(TimeDomainTest, SetNextWakeUpForQueueInThePast) {
+  constexpr auto kType = MessageLoop::TYPE_DEFAULT;
+  constexpr auto kDelay = TimeDelta::FromMilliseconds(20);
+  SimpleTestTickClock clock;
+  auto sequence_manager =
+      internal::SequenceManagerImpl::CreateUnboundWithPump(kType, &clock);
+  sequence_manager->BindToMessagePump(
+      MessageLoop::CreateMessagePumpForType(kType));
+  auto high_prio_queue =
+      sequence_manager->CreateTaskQueue(TaskQueue::Spec("high_prio_queue"));
+  high_prio_queue->SetQueuePriority(TaskQueue::kHighestPriority);
+  auto high_prio_runner = high_prio_queue->CreateTaskRunner(kTaskTypeNone);
+  auto low_prio_queue =
+      sequence_manager->CreateTaskQueue(TaskQueue::Spec("low_prio_queue"));
+  low_prio_queue->SetQueuePriority(TaskQueue::kBestEffortPriority);
+  auto low_prio_runner = low_prio_queue->CreateTaskRunner(kTaskTypeNone);
+  sequence_manager->SetDefaultTaskRunner(high_prio_runner);
+  base::MockCallback<base::OnceCallback<void()>> task_1, task_2;
+
+  testing::Sequence s;
+  // Expect task_2 to run after task_1
+  EXPECT_CALL(task_1, Run);
+  EXPECT_CALL(task_2, Run);
+  // Schedule high and low priority tasks in such a way that clock.Now() will be
+  // way into the future by the time the low prio task run time is used to setup
+  // a wake up.
+  low_prio_runner->PostDelayedTask(FROM_HERE, task_2.Get(), kDelay);
+  high_prio_runner->PostDelayedTask(FROM_HERE, task_1.Get(), kDelay * 2);
+  high_prio_runner->PostTask(
+      FROM_HERE, Bind([](SimpleTestTickClock* clock,
+                         TimeDelta delay) { clock->Advance(delay); },
+                      base::Unretained(&clock), kDelay * 2));
+  RunLoop().RunUntilIdle();
+}
+
 }  // namespace sequence_manager
 }  // namespace base