[scheduler] Change TaskQueue observer call mechanism.

third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc

Index: third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
diff --git a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
index 68d0e537ce629a3553084542354bf512e224efd3..f353768fdd9a7213a6a447544a07ba93612d0b6b 100644
--- a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
+++ b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
@@ -359,9 +359,9 @@ TEST_F(TaskQueueManagerTest, HasPendingImmediateWork_ImmediateTask) {
   Initialize(1u);
 
   std::vector<EnqueueOrder> run_order;
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
   runners_[0]->PostTask(FROM_HERE, base::Bind(&TestTask, 1, &run_order));
-  EXPECT_TRUE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_TRUE(runners_[0]->HasTaskToRunImmediately());
 
   // Move the task into the |immediate_work_queue|.
   EXPECT_TRUE(runners_[0]->immediate_work_queue()->Empty());
@@ -370,12 +370,12 @@ TEST_F(TaskQueueManagerTest, HasPendingImmediateWork_ImmediateTask) {
   voter->SetQueueEnabled(false);
   test_task_runner_->RunUntilIdle();
   EXPECT_FALSE(runners_[0]->immediate_work_queue()->Empty());
-  EXPECT_TRUE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_TRUE(runners_[0]->HasTaskToRunImmediately());
 
   // Run the task, making the queue empty.
   voter->SetQueueEnabled(true);
   test_task_runner_->RunUntilIdle();
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
 }
 
 TEST_F(TaskQueueManagerTest, HasPendingImmediateWork_DelayedTask) {
@@ -385,18 +385,18 @@ TEST_F(TaskQueueManagerTest, HasPendingImmediateWork_DelayedTask) {
   base::TimeDelta delay(base::TimeDelta::FromMilliseconds(10));
   runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&TestTask, 1, &run_order),
                                delay);
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
   now_src_->Advance(delay);
-  EXPECT_TRUE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_TRUE(runners_[0]->HasTaskToRunImmediately());
 
   // Move the task into the |delayed_work_queue|.
   WakeUpReadyDelayedQueues(LazyNow(now_src_.get()));
   EXPECT_FALSE(runners_[0]->delayed_work_queue()->Empty());
-  EXPECT_TRUE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_TRUE(runners_[0]->HasTaskToRunImmediately());
 
   // Run the task, making the queue empty.
   test_task_runner_->RunUntilIdle();
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
 }
 
 TEST_F(TaskQueueManagerTest, DelayedTaskPosting) {
@@ -407,7 +407,7 @@ TEST_F(TaskQueueManagerTest, DelayedTaskPosting) {
   runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&TestTask, 1, &run_order),
                                delay);
   EXPECT_EQ(delay, test_task_runner_->DelayToNextTaskTime());
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
   EXPECT_TRUE(run_order.empty());
 
   // The task doesn't run before the delay has completed.
