cc: Add readback and forced draw states to the Scheduler

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 458428463ffaf4f6e46e6768fff9404a16ffda67..9e37c9f9f04ca4fbc849b505daf0b446f2034d5f 100644
--- a/cc/scheduler/scheduler_state_machine_unittest.cc
+++ b/cc/scheduler/scheduler_state_machine_unittest.cc
@@ -7,6 +7,10 @@
 #include "cc/scheduler/scheduler.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#define EXPECT_ACTION_UPDATE_STATE(action)                   \
+  EXPECT_EQ(action, state.NextAction()) << *state.AsValue(); \
+  state.UpdateState(action);
+
 namespace cc {
 
 namespace {
@@ -32,13 +36,29 @@ class StateMachine : public SchedulerStateMachine {
   void SetCommitState(CommitState cs) { commit_state_ = cs; }
   CommitState CommitState() const { return commit_state_; }
 
+  OutputSurfaceState output_surface_state() { return output_surface_state_; }
+
   bool NeedsCommit() const { return needs_commit_; }
 
   void SetNeedsRedraw(bool b) { needs_redraw_ = b; }
   bool NeedsRedraw() const { return needs_redraw_; }
 
-  void SetNeedsForcedRedraw(bool b) { needs_forced_redraw_ = b; }
-  bool NeedsForcedRedraw() const { return needs_forced_redraw_; }
+  void SetNeedsForcedRedrawForTimeout(bool b) {
+    forced_redraw_state_ = FORCED_REDRAW_STATE_WAITING_FOR_COMMIT;
+    commit_state_ = COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
+  }
+  bool NeedsForcedRedrawForTimeout() const {
+    return forced_redraw_state_ != FORCED_REDRAW_STATE_IDLE;
+  }
+
+  void SetNeedsForcedRedrawForReadback() {
+    readback_state_ = READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK;
+    commit_state_ = COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
+  }
+
+  bool NeedsForcedRedrawForReadback() const {
+    return readback_state_ != READBACK_STATE_IDLE;
+  }
 
   bool CanDraw() const { return can_draw_; }
   bool Visible() const { return visible_; }
@@ -108,15 +128,6 @@ TEST(SchedulerStateMachineTest, TestNextActionBeginsMainFrameIfNeeded) {
   }
 }
 
-TEST(SchedulerStateMachineTest, TestSetForcedRedrawDoesNotSetsNormalRedraw) {

[brianderson, 2013/08/29 19:23:10]

I removed this test because ToT has been setting needs_redraw_ at a later point
in the readback process anyway, even if needs_redraw_ isn't set at this point.
This causes more redraws than strictly necessary after a readback, but doesn't
cause any incorrectness bugs.

Fixing the test and the needs_redraw_ logic is going to be hairy to get correct
though, and I would like to solve it in a separate patch (assuming we actually
care about performance of CompositeAndReadback):

1) If both the readback commit and replacement commit are aborted, needs_redraw_
should not change value during that entire period.
2) If the readback commit is aborted, but the replacement commit is not aborted,
needs_redraw_ should be set to true.
3) If the readback commit is not aborted, but the replacement commit is aborted,
needs_redraw_ should also still be set to true.
4) If both the readback commit and replacement commit are not aborted, it's
possible that we still don't need a redraw because the replacement commit could
have been aborted if we didn't have the readback commit. This will be the
hardest case to make perfect.

