Create a new action triggered when a BeginMainFrame is not expected before vsync

cc/scheduler/scheduler_state_machine_unittest.cc

Index: cc/scheduler/scheduler_state_machine_unittest.cc
diff --git a/cc/scheduler/scheduler_state_machine_unittest.cc b/cc/scheduler/scheduler_state_machine_unittest.cc
index 0a99a0969a0b4ab28055457e3102bed1093e44a5..d5f66012a25d5115c7ae0821e54e1c0bc87bc065 100644
--- a/cc/scheduler/scheduler_state_machine_unittest.cc
+++ b/cc/scheduler/scheduler_state_machine_unittest.cc
@@ -204,6 +204,10 @@ void PerformAction(StateMachine* sm, SchedulerStateMachine::Action action) {
       sm->WillSendBeginMainFrame();
       return;
 
+    case SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK:
+      sm->WillDoShortIdleWork();
+      return;
+
     case SchedulerStateMachine::ACTION_COMMIT: {
       bool commit_has_no_updates = false;
       sm->WillCommit(commit_has_no_updates);
@@ -309,6 +313,8 @@ TEST(SchedulerStateMachineTest, TestNextActionBeginsMainFrameIfNeeded) {
     EXPECT_FALSE(state.NeedsCommit());
 
     state.IssueNextBeginImplFrame();
+    EXPECT_ACTION_UPDATE_STATE(
+        SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
     EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
     state.OnBeginImplFrameDeadline();
@@ -458,6 +464,7 @@ TEST(SchedulerStateMachineTest,
 
   // Start a frame.
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);

[Sami, 2017/03/24 15:24:10]

I was thinking we might wanna avoid idling here, but looks like the next
BeginMainFrame comes in after the vsync so this should be fine.

[Dan Elphick, 2017/03/29 16:02:44]

Acknowledged.

   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_FALSE(state.CommitPending());
 
@@ -495,6 +502,7 @@ TEST(SchedulerStateMachineTest, FailedDrawForMissingHighResNeedsCommit) {
 
   // Start a frame.
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_FALSE(state.CommitPending());
 
@@ -523,6 +531,7 @@ TEST(SchedulerStateMachineTest, FailedDrawForMissingHighResNeedsCommit) {
   state.NotifyBeginMainFrameStarted();
   state.NotifyReadyToCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);

[Sami, 2017/03/24 15:24:11]

Is this right? We just did a BeginMainFrame.

[Dan Elphick, 2017/03/29 16:02:44]

It's right in that the tests don't pass without it.

The reason it happens is because the IssueNextBeginImplFrame on line 523, which
clears the did_do_idle_work_ and did_send_begin_main_frame_for_current_frame_
flags. The Commit action then sets the begin_main_frame_state_ flag to IDLE
which is sufficient to allow short idle work to go.

What condition do you think should prevent this?

[Dan Elphick, 2017/04/06 16:10:16]

This is now removed after tweaking the state machine.

   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.NotifyReadyToActivate();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_ACTIVATE_SYNC_TREE);
@@ -531,6 +540,7 @@ TEST(SchedulerStateMachineTest, FailedDrawForMissingHighResNeedsCommit) {
 
   // Verify we draw with the new frame.
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   state.SetDrawResultForTest(DRAW_SUCCESS);
@@ -664,6 +674,7 @@ TEST(SchedulerStateMachineTest, TestFailedDrawIsRetriedInNextBeginImplFrame) {
   state.SetNeedsRedraw(true);
   EXPECT_TRUE(state.BeginFrameNeeded());
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_TRUE(state.RedrawPending());
@@ -700,6 +711,7 @@ TEST(SchedulerStateMachineTest, TestDoestDrawTwiceInSameFrame) {
   // Draw the first frame.
   EXPECT_TRUE(state.BeginFrameNeeded());
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   state.OnBeginImplFrameDeadline();
@@ -716,6 +728,7 @@ TEST(SchedulerStateMachineTest, TestDoestDrawTwiceInSameFrame) {
   // Move to another frame. This should now draw.
   EXPECT_TRUE(state.BeginFrameNeeded());
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
@@ -1400,6 +1413,7 @@ TEST(SchedulerStateMachineTest, TestAbortBeginMainFrameBecauseCommitNotNeeded) {
   EXPECT_MAIN_FRAME_STATE(SchedulerStateMachine::BEGIN_MAIN_FRAME_STATE_IDLE);
 
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
@@ -1424,6 +1438,7 @@ TEST(SchedulerStateMachineTest, TestFirstContextCreation) {
 
   // Check that the first init does not SetNeedsBeginMainFrame.
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
@@ -1478,6 +1493,7 @@ TEST(SchedulerStateMachineTest,
 
   // Once context recreation begins, nothing should happen.
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
@@ -1528,6 +1544,7 @@ TEST(SchedulerStateMachineTest,
   // automatically cause a redraw.
   EXPECT_TRUE(state.RedrawPending());
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
@@ -1536,6 +1553,7 @@ TEST(SchedulerStateMachineTest,
 
   // Next frame as no work to do.
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
@@ -1544,6 +1562,7 @@ TEST(SchedulerStateMachineTest,
   // SetCanDraw if waiting on first draw after activate.
   state.SetNeedsRedraw(true);
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
@@ -1636,6 +1655,7 @@ TEST(SchedulerStateMachineTest, TestContextLostWhileCommitInProgress) {
   state.OnBeginImplFrame(0, 11);
   EXPECT_IMPL_FRAME_STATE(
       SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION(SchedulerStateMachine::ACTION_NONE);
   EXPECT_SEQUENCE_NUMBERS(11u, 10u, 10u, 10u,
                           BeginFrameArgs::kInvalidFrameNumber);
@@ -1993,7 +2013,8 @@ TEST(SchedulerStateMachineTest,
 }
 
 void FinishPreviousCommitAndDrawWithoutExitingDeadline(
-    StateMachine* state_ptr) {
+    StateMachine* state_ptr,
+    bool expectIdleWorkBeforeImplFrame) {

[Sami, 2017/03/24 15:24:11]

nit: hacker_style

Also expect_idle_work_before_impl_frame_deadline would be a little more
accurate.

[Dan Elphick, 2017/03/29 16:02:44]

Done.

   // Gross, but allows us to use macros below.
   StateMachine& state = *state_ptr;
 
@@ -2006,6 +2027,10 @@ void FinishPreviousCommitAndDrawWithoutExitingDeadline(
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   state.IssueNextBeginImplFrame();
+  if (expectIdleWorkBeforeImplFrame) {
+    EXPECT_ACTION_UPDATE_STATE(
+        SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
+  }
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   EXPECT_TRUE(state.ShouldTriggerBeginImplFrameDeadlineImmediately());
@@ -2045,7 +2070,7 @@ TEST(SchedulerStateMachineTest, TestImplLatencyTakesPriority) {
 
   // Request a new commit and finish the previous one.
   state.SetNeedsBeginMainFrame();
-  FinishPreviousCommitAndDrawWithoutExitingDeadline(&state);
+  FinishPreviousCommitAndDrawWithoutExitingDeadline(&state, false);
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
@@ -2053,7 +2078,7 @@ TEST(SchedulerStateMachineTest, TestImplLatencyTakesPriority) {
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Finish the previous commit and draw it.
-  FinishPreviousCommitAndDrawWithoutExitingDeadline(&state);
+  FinishPreviousCommitAndDrawWithoutExitingDeadline(&state, true);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Verify we do not send another BeginMainFrame if was are submit-frame
@@ -2234,6 +2259,7 @@ TEST(SchedulerStateMachineTest, ImplSideInvalidationOnlyInsideDeadline) {
 
   state.SetNeedsImplSideInvalidation();
   state.IssueNextBeginImplFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
@@ -2257,6 +2283,7 @@ TEST(SchedulerStateMachineTest,
   state.SetNeedsImplSideInvalidation();
   state.IssueNextBeginImplFrame();
   state.OnBeginImplFrameDeadline();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Initializing the CompositorFrameSink puts us in a state waiting for the
@@ -2351,6 +2378,7 @@ TEST(SchedulerStateMachineTest,
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_PERFORM_IMPL_SIDE_INVALIDATION);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Request another invalidation, which should wait until the pending tree is
@@ -2399,6 +2427,7 @@ TEST(SchedulerStateMachineTest, ImplSideInvalidationsThrottledOnDraw) {
   state.SetNeedsImplSideInvalidation();
   state.IssueNextBeginImplFrame();
   state.OnBeginImplFrameDeadline();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Ack the previous frame and begin impl frame, which should perform the
@@ -2447,6 +2476,7 @@ TEST(SchedulerStateMachineTest, PrepareTilesWaitForImplSideInvalidation) {
   EXPECT_ACTION_UPDATE_STATE(
       SchedulerStateMachine::ACTION_PERFORM_IMPL_SIDE_INVALIDATION);
   state.DidPrepareTiles();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 }
 
@@ -2473,6 +2503,7 @@ TEST(SchedulerStateMachineTest, TestBeginFrameFreshnessWithoutUpdates) {
 
   // OnBeginImplFrame() updates the sequence number.
   state.OnBeginImplFrame(0, 10);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_SEQUENCE_NUMBERS(10u, BeginFrameArgs::kInvalidFrameNumber,
                           BeginFrameArgs::kInvalidFrameNumber,
@@ -2500,6 +2531,7 @@ TEST(SchedulerStateMachineTest, TestBeginFrameFreshnessWithImplFrameUpdates) {
 
   // OnBeginImplFrame() updates the sequence number.
   state.OnBeginImplFrame(0, 10);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_SEQUENCE_NUMBERS(10u, BeginFrameArgs::kInvalidFrameNumber,
                           BeginFrameArgs::kInvalidFrameNumber,
@@ -2574,6 +2606,7 @@ TEST(SchedulerStateMachineTest, TestBeginFrameFreshnessWithMainFrameUpdates) {
   // Pending tree freshness is updated when the new pending tree is committed.
   state.NotifyReadyToCommit();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);

[Sami, 2017/03/24 15:24:10]

Here a begin main frame seems to have recently completeted
(NotifyBeginMainFrameStarted + commit) -- do we wanna idle?

[Dan Elphick, 2017/03/29 16:02:44]

Seems like this is the same as the last one. What would clear that condition?

[Dan Elphick, 2017/04/06 16:10:16]

Done.

   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_FALSE(state.CommitPending());
   EXPECT_TRUE(state.has_pending_tree());
@@ -2583,6 +2616,7 @@ TEST(SchedulerStateMachineTest, TestBeginFrameFreshnessWithMainFrameUpdates) {
   // If no further BeginMainFrame is needed, OnBeginFrameImplDeadline()
   // updates the pending tree's frame number.
   state.OnBeginImplFrame(0, 12);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_SEQUENCE_NUMBERS(12u, 10u, 10u, BeginFrameArgs::kInvalidFrameNumber,
                           BeginFrameArgs::kInvalidFrameNumber);
@@ -2644,6 +2678,7 @@ TEST(SchedulerStateMachineTest, TestBeginFrameFreshnessWithMainFrameUpdates) {
   // When no updates are required, OnBeginImplFrameDeadline() updates active
   // tree and compositor frame freshness.
   state.OnBeginImplFrame(0, 15);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.OnBeginImplFrameDeadline();
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
@@ -2673,6 +2708,7 @@ TEST(SchedulerStateMachineTest, TestBeginFrameFreshnessWithMainFrameUpdates) {
   // When the source changes, the current frame number is updated and frame
   // numbers for freshness are reset to invalid numbers.
   state.OnBeginImplFrame(1, 5);
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DO_SHORT_IDLE_WORK);
   EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   EXPECT_SEQUENCE_NUMBERS(5u, BeginFrameArgs::kInvalidFrameNumber,
                           BeginFrameArgs::kInvalidFrameNumber,