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 704259beafd6ea9f86ea2d44c5fdc812236225d7..e93d0c84a2c80f7b2e50bef61a2b98b0ab88c08e 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -15,20 +15,18 @@ 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),
@@ -36,9 +34,8 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       can_draw_(false),
       has_pending_tree_(false),
       pending_tree_is_ready_for_activation_(false),
-      active_tree_has_been_drawn_(false),
+      active_tree_needs_first_draw_(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 +66,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 +84,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_NEEDS_BEGIN_FRAME:
+      return "READBACK_STATE_NEEDS_BEGIN_FRAME";
+    case READBACK_STATE_WAITING_FOR_COMMIT:
+      return "READBACK_STATE_WAITING_FOR_COMMIT";
+    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_WAITING_FOR_REPLACEMENT_COMMIT:
+      return "READBACK_STATE_WAITING_FOR_REPLACEMENT_COMMIT";
+    case READBACK_STATE_WAITING_FOR_REPLACEMENT_ACTIVATION:
+      return "READBACK_STATE_WAITING_FOR_REPLACEMENT_ACTIVATION";
+  }
+  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 +134,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 +161,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 +177,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)
@@ -161,26 +201,17 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const  {
   minor_state->SetInteger(
       "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_);
+  minor_state->SetInteger("last_frame_number_swap_performed_",
+                          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_);
   minor_state->SetInteger("consecutive_failed_draws",
                           consecutive_failed_draws_);
-  minor_state->SetInteger(
-      "maximum_number_of_failed_draws_before_draw_is_forced",
-      maximum_number_of_failed_draws_before_draw_is_forced_);
   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_);
@@ -190,8 +221,8 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const  {
   minor_state->SetBoolean("has_pending_tree", has_pending_tree_);
   minor_state->SetBoolean("pending_tree_is_ready_for_activation_",
                           pending_tree_is_ready_for_activation_);
-  minor_state->SetBoolean("active_tree_has_been_drawn_",
-                          active_tree_has_been_drawn_);
+  minor_state->SetBoolean("active_tree_needs_first_draw_",
+                          active_tree_needs_first_draw_);
   minor_state->SetBoolean("draw_if_possible_failed", draw_if_possible_failed_);
   minor_state->SetBoolean("did_create_and_initialize_first_output_surface",
                           did_create_and_initialize_first_output_surface_);
@@ -200,8 +231,8 @@ scoped_ptr<base::Value> SchedulerStateMachine::AsValue() const  {
   return state.PassAs<base::Value>();
 }
 
-bool SchedulerStateMachine::HasDrawnThisFrame() const {
-  return current_frame_number_ == last_frame_number_where_draw_was_called_;
+bool SchedulerStateMachine::HasDrawnAndSwappedThisFrame() const {
+  return current_frame_number_ == last_frame_number_swap_performed_;
 }
 
 bool SchedulerStateMachine::HasUpdatedVisibleTilesThisFrame() const {
@@ -231,20 +262,22 @@ bool SchedulerStateMachine::PendingDrawsShouldBeAborted() const {
     return true;
   if (!visible_)
     return true;
-  if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
-    return true;
   return false;
 }
 
 bool SchedulerStateMachine::PendingActivationsShouldBeForced() const {
   // These are all the cases where, if we do not force activations to make
   // forward progress, we might deadlock with the main thread.
+
+  // The impl thread cannot lock layer textures unless the pending
+  // tree can be activated to unblock the commit.
   if (texture_state_ == LAYER_TEXTURE_STATE_ACQUIRED_BY_MAIN_THREAD)
     return true;
-  if (output_surface_state_ == OUTPUT_SURFACE_LOST ||
-      output_surface_state_ == OUTPUT_SURFACE_CREATING ||
-      output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT)
+
+  // There is no output surface to trigger our activations.
+  if (output_surface_state_ == OUTPUT_SURFACE_LOST)
     return true;
+
   return false;
 }
 
@@ -258,28 +291,31 @@ bool SchedulerStateMachine::ShouldBeginOutputSurfaceCreation() const {
   if (commit_state_ != COMMIT_STATE_IDLE)
     return false;
 
+  // We want to clear the pipline of any pending draws and activations
+  // before starting output surface initialization. This allows us to avoid
+  // weird corner cases where we abort draws or force activation while we
+  // are initializing the output surface and can potentially have a pending
+  // readback.
+  if (active_tree_needs_first_draw_ || has_pending_tree_)
+    return false;
+
   // We need to create the output surface if we don't have one and we haven't
   // started creating one yet.
   return output_surface_state_ == OUTPUT_SURFACE_LOST;
 }
 
 bool SchedulerStateMachine::ShouldDraw() const {
-  // We should not draw or abort draws while we are waiting for the
-  // first activation. Doing so will cause us to transition to
-  // COMMIT_STATE_IDLE and start the 2nd commit before the 1st commit
-  // has been drawn. It will also cause us to fail readbacks when we could
-  // have succeeded by waiting a little longer.
-  if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
+  // 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_WAITING_FOR_REPLACEMENT_COMMIT ||
+      readback_state_ == READBACK_STATE_WAITING_FOR_REPLACEMENT_ACTIVATION)
     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_;
-
-  // 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),
@@ -287,16 +323,22 @@ bool SchedulerStateMachine::ShouldDraw() const {
   // aborted draw to keep things moving. If we are not waiting for the first
   // draw however, we don't want to abort for no reason.
   if (PendingDrawsShouldBeAborted())
-    return commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
+    return active_tree_needs_first_draw_;
 
   // After this line, we only want to draw once per frame.
-  if (HasDrawnThisFrame())
+  if (HasDrawnAndSwappedThisFrame())
     return false;
 
   // We currently only draw within the BeginFrame.
   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 +356,10 @@ bool SchedulerStateMachine::ShouldActivatePendingTree() const {
   if (!has_pending_tree_)
     return false;
 
-  // 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)
+  // We should not activate a second tree before drawing the first one.
+  // Even if we need to force activation of the pending tree, we should abort
+  // drawing the active tree first.
+  if (active_tree_needs_first_draw_)
     return false;
 
   // If we want to force activation, do so ASAP.
@@ -366,9 +406,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_NEEDS_BEGIN_FRAME)
+    return !CommitPending();
 
   // We do not need commits if we are not visible, unless there's a
   // request for a forced commit.
@@ -379,6 +422,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 +452,14 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
   if (ShouldCommit())
     return ACTION_COMMIT;
   if (ShouldDraw()) {
-    if (needs_forced_redraw_)
-      return ACTION_DRAW_FORCED;
+    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 if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW)
+      return ACTION_DRAW_AND_SWAP_FORCED;
     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 +468,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 +491,11 @@ 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_NEEDS_BEGIN_FRAME);
       commit_state_ = COMMIT_STATE_FRAME_IN_PROGRESS;
       needs_commit_ = false;
-      needs_forced_commit_ = false;
+      if (readback_state_ == READBACK_STATE_NEEDS_BEGIN_FRAME)
+        readback_state_ = READBACK_STATE_WAITING_FOR_COMMIT;
       last_frame_number_where_begin_frame_sent_to_main_thread_ =
           current_frame_number_;
       return;
@@ -453,23 +506,30 @@ 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;
     }
 
     case ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
-      DCHECK_EQ(commit_state_, COMMIT_STATE_IDLE);
       DCHECK_EQ(output_surface_state_, OUTPUT_SURFACE_LOST);
       output_surface_state_ = OUTPUT_SURFACE_CREATING;
+
+      // The following DCHECKs make sure we are in the proper quiescent state.
+      // The pipeline should be flushed entirely before we start output
+      // surface creation to avoid complicated corner cases.
+      DCHECK_EQ(commit_state_, COMMIT_STATE_IDLE);
+      DCHECK(!has_pending_tree_);
+      DCHECK(!active_tree_needs_first_draw_);
       return;
 
     case ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD:
@@ -486,82 +546,137 @@ void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
   // mostly as if we are not impl-side-painting since there is no pending tree.
   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 state related to readbacks.
+  if (readback_state_ == READBACK_STATE_WAITING_FOR_COMMIT) {
+    // 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_WAITING_FOR_REPLACEMENT_COMMIT) {
+    // Update the state if this is the commit replacing the readback commit.
+    readback_state_ = has_pending_tree_
+                          ? READBACK_STATE_WAITING_FOR_REPLACEMENT_ACTIVATION
+                          : READBACK_STATE_IDLE;
+  } else {
+    DCHECK(readback_state_ == READBACK_STATE_IDLE);
+  }
 
-  // Update the output surface state.
-  DCHECK_NE(output_surface_state_, OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION);
-  if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT) {
-    if (has_pending_tree_) {
-      output_surface_state_ = OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION;
-    } else {
-      output_surface_state_ = OUTPUT_SURFACE_ACTIVE;
-      needs_redraw_ = true;
+  // Readbacks can interrupt output surface initialization and forced draws,
+  // so we do not want to advance those states if we are in the middle of a
+  // readback. Note: It is possible for the readback's replacement commit to
+  // be the output surface's first commit and/or the forced redraw's commit.
+  if (readback_state_ == READBACK_STATE_IDLE ||
+      readback_state_ == READBACK_STATE_WAITING_FOR_REPLACEMENT_ACTIVATION) {
+    // Update state related to forced draws.
+    if (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 output surface state.
+    DCHECK_NE(output_surface_state_,
+              OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION);
+    if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT) {
+      if (has_pending_tree_) {
+        output_surface_state_ = OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION;
+      } else {
+        output_surface_state_ = OUTPUT_SURFACE_ACTIVE;
+        needs_redraw_ = true;
+      }
     }
   }
 
+  // Update the commit state. We expect and wait for a draw if the commit
+  // was not aborted or if we are 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;
+    active_tree_needs_first_draw_ = true;
   }
 
   // This post-commit work is common to both completed and aborted commits.
   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_WAITING_FOR_REPLACEMENT_ACTIVATION)
+    readback_state_ = READBACK_STATE_IDLE;
+
   has_pending_tree_ = false;
   pending_tree_is_ready_for_activation_ = false;
-  active_tree_has_been_drawn_ = false;
+  active_tree_needs_first_draw_ = true;
   needs_redraw_ = true;
 }
 
 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(readback_state_ != READBACK_STATE_WAITING_FOR_REPLACEMENT_COMMIT &&
+         readback_state_ != READBACK_STATE_WAITING_FOR_REPLACEMENT_ACTIVATION)
+      << *AsValue();
+
+  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;
-  } else if (commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW) {
+    readback_state_ = READBACK_STATE_WAITING_FOR_REPLACEMENT_COMMIT;
+  } else 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 (commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW &&
+             !has_pending_tree_) {
     commit_state_ = COMMIT_STATE_IDLE;

[brianderson, 2013/10/04 17:17:44]

If the active tree was drawn twice (because of an animation or otherwise) ToT
prior to this patch would start the next commit before activation because it
would transition straight to idle here.  ToT doesn't (and didn't) have the logic
to enforce proper back pressure if a commit was started while there was still a
pending tree.