-  SchedulerSettings default_scheduler_settings;
-  SchedulerStateMachine state(default_scheduler_settings);
-  state.SetCanDraw(true);
-  state.SetNeedsForcedRedraw();
-  EXPECT_FALSE(state.RedrawPending());
-  EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
-}
-
 TEST(SchedulerStateMachineTest,
      TestFailedDrawSetsNeedsCommitAndDoesNotDrawAgain) {
   SchedulerSettings default_scheduler_settings;
@@ -132,8 +143,9 @@ TEST(SchedulerStateMachineTest,
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
 
   // We're drawing now.
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
   EXPECT_FALSE(state.RedrawPending());
   EXPECT_FALSE(state.CommitPending());
@@ -164,8 +176,9 @@ TEST(SchedulerStateMachineTest,
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
 
   // We're drawing now.
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
   EXPECT_FALSE(state.RedrawPending());
   EXPECT_FALSE(state.CommitPending());
@@ -204,11 +217,12 @@ TEST(SchedulerStateMachineTest,
   state.SetNeedsRedraw(true);
   EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
   EXPECT_TRUE(state.RedrawPending());
 
   // Fail the draw.
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
   state.DidDrawIfPossibleCompleted(false);
   EXPECT_TRUE(state.RedrawPending());
@@ -222,61 +236,8 @@ TEST(SchedulerStateMachineTest,
   EXPECT_TRUE(state.RedrawPending());
 
   // And we should be allowed to draw again.
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
-}
-
-TEST(SchedulerStateMachineTest,
-     TestCommitAfterFailedAndSuccessfulDrawDoesNotAllowDrawInSameFrame) {
-  SchedulerSettings default_scheduler_settings;
-  StateMachine state(default_scheduler_settings);
-  state.SetCanStart();
-  state.UpdateState(state.NextAction());
-  state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
-  state.SetVisible(true);
-  state.SetCanDraw(true);
-
-  // Start a commit.
-  state.SetNeedsCommit();
-  EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
             state.NextAction());
-  state.UpdateState(
-      SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD);
-  EXPECT_TRUE(state.CommitPending());
-
-  // Then initiate a draw.
-  state.SetNeedsRedraw(true);
-  EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
-  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
-  EXPECT_TRUE(state.RedrawPending());
-
-  // Fail the draw.
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  state.DidDrawIfPossibleCompleted(false);
-  EXPECT_TRUE(state.RedrawPending());
-  // But the commit is ongoing.
-  EXPECT_TRUE(state.CommitPending());
-
-  // Force a draw.
-  state.SetNeedsForcedRedraw(true);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_FORCED, state.NextAction());
-
-  // Do the forced draw.
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_FORCED);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  EXPECT_FALSE(state.RedrawPending());
-  // And the commit is still ongoing.
-  EXPECT_TRUE(state.CommitPending());
-
-  // Finish the commit.
-  state.FinishCommit();
-  EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
-  state.UpdateState(SchedulerStateMachine::ACTION_COMMIT);
-  EXPECT_TRUE(state.RedrawPending());
-
-  // And we should not be allowed to draw again in the same frame..
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
 }
 
 TEST(SchedulerStateMachineTest,
@@ -302,11 +263,12 @@ TEST(SchedulerStateMachineTest,
   state.SetNeedsRedraw(true);
   EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
   EXPECT_TRUE(state.RedrawPending());
 
   // Fail the draw.
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
   state.DidDrawIfPossibleCompleted(false);
   EXPECT_TRUE(state.RedrawPending());
@@ -321,7 +283,8 @@ TEST(SchedulerStateMachineTest,
   EXPECT_TRUE(state.RedrawPending());
 
   // The redraw should be forced in this case.
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_FORCED, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_FORCED,
+            state.NextAction());
 }
 
 TEST(SchedulerStateMachineTest,
@@ -338,11 +301,12 @@ TEST(SchedulerStateMachineTest,
   state.SetNeedsRedraw(true);
   EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
   EXPECT_TRUE(state.RedrawPending());
 
   // Fail the draw.
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
   state.DidDrawIfPossibleCompleted(false);
   EXPECT_TRUE(state.RedrawPending());
@@ -356,7 +320,8 @@ TEST(SchedulerStateMachineTest,
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
 
   // We should try to draw again in the next begin frame on the impl thread.
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
 }
 
 TEST(SchedulerStateMachineTest, TestDoestDrawTwiceInSameFrame) {
@@ -370,8 +335,9 @@ TEST(SchedulerStateMachineTest, TestDoestDrawTwiceInSameFrame) {
   state.SetNeedsRedraw(true);
   EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
 
   // While still in the same begin frame for the impl thread, set needs redraw
   // again. This should not redraw.
@@ -384,8 +350,9 @@ TEST(SchedulerStateMachineTest, TestDoestDrawTwiceInSameFrame) {
   EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
 
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   state.DidDrawIfPossibleCompleted(true);
   EXPECT_FALSE(state.BeginFrameNeededToDrawByImplThread());
 }
@@ -413,12 +380,12 @@ TEST(SchedulerStateMachineTest, TestNextActionDrawsOnBeginFrame) {
       }
 
       // Case 1: needs_commit=false
-      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE,
+      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
                 state.NextAction());
 
       // Case 2: needs_commit=true
       state.SetNeedsCommit();
-      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE,
+      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
                 state.NextAction());
     }
   }
@@ -428,38 +395,48 @@ TEST(SchedulerStateMachineTest, TestNextActionDrawsOnBeginFrame) {
   // commit.
   for (size_t i = 0; i < num_commit_states; ++i) {
     for (size_t j = 0; j < 2; ++j) {
+      bool request_readback = j;
+
+      // Skip invalid states
+      if (request_readback &&
+          (SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW !=
+           all_commit_states[i]))
+        continue;
+
       StateMachine state(default_scheduler_settings);
       state.SetCanStart();
       state.UpdateState(state.NextAction());
       state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
       state.SetCanDraw(true);
       state.SetCommitState(all_commit_states[i]);
-      bool forced_draw = j;
-      if (!forced_draw) {
+      if (!request_readback) {
         state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
         state.SetNeedsRedraw(true);
         state.SetVisible(true);
       } else {
-        state.SetNeedsForcedRedraw(true);
+        state.SetNeedsForcedRedrawForReadback();
       }
 
       SchedulerStateMachine::Action expected_action;
       if (all_commit_states[i] !=
           SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT) {
         expected_action =
-            forced_draw ? SchedulerStateMachine::ACTION_DRAW_FORCED
-                        : SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE;
+            request_readback
+                ? SchedulerStateMachine::ACTION_DRAW_AND_READBACK
+                : SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE;
       } else {
         expected_action = SchedulerStateMachine::ACTION_COMMIT;
       }
 
       // Case 1: needs_commit=false.
-      EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
+      EXPECT_NE(state.BeginFrameNeededToDrawByImplThread(), request_readback)
+          << *state.AsValue();
       EXPECT_EQ(expected_action, state.NextAction()) << *state.AsValue();
 
       // Case 2: needs_commit=true.
       state.SetNeedsCommit();
-      EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
+      EXPECT_NE(state.BeginFrameNeededToDrawByImplThread(), request_readback)
+          << *state.AsValue();
       EXPECT_EQ(expected_action, state.NextAction()) << *state.AsValue();
     }
   }
@@ -480,17 +457,16 @@ TEST(SchedulerStateMachineTest, TestNoCommitStatesRedrawWhenInvisible) {
       state.SetCommitState(all_commit_states[i]);
       state.SetVisible(false);
       state.SetNeedsRedraw(true);
-      state.SetNeedsForcedRedraw(false);
       if (j == 1)
         state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
 
       // Case 1: needs_commit=false.
-      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE,
+      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
                 state.NextAction());
 
       // Case 2: needs_commit=true.
       state.SetNeedsCommit();
-      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE,
+      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
                 state.NextAction());
     }
   }
@@ -511,12 +487,11 @@ TEST(SchedulerStateMachineTest, TestCanRedraw_StopsDraw) {
       state.SetCommitState(all_commit_states[i]);
       state.SetVisible(false);
       state.SetNeedsRedraw(true);
-      state.SetNeedsForcedRedraw(false);
       if (j == 1)
         state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
 
       state.SetCanDraw(false);
-      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE,
+      EXPECT_NE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
                 state.NextAction());
     }
   }