@@ -417,7 +417,7 @@ TEST_F(TaskQueueManagerTest, DelayedTaskPosting) {
   // After the delay has completed, the task runs normally.
   test_task_runner_->RunForPeriod(base::TimeDelta::FromMilliseconds(1));
   EXPECT_THAT(run_order, ElementsAre(1));
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
 }
 
 bool MessageLoopTaskCounter(size_t* count) {
@@ -551,7 +551,7 @@ TEST_F(TaskQueueManagerTest, InsertAndRemoveFence) {
   EXPECT_FALSE(test_task_runner_->HasPendingTasks());
 
   // However polling still works.
-  EXPECT_TRUE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_TRUE(runners_[0]->HasTaskToRunImmediately());
 
   // After removing the fence the task runs normally.
   runners_[0]->RemoveFence();
@@ -1134,34 +1134,34 @@ TEST_F(TaskQueueManagerTest, GetAndClearSystemIsQuiescentBit) {
 TEST_F(TaskQueueManagerTest, HasPendingImmediateWork) {
   Initialize(1u);
 
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
   runners_[0]->PostTask(FROM_HERE, base::Bind(NullTask));
-  EXPECT_TRUE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_TRUE(runners_[0]->HasTaskToRunImmediately());
 
   test_task_runner_->RunUntilIdle();
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
 }
 
 TEST_F(TaskQueueManagerTest, HasPendingImmediateWork_DelayedTasks) {
   Initialize(1u);
 
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
   runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(NullTask),
                                base::TimeDelta::FromMilliseconds(12));
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
 
   // Move time forwards until just before the delayed task should run.
   now_src_->Advance(base::TimeDelta::FromMilliseconds(10));
   WakeUpReadyDelayedQueues(LazyNow(now_src_.get()));
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
 
   // Force the delayed task onto the work queue.
   now_src_->Advance(base::TimeDelta::FromMilliseconds(2));
   WakeUpReadyDelayedQueues(LazyNow(now_src_.get()));
-  EXPECT_TRUE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_TRUE(runners_[0]->HasTaskToRunImmediately());
 
   test_task_runner_->RunUntilIdle();
-  EXPECT_FALSE(runners_[0]->HasPendingImmediateWork());
+  EXPECT_FALSE(runners_[0]->HasTaskToRunImmediately());
 }
 
 void ExpensiveTestTask(int value,
@@ -1748,12 +1748,14 @@ TEST_F(TaskQueueManagerTest, TaskQueueObserver_DelayedTask) {
   std::unique_ptr<TaskQueue::QueueEnabledVoter> voter =
       runners_[0]->CreateQueueEnabledVoter();
   voter->SetQueueEnabled(false);
+  Mock::VerifyAndClearExpectations(&observer);
 
   // When a queue has been enabled, we may get a notification if the
   // TimeDomain's next scheduled wake-up has changed.
   EXPECT_CALL(observer, OnQueueNextWakeUpChanged(runners_[0].get(),
                                                  start_time + delay1s));
   voter->SetQueueEnabled(true);
+  Mock::VerifyAndClearExpectations(&observer);
 
   // Tidy up.
   runners_[0]->UnregisterTaskQueue();
@@ -1813,6 +1815,46 @@ TEST_F(TaskQueueManagerTest, TaskQueueObserver_DelayedTaskMultipleQueues) {
   runners_[1]->UnregisterTaskQueue();
 }
 
+TEST_F(TaskQueueManagerTest, TaskQueueObserver_DelayedWorkWhichCanRunNow) {
+  // This test checks that when delayed work becomes available
+  // the notification still fires. This usually happens when time advances
+  // and task becomes available in the middle of the scheduling code.
+  // For this test we rely on the fact that notification dispatching code
+  // is the same in all conditions and just change a time domain to
+  // trigger notification.
+
+  Initialize(1u);
+
+  base::TimeTicks start_time = manager_->Delegate()->NowTicks();
+  base::TimeDelta delay1s(base::TimeDelta::FromSeconds(1));
+  base::TimeDelta delay10s(base::TimeDelta::FromSeconds(10));
+
+  MockTaskQueueObserver observer;
+  runners_[0]->SetObserver(&observer);
+
+  // We should get a notification when a delayed task is posted on an empty
+  // queue.
+  EXPECT_CALL(observer, OnQueueNextWakeUpChanged(runners_[0].get(),
+                                                 start_time + delay1s));

[alex clarke (OOO till 29th), 2017/05/09 19:18:42]

This is testing something that is kind of incidental to this test that I believe
we already have sufficient coverage for.   My suggestion is to change this to
EXPECT_CALL(observer, OnQueueNextWakeUpChanged((_, _));

[altimin, 2017/05/10 10:42:25]

Yes, we don't really care about exact values in this test.

+  runners_[0]->PostDelayedTask(FROM_HERE, base::Bind(&NopTask), delay1s);
+  Mock::VerifyAndClearExpectations(&observer);
+
+  std::unique_ptr<TimeDomain> mock_time_domain =

[alex clarke (OOO till 29th), 2017/05/09 12:50:51]

Seems like we're testing several things here, should we split this into two
tests?

[altimin, 2017/05/09 13:15:04]

It's my understanding that we're testing only one thing here. The comment above
explains why we need a time domain to achieve the desired effect.

+      base::MakeUnique<RealTimeDomain>();
+  manager_->RegisterTimeDomain(mock_time_domain.get());
+
+  now_src_->Advance(delay10s);
+
+  EXPECT_CALL(observer,

[alex clarke (OOO till 29th), 2017/05/09 19:18:42]

This is the thing we really care about right?  Incidentally .Times(1) is I
believe a nop although it's OK to leave it but please be  consistent about using
it or not.

[altimin, 2017/05/10 10:42:25]

Yes! The problem in the linked bug was caused by this notification not being
issued. The underlying reason wasn't time domain change, but the mechanism
issuing notification is the same and time domain change is the easiest way to
test it.

+              OnQueueNextWakeUpChanged(runners_[0].get(), start_time + delay1s))
+      .Times(1);
+  runners_[0]->SetTimeDomain(mock_time_domain.get());
+  Mock::VerifyAndClearExpectations(&observer);
+
+  // Tidy up.
+  runners_[0]->UnregisterTaskQueue();
+}
+
 class CancelableTask {
  public:
   explicit CancelableTask(base::TickClock* clock)