Scary. It seems to me that the pending tree could have just been overwritten
when the next commit came in?  This was also artificially inflating our perf
benchmarks. See https://code.google.com/p/chromium/issues/detail?id=287756 for
background.

It's also a good indication though that we should try to get
https://codereview.chromium.org/23907006 in, which does implement the proper
back pressure and starts the next commit without needing an additional draw to
kick it off.

   }
+
   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;
+  active_tree_needs_first_draw_ = false;
 
-  if (did_swap)
+  if (did_swap) {
     swap_used_incomplete_tile_ = false;
+    last_frame_number_swap_performed_ = current_frame_number_;
+  }
 }
 
 void SchedulerStateMachine::SetMainThreadNeedsLayerTextures() {
@@ -570,20 +685,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 compositor 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.
@@ -594,10 +732,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) {
@@ -618,10 +763,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_) {
@@ -630,11 +771,11 @@ void SchedulerStateMachine::DidDrawIfPossibleCompleted(bool success) {
     consecutive_failed_draws_++;
     if (settings_.timeout_and_draw_when_animation_checkerboards &&
         consecutive_failed_draws_ >=
-        maximum_number_of_failed_draws_before_draw_is_forced_) {
+            settings_.maximum_number_of_failed_draws_before_draw_is_forced_) {
       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;
@@ -643,16 +784,29 @@ 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_WAITING_FOR_REPLACEMENT_COMMIT, 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_WAITING_FOR_REPLACEMENT_COMMIT);
+
+  // 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. In that case, we can skip
+  // READBACK_STATE_NEEDS_BEGIN_FRAME and go directly to
+  // READBACK_STATE_WAITING_FOR_COMMIT
+  if (commit_state_ == COMMIT_STATE_FRAME_IN_PROGRESS)
+    readback_state_ = READBACK_STATE_WAITING_FOR_COMMIT;
+  else
+    readback_state_ = READBACK_STATE_NEEDS_BEGIN_FRAME;
 }
 
 void SchedulerStateMachine::FinishCommit() {
-  DCHECK(commit_state_ == COMMIT_STATE_FRAME_IN_PROGRESS ||
-         (expect_immediate_begin_frame_for_main_thread_ &&
-          commit_state_ != COMMIT_STATE_IDLE))
-      << *AsValue();
+  DCHECK(commit_state_ == COMMIT_STATE_FRAME_IN_PROGRESS) << *AsValue();
   commit_state_ = COMMIT_STATE_READY_TO_COMMIT;
 }
 
@@ -661,9 +815,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 {
+    DCHECK_NE(readback_state_, READBACK_STATE_WAITING_FOR_COMMIT);
     commit_state_ = COMMIT_STATE_IDLE;
     SetNeedsCommit();
   }
@@ -711,9 +864,4 @@ bool SchedulerStateMachine::HasInitializedOutputSurface() const {
   return false;
 }
 
-void SchedulerStateMachine::SetMaximumNumberOfFailedDrawsBeforeDrawIsForced(
-    int num_draws) {
-  maximum_number_of_failed_draws_before_draw_is_forced_ = num_draws;
-}
-
 }  // namespace cc