@@ -529,17 +504,27 @@ TEST(SchedulerStateMachineTest,
   state.SetCanStart();
   state.UpdateState(state.NextAction());
   state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
-  state.SetCommitState(
-      SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
+
+  state.SetVisible(true);
+  state.SetNeedsCommit();
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD);
+  state.FinishCommit();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
+  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
+            state.CommitState());
+
   state.SetNeedsCommit();
   state.SetNeedsRedraw(true);
   state.SetVisible(true);
   state.SetCanDraw(false);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT,
-            state.NextAction());
-  state.UpdateState(state.NextAction());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
-            state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT)
+
+  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD);
+  state.DidLeaveBeginFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 }
 
 TEST(SchedulerStateMachineTest, TestsetNeedsCommitIsNotLost) {
@@ -576,10 +561,11 @@ TEST(SchedulerStateMachineTest, TestsetNeedsCommitIsNotLost) {
 
   // Commit and make sure we draw on next BeginFrame
   state.UpdateState(SchedulerStateMachine::ACTION_COMMIT);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
             state.CommitState());
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   state.DidDrawIfPossibleCompleted(true);
 
   // Verify that another commit will begin.
@@ -627,8 +613,9 @@ TEST(SchedulerStateMachineTest, TestFullCycle) {
 
   // At BeginFrame, draw.
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   state.DidDrawIfPossibleCompleted(true);
   state.DidLeaveBeginFrame();
 
@@ -681,8 +668,9 @@ TEST(SchedulerStateMachineTest, TestFullCycleWithCommitRequestInbetween) {
 
   // At BeginFrame, draw.
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
-  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE);
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
+  state.UpdateState(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
   state.DidDrawIfPossibleCompleted(true);
   state.DidLeaveBeginFrame();
 
@@ -787,7 +775,8 @@ TEST(SchedulerStateMachineTest, AbortBeginFrameAndCancelCommit) {
   // Start a new frame; draw because this is the first frame since output
   // surface init'd.
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction())
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction())
       << *state.AsValue();
   state.DidLeaveBeginFrame();
 
@@ -896,7 +885,8 @@ TEST(SchedulerStateMachineTest,
   // Once the context is recreated, whether we draw should be based on
   // SetCanDraw.
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
   state.SetCanDraw(false);
   EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT,
             state.NextAction());
@@ -922,7 +912,8 @@ TEST(SchedulerStateMachineTest, TestContextLostWhileCommitInProgress) {
   // Set damage and expect a draw.
   state.SetNeedsRedraw(true);
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
   state.UpdateState(state.NextAction());
   state.DidLeaveBeginFrame();
 
@@ -975,7 +966,8 @@ TEST(SchedulerStateMachineTest,
   // Set damage and expect a draw.
   state.SetNeedsRedraw(true);
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE,
+            state.NextAction());
   state.UpdateState(state.NextAction());
   state.DidLeaveBeginFrame();
 
@@ -1022,26 +1014,30 @@ TEST(SchedulerStateMachineTest, TestFinishAllRenderingWhileContextLost) {
   // Cause a lost context lost.
   state.DidLoseOutputSurface();
 
-  // Ask a forced redraw and verify it ocurrs.
-  state.SetNeedsForcedRedraw(true);
-  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_FORCED, state.NextAction());
-  state.DidLeaveBeginFrame();
+  // Ask a forced redraw and verify it ocurrs, even with a lost context,
+  // independent of the BeginFrame stae.
+  state.SetNeedsForcedRedrawForReadback();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
+
+  // Forced redraws for readbacks need to be followed by a new commit
+  // to replace the readback commit.
+  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
+            state.CommitState());
+  state.FinishCommit();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  // Clear the forced redraw bit.
-  state.SetNeedsForcedRedraw(false);
+  // We don't yet have an output surface, so we the draw and swap should abort.
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT);
 
   // Expect to be told to begin context recreation, independent of
-  // BeginFrame state
-  EXPECT_EQ(SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION,
-            state.NextAction());
-  state.UpdateState(state.NextAction());
+  // BeginFrame state.
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION);
 
