cc: Simplify SetNeedsBeginFrames code in scheduler.

cc/scheduler/scheduler_unittest.cc

Index: cc/scheduler/scheduler_unittest.cc
diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc
index d2ec6b9b29fd23a6dfcd33cbbf81f0f57af6e532..e1e7bb1728d0d1d80dbf1e83b24d851e5265c1df 100644
--- a/cc/scheduler/scheduler_unittest.cc
+++ b/cc/scheduler/scheduler_unittest.cc
@@ -1565,9 +1565,8 @@ TEST_F(SchedulerTest, DidLoseOutputSurfaceAfterBeginFrameStarted) {
 
   client_->Reset();
   scheduler_->DidLoseOutputSurface();
-  // Do nothing when impl frame is in deadine pending state.
-  EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 0, 2);
-  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 1, 2);
+  // SetNeedsBeginFrames(false) is not called until the end of the frame.

[brianderson, 2015/03/06 01:34:36]

It's a little concerning that we won't stop requesting SetNeedsBeginFrames
immediately anymore. But I guess we trigger the deadline asap when we lose the
output surface and will stop requesting the BeginFrames at that point.

We may want to decouple SetNeedsBeginFrames from OS lost in the future (and try
to recreate the OS super fast) anyway, so this is a tiny step in that direction.
=)

[sunnyps, 2015/03/06 01:38:50]

We do have the ability to ignore BeginFrames in the scheduler so maybe this is
doable now.

+  EXPECT_NO_ACTION(client_);
 
   client_->Reset();
   scheduler_->NotifyBeginMainFrameStarted();
@@ -1575,8 +1574,10 @@ TEST_F(SchedulerTest, DidLoseOutputSurfaceAfterBeginFrameStarted) {
   EXPECT_ACTION("ScheduledActionCommit", client_, 0, 1);
 
   client_->Reset();
-  task_runner().RunPendingTasks();  // Run posted deadline.
-  EXPECT_SINGLE_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_);
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_, 0, 3);
+  EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 1, 3);

[brianderson, 2015/03/06 01:34:36]

I was going to say to try and change the code so SetNeedsBeginFrames(false) is
called before ScheduledActionBeginOutputSurfaceCreation, but both actions are
potentially async and our code should work either way.

Just trying to think of potential race conditions. Haven't thought of any real
ones, which is good.

[sunnyps, 2015/03/06 01:38:50]

I would've liked that too - it's difficult to achieve because the state machine
is responsible for ScheduledActionBeginOutputSurfaceCreation while the scheduler
is responsible for SetNeedsBeginFrames. The scheduler is trying to enforce that
SetNeedsBeginFrames is the last thing which happens in a frame.

+  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 2, 3);
 }
 
 void SchedulerTest::DidLoseOutputSurfaceAfterBeginFrameStartedWithHighLatency(
@@ -1598,19 +1599,20 @@ void SchedulerTest::DidLoseOutputSurfaceAfterBeginFrameStartedWithHighLatency(
   client_->Reset();
   scheduler_->DidLoseOutputSurface();
   // Do nothing when impl frame is in deadine pending state.
-  EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 0, 2);
-  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 1, 2);
+  EXPECT_NO_ACTION(client_);
 
   client_->Reset();
   // Run posted deadline.
   EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
   task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
-  // OnBeginImplFrameDeadline didn't schedule any actions because main frame is
-  // not yet completed.
-  EXPECT_NO_ACTION(client_);
+  // OnBeginImplFrameDeadline didn't schedule output surface creation because
+  // main frame is not yet completed.
+  EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 0, 2);
+  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 1, 2);
   EXPECT_FALSE(scheduler_->BeginImplFrameDeadlinePending());
 
   // BeginImplFrame is not started.
+  client_->Reset();
   task_runner().RunUntilTime(now_src()->Now() +
                              base::TimeDelta::FromMilliseconds(10));
   EXPECT_NO_ACTION(client_);
@@ -1664,19 +1666,19 @@ void SchedulerTest::DidLoseOutputSurfaceAfterReadyToCommit(
 
   client_->Reset();
   scheduler_->DidLoseOutputSurface();
+  // SetNeedsBeginFrames(false) is not called until the end of the frame.
   if (impl_side_painting) {
     // Sync tree should be forced to activate.
-    EXPECT_ACTION("ScheduledActionActivateSyncTree", client_, 0, 3);
-    EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 1, 3);
-    EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 2, 3);
+    EXPECT_SINGLE_ACTION("ScheduledActionActivateSyncTree", client_);
   } else {
-    EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 0, 2);
-    EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 1, 2);
+    EXPECT_NO_ACTION(client_);
   }
 
   client_->Reset();
