cc: Add readback and forced draw states to the Scheduler

cc/scheduler/scheduler_state_machine.cc

Index: cc/scheduler/scheduler_state_machine.cc
diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
index c20238b3a589fd456a164e6783facc30eff34966..90215afaaca332c54e5e2a3b9ece1961d144aea6 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -15,20 +15,19 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
     : settings_(settings),
       output_surface_state_(OUTPUT_SURFACE_LOST),
       commit_state_(COMMIT_STATE_IDLE),
+      texture_state_(LAYER_TEXTURE_STATE_UNLOCKED),
+      forced_redraw_state_(FORCED_REDRAW_STATE_IDLE),
+      readback_state_(READBACK_STATE_IDLE),
       commit_count_(0),
       current_frame_number_(0),
       last_frame_number_where_begin_frame_sent_to_main_thread_(-1),
-      last_frame_number_where_draw_was_called_(-1),
+      last_frame_number_swap_performed_(-1),
       last_frame_number_where_update_visible_tiles_was_called_(-1),
       consecutive_failed_draws_(0),
       maximum_number_of_failed_draws_before_draw_is_forced_(3),
       needs_redraw_(false),
       swap_used_incomplete_tile_(false),
-      needs_forced_redraw_(false),
-      needs_forced_redraw_after_next_commit_(false),
       needs_commit_(false),
-      needs_forced_commit_(false),
-      expect_immediate_begin_frame_for_main_thread_(false),
       main_thread_needs_layer_textures_(false),
       inside_begin_frame_(false),
       visible_(false),
@@ -38,7 +37,6 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       pending_tree_is_ready_for_activation_(false),
       active_tree_has_been_drawn_(false),
       draw_if_possible_failed_(false),