-  // Ask a forced redraw and verify it ocurrs.
-  state.SetNeedsForcedRedraw(true);
-  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_FORCED, state.NextAction());
-  state.DidLeaveBeginFrame();
+  // Ask a readback and verify it ocurrs.
+  state.SetNeedsForcedRedrawForReadback();
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_READBACK,
+            state.NextAction());
 }
 
 TEST(SchedulerStateMachineTest, DontDrawBeforeCommitAfterLostOutputSurface) {
@@ -1076,7 +1072,7 @@ TEST(SchedulerStateMachineTest,
   state.DidCreateAndInitializeOutputSurface();
   state.SetVisible(false);
   state.SetNeedsCommit();
-  state.SetNeedsForcedCommit();
+  state.SetNeedsForcedCommitForReadback();
   EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
             state.NextAction());
 }
@@ -1088,7 +1084,7 @@ TEST(SchedulerStateMachineTest,
   state.SetVisible(true);
   state.SetCanDraw(true);
   state.SetNeedsCommit();
-  state.SetNeedsForcedCommit();
+  state.SetNeedsForcedCommitForReadback();
   EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
             state.NextAction());
 }
@@ -1121,22 +1117,28 @@ TEST(SchedulerStateMachineTest, TestFinishCommitWhenForcedCommitInProgress) {
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
-  state.DidCreateAndInitializeOutputSurface();
+  state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(false);
   state.SetCommitState(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS);
   state.SetNeedsCommit();
-  state.SetNeedsForcedCommit();
+  state.SetNeedsForcedCommitForReadback();
 
+  // The commit for readback interupts the normal commit.
   state.FinishCommit();
-  EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW,
+  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
             state.CommitState());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
 