-  task_runner().RunPendingTasks();  // Run posted deadline.
-  EXPECT_SINGLE_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_);
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_, 0, 3);
+  EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 1, 3);
+  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 2, 3);
 }
 
 TEST_F(SchedulerTest, DidLoseOutputSurfaceAfterReadyToCommit) {
@@ -1703,17 +1705,18 @@ TEST_F(SchedulerTest, DidLoseOutputSurfaceAfterSetNeedsPrepareTiles) {
 
   client_->Reset();
   scheduler_->DidLoseOutputSurface();
-  EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 0, 2);
-  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 1, 2);
+  // SetNeedsBeginFrames(false) is not called until the end of the frame.
+  EXPECT_NO_ACTION(client_);
 
   client_->Reset();
-  task_runner().RunPendingTasks();  // Run posted deadline.
-  EXPECT_ACTION("ScheduledActionPrepareTiles", client_, 0, 2);
-  EXPECT_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_, 1, 2);
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_ACTION("ScheduledActionPrepareTiles", client_, 0, 4);
+  EXPECT_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_, 1, 4);
+  EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 2, 4);
+  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 3, 4);
 }
 
-TEST_F(SchedulerTest,
-       StopBeginFrameAfterDidLoseOutputSurfaceWithSyntheticBeginFrameSource) {
+TEST_F(SchedulerTest, DidLoseOutputSurfaceWithSyntheticBeginFrameSource) {
   SetUpScheduler(true);
 
   // SetNeedsCommit should begin the frame on the next BeginImplFrame.
@@ -1722,7 +1725,7 @@ TEST_F(SchedulerTest,
   EXPECT_TRUE(scheduler_->frame_source().NeedsBeginFrames());
 
   client_->Reset();
-  task_runner().RunPendingTasks();  // Run posted Tick.
+  AdvanceFrame();
   EXPECT_ACTION("WillBeginImplFrame", client_, 0, 2);
   EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 1, 2);
   EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
@@ -1737,15 +1740,49 @@ TEST_F(SchedulerTest,
 
   client_->Reset();
   scheduler_->DidLoseOutputSurface();
-  EXPECT_SINGLE_ACTION("SendBeginMainFrameNotExpectedSoon", client_);
-  EXPECT_FALSE(scheduler_->frame_source().NeedsBeginFrames());
+  // SetNeedsBeginFrames(false) is not called until the end of the frame.
+  EXPECT_NO_ACTION(client_);
+  EXPECT_TRUE(scheduler_->frame_source().NeedsBeginFrames());
 
   client_->Reset();
-  task_runner().RunPendingTasks();  // Run posted deadline.
-  EXPECT_SINGLE_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_);
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_, 0, 2);
+  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 1, 2);
   EXPECT_FALSE(scheduler_->frame_source().NeedsBeginFrames());
 }
 
+TEST_F(SchedulerTest, DidLoseOutputSurfaceWhenIdle) {
+  scheduler_settings_.use_external_begin_frame_source = true;
+  SetUpScheduler(true);
+
+  // SetNeedsCommit should begin the frame.
+  scheduler_->SetNeedsCommit();
+  EXPECT_SINGLE_ACTION("SetNeedsBeginFrames(true)", client_);
+
+  client_->Reset();
+  EXPECT_SCOPED(AdvanceFrame());
+  EXPECT_ACTION("WillBeginImplFrame", client_, 0, 2);
+  EXPECT_ACTION("ScheduledActionSendBeginMainFrame", client_, 1, 2);
+  EXPECT_TRUE(scheduler_->BeginImplFrameDeadlinePending());
+
+  client_->Reset();
+  scheduler_->NotifyBeginMainFrameStarted();
+  scheduler_->NotifyReadyToCommit();
+  EXPECT_SINGLE_ACTION("ScheduledActionCommit", client_);
+
+  client_->Reset();
+  task_runner().RunTasksWhile(client_->ImplFrameDeadlinePending(true));
+  EXPECT_ACTION("ScheduledActionAnimate", client_, 0, 2);
+  EXPECT_ACTION("ScheduledActionDrawAndSwapIfPossible", client_, 1, 2);
+
+  // Idle time between BeginFrames.
+  client_->Reset();
+  scheduler_->DidLoseOutputSurface();
+  EXPECT_ACTION("ScheduledActionBeginOutputSurfaceCreation", client_, 0, 3);
+  EXPECT_ACTION("SetNeedsBeginFrames(false)", client_, 1, 3);
+  EXPECT_ACTION("SendBeginMainFrameNotExpectedSoon", client_, 2, 3);
+}
+
 TEST_F(SchedulerTest, ScheduledActionActivateAfterBecomingInvisible) {
   scheduler_settings_.impl_side_painting = true;
   scheduler_settings_.use_external_begin_frame_source = true;