-      texture_state_(LAYER_TEXTURE_STATE_UNLOCKED),
       did_create_and_initialize_first_output_surface_(false) {}
 
 const char* SchedulerStateMachine::OutputSurfaceStateToString(
@@ -69,8 +67,6 @@ const char* SchedulerStateMachine::CommitStateToString(CommitState state) {
       return "COMMIT_STATE_READY_TO_COMMIT";
     case COMMIT_STATE_WAITING_FOR_FIRST_DRAW:
       return "COMMIT_STATE_WAITING_FOR_FIRST_DRAW";
-    case COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW:
-      return "COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW";
   }
   NOTREACHED();
   return "???";
@@ -89,6 +85,44 @@ const char* SchedulerStateMachine::TextureStateToString(TextureState state) {
   return "???";
 }
 
+const char* SchedulerStateMachine::SynchronousReadbackStateToString(
+    SynchronousReadbackState state) {
+  switch (state) {
+    case READBACK_STATE_IDLE:
+      return "READBACK_STATE_IDLE";
+    case READBACK_STATE_FORCED_COMMIT_REQUESTED:
+      return "READBACK_STATE_FORCED_COMMIT_REQUESTED";
+    case READBACK_STATE_FORCED_COMMIT_PENDING:
+      return "READBACK_STATE_FORCED_COMMIT_PENDING";
+    case READBACK_STATE_WAITING_FOR_ACTIVATION:
+      return "READBACK_STATE_WAITING_FOR_ACTIVATION";
+    case READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK:
+      return "READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK";
+    case READBACK_STATE_REPLACEMENT_COMMIT_PENDING:
+      return "READBACK_STATE_REPLACEMENT_COMMIT_PENDING";
+    case READBACK_STATE_REPLACEMENT_COMMIT_ACTIVATING:
+      return "READBACK_STATE_REPLACEMENT_COMMIT_ACTIVATING";
+  }
+  NOTREACHED();
+  return "???";
+}
+
+const char* SchedulerStateMachine::ForcedRedrawOnTimeoutStateToString(
+    ForcedRedrawOnTimeoutState state) {
+  switch (state) {
+    case FORCED_REDRAW_STATE_IDLE:
+      return "FORCED_REDRAW_STATE_IDLE";
+    case FORCED_REDRAW_STATE_WAITING_FOR_COMMIT:
+      return "FORCED_REDRAW_STATE_WAITING_FOR_COMMIT";
+    case FORCED_REDRAW_STATE_WAITING_FOR_ACTIVATION:
+      return "FORCED_REDRAW_STATE_WAITING_FOR_ACTIVATION";
+    case FORCED_REDRAW_STATE_WAITING_FOR_DRAW:
+      return "FORCED_REDRAW_STATE_WAITING_FOR_DRAW";
+  }
+  NOTREACHED();
+  return "???";
+}
+
 const char* SchedulerStateMachine::ActionToString(Action action) {
   switch (action) {
     case ACTION_NONE:
@@ -101,12 +135,14 @@ const char* SchedulerStateMachine::ActionToString(Action action) {
       return "ACTION_UPDATE_VISIBLE_TILES";
     case ACTION_ACTIVATE_PENDING_TREE:
       return "ACTION_ACTIVATE_PENDING_TREE";
-    case ACTION_DRAW_IF_POSSIBLE:
-      return "ACTION_DRAW_IF_POSSIBLE";
-    case ACTION_DRAW_FORCED:
-      return "ACTION_DRAW_FORCED";
+    case ACTION_DRAW_AND_SWAP_IF_POSSIBLE:
+      return "ACTION_DRAW_AND_SWAP_IF_POSSIBLE";
+    case ACTION_DRAW_AND_SWAP_FORCED:
+      return "ACTION_DRAW_AND_SWAP_FORCED";
     case ACTION_DRAW_AND_SWAP_ABORT:
       return "ACTION_DRAW_AND_SWAP_ABORT";
+    case ACTION_DRAW_AND_READBACK:
+      return "ACTION_DRAW_AND_READBACK";
     case ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
       return "ACTION_BEGIN_OUTPUT_SURFACE_CREATION";
     case ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD:
@@ -126,6 +162,11 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const  {
                          TextureStateToString(texture_state_));
   major_state->SetString("output_surface_state_",
                          OutputSurfaceStateToString(output_surface_state_));
+  major_state->SetString(
+      "forced_redraw_state",
+      ForcedRedrawOnTimeoutStateToString(forced_redraw_state_));
+  major_state->SetString("readback_state",
+                         SynchronousReadbackStateToString(readback_state_));
   state->Set("major_state", major_state.release());
 
   scoped_ptr<base::DictionaryValue> timestamps_state(new base::DictionaryValue);
@@ -137,7 +178,7 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const  {
       (last_begin_frame_args_.deadline - now).InMicroseconds() / 1000.0L);
   timestamps_state->SetDouble(
       "2_frame_time_to_now",
-      (last_begin_frame_args_.deadline - now).InMicroseconds() / 1000.0L);
+      (now - last_begin_frame_args_.frame_time).InMicroseconds() / 1000.0L);
   timestamps_state->SetDouble(
       "3_frame_time_to_deadline",
       (last_begin_frame_args_.deadline - last_begin_frame_args_.frame_time)
@@ -162,7 +203,7 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const  {
       "last_frame_number_where_begin_frame_sent_to_main_thread",
       last_frame_number_where_begin_frame_sent_to_main_thread_);
   minor_state->SetInteger("last_frame_number_where_draw_was_called",
-                          last_frame_number_where_draw_was_called_);
+                          last_frame_number_swap_performed_);
   minor_state->SetInteger(
       "last_frame_number_where_update_visible_tiles_was_called",
       last_frame_number_where_update_visible_tiles_was_called_);
@@ -174,13 +215,7 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const  {
   minor_state->SetBoolean("needs_redraw", needs_redraw_);
   minor_state->SetBoolean("swap_used_incomplete_tile",
                           swap_used_incomplete_tile_);
-  minor_state->SetBoolean("needs_forced_redraw", needs_forced_redraw_);
-  minor_state->SetBoolean("needs_forced_redraw_after_next_commit",
-                          needs_forced_redraw_after_next_commit_);
   minor_state->SetBoolean("needs_commit", needs_commit_);
-  minor_state->SetBoolean("needs_forced_commit", needs_forced_commit_);
-  minor_state->SetBoolean("expect_immediate_begin_frame_for_main_thread",
-                          expect_immediate_begin_frame_for_main_thread_);
   minor_state->SetBoolean("main_thread_needs_layer_textures",
                           main_thread_needs_layer_textures_);
   minor_state->SetBoolean("inside_begin_frame", inside_begin_frame_);
@@ -201,7 +236,7 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const  {
 }
 
 bool SchedulerStateMachine::HasDrawnThisFrame() const {
-  return current_frame_number_ == last_frame_number_where_draw_was_called_;
+  return current_frame_number_ == last_frame_number_swap_performed_;
 }
 
 bool SchedulerStateMachine::HasUpdatedVisibleTilesThisFrame() const {
@@ -272,14 +307,17 @@ bool SchedulerStateMachine::ShouldDraw() const {
   if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)

[brianderson, 2013/08/28 02:52:08]

This is no longer needed because of changes to the abort draw / forced
activation timing and because of the following line in UpdateStateOnDraw:
else if (commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW &&
!has_pending_tree_)

     return false;
 
-  // When we are waiting for a forced draw, only force draw once
-  // needs_forced_redraw_ is true.
-  if (commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW)
-    return needs_forced_redraw_;
+  // After a readback, make sure not to draw again until we've replaced the
+  // readback commit with a real one.
+  if (readback_state_ == READBACK_STATE_REPLACEMENT_COMMIT_PENDING ||
+      readback_state_ == READBACK_STATE_REPLACEMENT_COMMIT_ACTIVATING)
+    return false;
 
-  // Always handle forced draws ASAP.
-  if (needs_forced_redraw_)
+  // Draw immediately for readbacks to unblock the main thread quickly.
+  if (readback_state_ == READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK) {
+    DCHECK_EQ(commit_state_, COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
     return true;
+  }
 
   // If we need to abort draws, we should do so ASAP since the draw could
   // be blocking other important actions (like output surface initialization),
@@ -297,6 +335,12 @@ bool SchedulerStateMachine::ShouldDraw() const {
   if (!inside_begin_frame_)
     return false;
 
+  // Only handle forced redraws due to timeouts on the regular deadline.
+  if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW) {
+    DCHECK_EQ(commit_state_, COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
+    return true;
+  }
+
   return needs_redraw_;
 }
 
@@ -314,12 +358,12 @@ bool SchedulerStateMachine::ShouldActivatePendingTree() const {
   if (!has_pending_tree_)
     return false;
 
+  // If this is our first activation, we don't want to activate early.
+  if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
+    return pending_tree_is_ready_for_activation_;
+
   // We don't want to activate a second tree before drawing the first one.
-  // Note: It is possible that there is no active tree to draw when
-  // output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION,
-  // so we don't block activation on draw in that case.
-  if (!active_tree_has_been_drawn_ &&
-      output_surface_state_ != OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
+  if (!active_tree_has_been_drawn_)
     return false;
 
   // If we want to force activation, do so ASAP.
@@ -366,9 +410,12 @@ bool SchedulerStateMachine::ShouldSendBeginFrameToMainThread() const {
   if (has_pending_tree_)
     return false;
 
-  // We want to start forced commits ASAP.
-  if (needs_forced_commit_)
-    return true;
+  // We want to handle readback commits immediately to unblock the main thread.
+  // Note: This BeginFrame will correspond to the replacement commit that comes
+  // after the readback commit itself, so we only send the BeginFrame if a
+  // commit isn't already pending behind the readback.
+  if (readback_state_ == READBACK_STATE_FORCED_COMMIT_REQUESTED)
+    return !CommitPending();
 
   // We do not need commits if we are not visible, unless there's a
   // request for a forced commit.
@@ -379,6 +426,11 @@ bool SchedulerStateMachine::ShouldSendBeginFrameToMainThread() const {
   if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT)
     return true;
 
+  // We need a new commit for the forced redraw. This honors the
+  // single commit per interval because the result will be swapped to screen.
+  if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_COMMIT)
+    return true;
+
   // After this point, we only start a commit once per frame.
   if (HasSentBeginFrameToMainThreadThisFrame())
     return false;
@@ -404,12 +456,14 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
   if (ShouldCommit())
     return ACTION_COMMIT;
   if (ShouldDraw()) {
-    if (needs_forced_redraw_)
-      return ACTION_DRAW_FORCED;
+    if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW)
+      return ACTION_DRAW_AND_SWAP_FORCED;
+    else if (readback_state_ == READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK)
+      return ACTION_DRAW_AND_READBACK;
     else if (PendingDrawsShouldBeAborted())
       return ACTION_DRAW_AND_SWAP_ABORT;
     else
-      return ACTION_DRAW_IF_POSSIBLE;
+      return ACTION_DRAW_AND_SWAP_IF_POSSIBLE;
   }
   if (ShouldSendBeginFrameToMainThread())
     return ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD;
@@ -418,6 +472,13 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
   return ACTION_NONE;
 }
 
+void SchedulerStateMachine::CheckInvariants() {
+  // We should never try to perform a draw for readback and forced draw due to
+  // timeout simultaneously.
+  DCHECK(!(forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW &&
+           readback_state_ == READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK));
+}
+
 void SchedulerStateMachine::UpdateState(Action action) {
   switch (action) {
     case ACTION_NONE:
@@ -434,15 +495,12 @@ void SchedulerStateMachine::UpdateState(Action action) {
 
     case ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD:
       DCHECK(!has_pending_tree_);
-      if (!needs_forced_commit_ &&
-          output_surface_state_ != OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT) {
-        DCHECK(visible_);
-        DCHECK_GT(current_frame_number_,
-                  last_frame_number_where_begin_frame_sent_to_main_thread_);
-      }
+      DCHECK(visible_ ||
+             readback_state_ == READBACK_STATE_FORCED_COMMIT_REQUESTED ||
+             forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_COMMIT ||
+             output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT);
       commit_state_ = COMMIT_STATE_FRAME_IN_PROGRESS;
       needs_commit_ = false;
-      needs_forced_commit_ = false;
       last_frame_number_where_begin_frame_sent_to_main_thread_ =
           current_frame_number_;
       return;
@@ -453,14 +511,15 @@ void SchedulerStateMachine::UpdateState(Action action) {
       return;
     }
 
-    case ACTION_DRAW_FORCED:
-    case ACTION_DRAW_IF_POSSIBLE: {
+    case ACTION_DRAW_AND_SWAP_FORCED:
+    case ACTION_DRAW_AND_SWAP_IF_POSSIBLE: {
       bool did_swap = true;
       UpdateStateOnDraw(did_swap);
       return;
     }
 
-    case ACTION_DRAW_AND_SWAP_ABORT: {
+    case ACTION_DRAW_AND_SWAP_ABORT:
+    case ACTION_DRAW_AND_READBACK: {
       bool did_swap = false;
       UpdateStateOnDraw(did_swap);
       return;
@@ -487,14 +546,6 @@ void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
   has_pending_tree_ =
       settings_.impl_side_painting && !commit_was_aborted;
 
-  // Update the commit state.
-  if (expect_immediate_begin_frame_for_main_thread_)
-    commit_state_ = COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW;
-  else if (!commit_was_aborted)
-    commit_state_ = COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
-  else
-    commit_state_ = COMMIT_STATE_IDLE;
-
   // Update the output surface state.
   DCHECK_NE(output_surface_state_,
             OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION);
@@ -507,10 +558,47 @@ void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
     }
   }
 
+  // Update state related to readbacks.
+  if (readback_state_ == READBACK_STATE_FORCED_COMMIT_PENDING) {
+    // Update the state if this is the readback commit.
+    readback_state_ = has_pending_tree_
+                          ? READBACK_STATE_WAITING_FOR_ACTIVATION
+                          : READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK;
+  } else if (readback_state_ == READBACK_STATE_REPLACEMENT_COMMIT_PENDING) {
+    // Update the state if this is the commit replacing the readback commit.
+    readback_state_ = has_pending_tree_
+                          ? READBACK_STATE_REPLACEMENT_COMMIT_ACTIVATING
+                          : READBACK_STATE_IDLE;
+  } else {
+    DCHECK(readback_state_ == READBACK_STATE_IDLE);
+  }
+
+  // Update state related to forced draws.
+  // Note: It's possible for the readback's replacement commit to be the
+  // forced redraw's commit.
+  if (readback_state_ == READBACK_STATE_IDLE &&
+      forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_COMMIT) {
+    forced_redraw_state_ = has_pending_tree_
+                               ? FORCED_REDRAW_STATE_WAITING_FOR_ACTIVATION
+                               : FORCED_REDRAW_STATE_WAITING_FOR_DRAW;
+  }
+
+  // Update the commit state. We expect and wait for a draw if the commit
+  // was not aborted or if we in a readback or forced draw.
+  if (!commit_was_aborted)
+    commit_state_ = COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
+  else if (readback_state_ != READBACK_STATE_IDLE ||
+           forced_redraw_state_ != FORCED_REDRAW_STATE_IDLE)
+    commit_state_ = COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
+  else
+    commit_state_ = COMMIT_STATE_IDLE;
+
   // Update state if we have a new active tree to draw, or if the active tree
-  // was unchanged but we need to do a readback.
+  // was unchanged but we need to do a readback or forced draw.
   if (!has_pending_tree_ &&
-      (!commit_was_aborted || expect_immediate_begin_frame_for_main_thread_)) {
+      (!commit_was_aborted ||
+       readback_state_ == READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK ||
+       forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW)) {
     needs_redraw_ = true;
     active_tree_has_been_drawn_ = false;
   }
@@ -519,25 +607,31 @@ void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
   pending_tree_is_ready_for_activation_ = false;
 
   if (draw_if_possible_failed_)
-    last_frame_number_where_draw_was_called_ = -1;
-
-  if (needs_forced_redraw_after_next_commit_) {
-    needs_forced_redraw_after_next_commit_ = false;
-    needs_forced_redraw_ = true;
-  }
+    last_frame_number_swap_performed_ = -1;
 
   // If we are planing to draw with the new commit, lock the layer textures for
   // use on the impl thread. Otherwise, leave them unlocked.
-  if (has_pending_tree_ || needs_redraw_ || needs_forced_redraw_)
+  if (has_pending_tree_ || needs_redraw_)
     texture_state_ = LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD;
   else
     texture_state_ = LAYER_TEXTURE_STATE_UNLOCKED;
 }
 
 void SchedulerStateMachine::UpdateStateOnActivation() {
+  // Update output surface state.
   if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
     output_surface_state_ = OUTPUT_SURFACE_ACTIVE;
 
+  // Update readback state
+  if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_ACTIVATION)
+    forced_redraw_state_ = FORCED_REDRAW_STATE_WAITING_FOR_DRAW;
+
+  // Update forced redraw state
+  if (readback_state_ == READBACK_STATE_WAITING_FOR_ACTIVATION)
+    readback_state_ = READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK;
+  else if (readback_state_ == READBACK_STATE_REPLACEMENT_COMMIT_ACTIVATING)
+    readback_state_ = READBACK_STATE_IDLE;
+
   has_pending_tree_ = false;
   pending_tree_is_ready_for_activation_ = false;
   active_tree_has_been_drawn_ = false;
@@ -545,25 +639,37 @@ void SchedulerStateMachine::UpdateStateOnActivation() {
 }
 
 void SchedulerStateMachine::UpdateStateOnDraw(bool did_swap) {
-  if (inside_begin_frame_)
-    last_frame_number_where_draw_was_called_ = current_frame_number_;
-  if (commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW) {
-    DCHECK(expect_immediate_begin_frame_for_main_thread_);
+  DCHECK(needs_redraw_);
+
+  if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW) {
+    DCHECK_EQ(commit_state_, COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
+    commit_state_ = COMMIT_STATE_IDLE;
+    forced_redraw_state_ = FORCED_REDRAW_STATE_IDLE;
+  } else if (readback_state_ == READBACK_STATE_WAITING_FOR_DRAW_AND_READBACK) {
+    // The draw correspons to a readback commit.
+    DCHECK_EQ(commit_state_, COMMIT_STATE_WAITING_FOR_FIRST_DRAW);
+    // We are blocking commits from the main thread until after this draw, so
+    // we should not have a pending tree.
+    DCHECK(!has_pending_tree_);
+    // We transition to COMMIT_STATE_FRAME_IN_PROGRESS because there is a
+    // pending BeginFrame on the main thread behind the readback request.
     commit_state_ = COMMIT_STATE_FRAME_IN_PROGRESS;
-    expect_immediate_begin_frame_for_main_thread_ = false;
+    readback_state_ = READBACK_STATE_REPLACEMENT_COMMIT_PENDING;
   } else if (commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW) {
     commit_state_ = COMMIT_STATE_IDLE;
   }
+
   if (texture_state_ == LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD)
     texture_state_ = LAYER_TEXTURE_STATE_UNLOCKED;
 
   needs_redraw_ = false;
-  needs_forced_redraw_ = false;
   draw_if_possible_failed_ = false;
   active_tree_has_been_drawn_ = true;
 
-  if (did_swap)
+  if (did_swap) {
     swap_used_incomplete_tile_ = false;
+    last_frame_number_swap_performed_ = current_frame_number_;
+  }
 }
 
 void SchedulerStateMachine::SetMainThreadNeedsLayerTextures() {
@@ -572,20 +678,43 @@ void SchedulerStateMachine::SetMainThreadNeedsLayerTextures() {
   main_thread_needs_layer_textures_ = true;
 }
 
+// These are the cases where we definitely (or almost definitely) have a
+// new frame to draw and can draw.
 bool SchedulerStateMachine::BeginFrameNeededToDrawByImplThread() const {
+  // The output surface is the provider of BeginFrames for the impl thread,
+  // so we are not going to get them even if we ask for them.
+  if (!HasInitializedOutputSurface())
+    return false;
+
   // If we can't draw, don't tick until we are notified that we can draw again.
   if (!can_draw_)
     return false;
 
-  if (needs_forced_redraw_)
+  // The forced draw respects our normal draw scheduling, so we need to
+  // request a BeginFrame on the impl thread for it.
+  if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW)
     return true;
 
-  if (visible_ && swap_used_incomplete_tile_)
+  // There's no need to produce frames if we are not visible.
+  if (!visible_)
+    return false;
+
+  // We need to draw a more complete frame than we did the last BeginFrame,
+  // so request another BeginFrame in anticipation that we will have
+  // additional visible tiles.
+  if (swap_used_incomplete_tile_)
     return true;
 
-  return needs_redraw_ && visible_ && HasInitializedOutputSurface();
+  return needs_redraw_;
 }
 
+// These are cases where we are very likely to draw soon, but might not
+// actually have a new frame to draw when we receive the next BeginFrame.
+// Proactively requesting the BeginFrame helps hide the round trip latency of
+// the SetNeedsBeginFrame request that has to go to the Browser.
+// However, this is bad for the synchronous compoisitor because we have to
+// draw when we get the BeginFrame and could end up drawing many duplicate
+// frames.
 bool SchedulerStateMachine::ProactiveBeginFrameWantedByImplThread() const {
   // The output surface is the provider of BeginFrames for the impl thread,
   // so we are not going to get them even if we ask for them.
@@ -596,10 +725,17 @@ bool SchedulerStateMachine::ProactiveBeginFrameWantedByImplThread() const {
   if (!visible_)
     return false;
 
-  // We should proactively request a BeginFrame if a commit or a tree activation
-  // is pending.
-  return (needs_commit_ || needs_forced_commit_ ||
-          commit_state_ != COMMIT_STATE_IDLE || has_pending_tree_);
+  // We should proactively request a BeginFrame if a commit is pending
+  // because we will want to draw if the commit completes quickly.
+  if (needs_commit_ || commit_state_ != COMMIT_STATE_IDLE)
+    return true;
+
+  // If the pending tree activates quickly, we'll want a BeginFrame soon
+  // to draw the new active tree.
+  if (has_pending_tree_)
+    return true;
+
+  return false;
 }
 
 void SchedulerStateMachine::DidEnterBeginFrame(const BeginFrameArgs& args) {
@@ -620,10 +756,6 @@ void SchedulerStateMachine::DidSwapUseIncompleteTile() {
   swap_used_incomplete_tile_ = true;
 }
 
-void SchedulerStateMachine::SetNeedsForcedRedraw() {
-  needs_forced_redraw_ = true;
-}
-
 void SchedulerStateMachine::DidDrawIfPossibleCompleted(bool success) {
   draw_if_possible_failed_ = !success;
   if (draw_if_possible_failed_) {
@@ -636,7 +768,7 @@ void SchedulerStateMachine::DidDrawIfPossibleCompleted(bool success) {
       consecutive_failed_draws_ = 0;
       // We need to force a draw, but it doesn't make sense to do this until
       // we've committed and have new textures.
-      needs_forced_redraw_after_next_commit_ = true;
+      forced_redraw_state_ = FORCED_REDRAW_STATE_WAITING_FOR_COMMIT;
     }
   } else {
     consecutive_failed_draws_ = 0;
@@ -645,15 +777,28 @@ void SchedulerStateMachine::DidDrawIfPossibleCompleted(bool success) {
 
 void SchedulerStateMachine::SetNeedsCommit() { needs_commit_ = true; }
 
-void SchedulerStateMachine::SetNeedsForcedCommit() {
-  needs_forced_commit_ = true;
-  expect_immediate_begin_frame_for_main_thread_ = true;
+void SchedulerStateMachine::SetNeedsForcedCommitForReadback() {
+  // If this is called in READBACK_STATE_IDLE, this is a "first" readback
+  // request.
+  // If this is called in READBACK_STATE_REPLACEMENT_COMMIT_PENDING, this
+  // is a back-to-back readback request that started before the replacement
+  // commit had a chance to land.
+  DCHECK(readback_state_ == READBACK_STATE_IDLE ||
+         readback_state_ == READBACK_STATE_REPLACEMENT_COMMIT_PENDING);
+
+  // If there is already a commit in progress when we get the readback request
+  // (we are in COMMIT_STATE_FRAME_IN_PROGRESS), then we don't need to send a
+  // BeginFrame for the replacement commit, since there's already a BeginFrame
+  // behind the readback request.
+  if (commit_state_ == COMMIT_STATE_FRAME_IN_PROGRESS)
+    readback_state_ = READBACK_STATE_FORCED_COMMIT_PENDING;
+  else
+    readback_state_ = READBACK_STATE_FORCED_COMMIT_REQUESTED;
 }
 
 void SchedulerStateMachine::FinishCommit() {
   DCHECK(commit_state_ == COMMIT_STATE_FRAME_IN_PROGRESS ||
-         (expect_immediate_begin_frame_for_main_thread_ &&
-          commit_state_ != COMMIT_STATE_IDLE))
+         readback_state_ == READBACK_STATE_FORCED_COMMIT_PENDING)
       << *AsValue();
   commit_state_ = COMMIT_STATE_READY_TO_COMMIT;
 }
@@ -663,8 +808,8 @@ void SchedulerStateMachine::BeginFrameAbortedByMainThread(bool did_handle) {
   if (did_handle) {
     bool commit_was_aborted = true;
     UpdateStateOnCommit(commit_was_aborted);
-  } else if (expect_immediate_begin_frame_for_main_thread_) {
-    expect_immediate_begin_frame_for_main_thread_ = false;
+  } else if (readback_state_ == READBACK_STATE_FORCED_COMMIT_PENDING) {
+    readback_state_ = READBACK_STATE_IDLE;
   } else {
     commit_state_ = COMMIT_STATE_IDLE;
     SetNeedsCommit();