-  // If we are waiting for forced draw then we know a begin frame is already
-  // in flight for the main thread.
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
+  // When the readback interrupts the normal commit, we should not get
+  // another BeginFrame on the impl thread when the readback completes.
+  EXPECT_NE(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
+            state.NextAction());
+
+  // The normal commit can then proceed.
+  state.FinishCommit();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 }
 
 TEST(SchedulerStateMachineTest, TestInitialActionsWhenContextLost) {
@@ -1152,13 +1154,16 @@ TEST(SchedulerStateMachineTest, TestInitialActionsWhenContextLost) {
 
   // When we are visible, we normally want to begin output surface creation
   // as soon as possible.
-  EXPECT_EQ(SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION,
-            state.NextAction()) << *state.AsValue();
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION);
 
+  state.DidCreateAndInitializeOutputSurface();
+  EXPECT_EQ(state.output_surface_state(),
+            SchedulerStateMachine::OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT);
 
-  // We should not send a BeginFrame to them main thread when we are invisible
-  // even if we've lost the output surface since the main thread will
-  // just abort anyway.
+  // We should not send a BeginFrame to them main thread when we are invisible,
+  // even if we've lost the output surface and are trying to get the first
+  // commit, since the main thread will just abort anyway.
   state.SetVisible(false);
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE,
             state.NextAction()) << *state.AsValue();
@@ -1166,9 +1171,10 @@ TEST(SchedulerStateMachineTest, TestInitialActionsWhenContextLost) {
   // If there is a forced commit, however, we could be blocking a readback
   // on the main thread, so we need to unblock it before we can get our
   // output surface, even if we are not visible.
-  state.SetNeedsForcedCommit();
+  state.SetNeedsForcedCommitForReadback();
   EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
-            state.NextAction()) << *state.AsValue();
+            state.NextAction())
+      << *state.AsValue();
 }
 
 TEST(SchedulerStateMachineTest, TestImmediateFinishCommit) {
@@ -1180,26 +1186,23 @@ TEST(SchedulerStateMachineTest, TestImmediateFinishCommit) {
   state.SetVisible(true);
   state.SetCanDraw(true);
 
-  // Schedule a forced frame, commit it, draw it.
+  // Schedule a readback, commit it, draw it.
   state.SetNeedsCommit();
-  state.SetNeedsForcedCommit();
-  state.UpdateState(state.NextAction());
+  state.SetNeedsForcedCommitForReadback();
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD);
   state.FinishCommit();
   EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW,
+  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
             state.CommitState());
 
-  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  state.SetNeedsForcedRedraw(true);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_FORCED, state.NextAction());
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   state.DidDrawIfPossibleCompleted(true);
-  state.DidLeaveBeginFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Should be waiting for the normal begin frame from the main thread.
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
@@ -1211,34 +1214,30 @@ TEST(SchedulerStateMachineTest, TestImmediateFinishCommitDuringCommit) {
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
-  state.DidCreateAndInitializeOutputSurface();
+  state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(true);
   state.SetCanDraw(true);
 
   // Start a normal commit.
   state.SetNeedsCommit();
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD);
 
-  // Schedule a forced frame, commit it, draw it.
+  // Schedule a readback, commit it, draw it.
   state.SetNeedsCommit();
-  state.SetNeedsForcedCommit();
-  state.UpdateState(state.NextAction());
+  state.SetNeedsForcedCommitForReadback();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.FinishCommit();
-  EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW,
+  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
             state.CommitState());
 
-  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  state.SetNeedsForcedRedraw(true);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_FORCED, state.NextAction());
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   state.DidDrawIfPossibleCompleted(true);
-  state.DidLeaveBeginFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Should be waiting for the normal begin frame from the main thread.
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
@@ -1252,33 +1251,29 @@ TEST(SchedulerStateMachineTest,
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
-  state.DidCreateAndInitializeOutputSurface();
+  state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(true);
   state.SetCanDraw(true);
 
   state.SetNeedsCommit();
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(
+      SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD);
 
   state.SetNeedsCommit();
-  state.SetNeedsForcedCommit();
-  state.UpdateState(state.NextAction());
+  state.SetNeedsForcedCommitForReadback();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
   state.FinishCommit();
 
-  EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW,
+  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
             state.CommitState());
 
-  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  state.SetNeedsForcedRedraw(true);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_FORCED, state.NextAction());
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   state.DidDrawIfPossibleCompleted(true);
-  state.DidLeaveBeginFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 
   // Should be waiting for the main thread's begin frame.
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
@@ -1299,7 +1294,7 @@ TEST(SchedulerStateMachineTest, ImmediateFinishCommitWhileCantDraw) {
   StateMachine state(default_scheduler_settings);
   state.SetCanStart();
   state.UpdateState(state.NextAction());
-  state.DidCreateAndInitializeOutputSurface();
+  state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
   state.SetVisible(true);
   state.SetCanDraw(false);
 
@@ -1307,25 +1302,20 @@ TEST(SchedulerStateMachineTest, ImmediateFinishCommitWhileCantDraw) {
   state.UpdateState(state.NextAction());
 
   state.SetNeedsCommit();
-  state.SetNeedsForcedCommit();
+  state.SetNeedsForcedCommitForReadback();
   state.UpdateState(state.NextAction());
   state.FinishCommit();
 
-  EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_READY_TO_COMMIT,
             state.CommitState());
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
 
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW,
+  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
             state.CommitState());
 
-  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  state.SetNeedsForcedRedraw(true);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_FORCED, state.NextAction());
-  state.UpdateState(state.NextAction());
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_DRAW_AND_READBACK);
   state.DidDrawIfPossibleCompleted(true);
-  state.DidLeaveBeginFrame();
+  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
 }
 
 TEST(SchedulerStateMachineTest, ReportIfNotDrawing) {