cc: Control activation from the Scheduler

cc/scheduler/scheduler_state_machine.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/scheduler/scheduler_state_machine.h"
 
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
 
 namespace cc {
 
 SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
     : settings_(settings),
       output_surface_state_(OUTPUT_SURFACE_LOST),
       commit_state_(COMMIT_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_where_tree_activation_attempted_(-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),
       can_start_(false),
       can_draw_(false),
       has_pending_tree_(false),
+      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(
     OutputSurfaceState state) {
   switch (state) {
     case OUTPUT_SURFACE_ACTIVE:
       return "OUTPUT_SURFACE_ACTIVE";
     case OUTPUT_SURFACE_LOST:
@@ skipping 42 matching lines @@
 const char* SchedulerStateMachine::ActionToString(Action action) {
   switch (action) {
     case ACTION_NONE:
       return "ACTION_NONE";
     case ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD:
       return "ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD";
     case ACTION_COMMIT:
       return "ACTION_COMMIT";
     case ACTION_UPDATE_VISIBLE_TILES:
       return "ACTION_UPDATE_VISIBLE_TILES";
-    case ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED:
-      return "ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED";
+    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_ABORT:
       return "ACTION_DRAW_AND_SWAP_ABORT";
     case ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
       return "ACTION_BEGIN_OUTPUT_SURFACE_CREATION";
     case ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD:
       return "ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD";
@@ skipping 42 matching lines @@
   state->Set("major_timestamps_in_ms", timestamps_state.release());
 
   scoped_ptr<base::DictionaryValue> minor_state(new base::DictionaryValue);
   minor_state->SetInteger("commit_count", commit_count_);
   minor_state->SetInteger("current_frame_number", current_frame_number_);
   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_where_tree_activation_attempted",
-                          last_frame_number_where_tree_activation_attempted_);
   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_);
   minor_state->SetBoolean("visible", visible_);
   minor_state->SetBoolean("can_start", can_start_);
   minor_state->SetBoolean("can_draw", can_draw_);
   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("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_);
   state->Set("minor_state", minor_state.release());
 
   return state.PassAs<base::Value>();
 }
 
 bool SchedulerStateMachine::HasDrawnThisFrame() const {
   return current_frame_number_ == last_frame_number_where_draw_was_called_;
 }
 
-bool SchedulerStateMachine::HasAttemptedTreeActivationThisFrame() const {
-  return current_frame_number_ ==
-         last_frame_number_where_tree_activation_attempted_;
-}
-
 bool SchedulerStateMachine::HasUpdatedVisibleTilesThisFrame() const {
   return current_frame_number_ ==
          last_frame_number_where_update_visible_tiles_was_called_;
 }
 
 bool SchedulerStateMachine::HasSentBeginFrameToMainThreadThisFrame() const {
   return current_frame_number_ ==
          last_frame_number_where_begin_frame_sent_to_main_thread_;
 }
 
-void SchedulerStateMachine::HandleCommitInternal(bool commit_was_aborted) {
-  commit_count_++;
-
-  // If we are impl-side-painting but the commit was aborted, then we behave
-  // mostly as if we are not impl-side-painting since there is no pending tree.
-  bool commit_results_in_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);
-  if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT) {
-    if (commit_results_in_pending_tree) {
-      output_surface_state_ = OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION;
-    } else {
-      output_surface_state_ = OUTPUT_SURFACE_ACTIVE;
-      needs_redraw_ = true;
-    }
-  }
-
-  // if we don't have to wait for activation, update needs_redraw now.
-  if (!commit_results_in_pending_tree) {
-    if (!commit_was_aborted)
-      needs_redraw_ = true;
-    if (expect_immediate_begin_frame_for_main_thread_)
-      needs_redraw_ = true;
-  }
-
-  // This post-commit work is common to both completed and aborted commits.
-  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;
-  }
-
-  // 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 (commit_results_in_pending_tree || needs_redraw_ || needs_forced_redraw_)
-    texture_state_ = LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD;
-  else
-    texture_state_ = LAYER_TEXTURE_STATE_UNLOCKED;
-}
-
 bool SchedulerStateMachine::PendingDrawsShouldBeAborted() const {
   // These are all the cases where, if we do not abort draws to make

[danakj, 2013/08/27 00:59:12]

"if we do not perform aborted draws while needs_redraw_ is true, even though we
would not normally try to draw at all, to make"

can you make this say something like ^ to help clarify why returning true if
PendingActivationsShouldBeForced() is correct, so I can remember this in the
future?

[brianderson, 2013/08/27 01:45:37]

Done.

   // forward progress, we might deadlock with the main thread.
+  // This should be a superset of PendingActivationsShouldBeForced() since
+  // activation of the pending tree is blocked by drawing of the active tree.
+  if (PendingActivationsShouldBeForced())
+    return true;
+
+  // Additional states where we should abort draws.
+  // Note: We don't force activation in these cases because doing so would
+  // result in checkerboarding on resize, becoming visible, etc.
   if (!can_draw_)
     return true;
   if (!visible_)
     return true;
-  if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE)
-    return true;
-  if (texture_state_ == LAYER_TEXTURE_STATE_ACQUIRED_BY_MAIN_THREAD)
+  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.
+  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)
+    return true;
+  return false;
+}
+
+bool SchedulerStateMachine::ShouldBeginOutputSurfaceCreation() const {
+  // Don't try to initialize too early.
+  if (!can_start_)
+    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)
+    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_)
     return true;
 
   // If we are going to abort draws, we should do so ASAP.

[danakj, 2013/08/27 00:59:12]

"...we should do so ASAP. If we're waiting for the first draw, then perform the
aborted draw to keep the scheduler moving forward. If we're not then do not draw
just to have it aborted for no reason."

can you say something along those lines so i can remember this too?

[brianderson, 2013/08/27 01:45:37]

Done.

-  if (PendingDrawsShouldBeAborted()) {
-    // TODO(brianderson): Remove the !has_pending_tree_ condition once
-    // the Scheduler controls activation. It's dangerous for us to rely on
-    // an eventual activation if we've lost the output surface.
-    return commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW &&
-           !has_pending_tree_;
-  }
+  if (PendingDrawsShouldBeAborted())
+    return commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW;
 
   // After this line, we only want to draw once per frame.
   if (HasDrawnThisFrame())
     return false;
 
   // We currently only draw within the BeginFrame.
   if (!inside_begin_frame_)
     return false;
 
   return needs_redraw_;
 }
 
-bool SchedulerStateMachine::ShouldAttemptTreeActivation() const {
-  return has_pending_tree_ && inside_begin_frame_ &&
-         !HasAttemptedTreeActivationThisFrame();
+bool SchedulerStateMachine::ShouldAcquireLayerTexturesForMainThread() const {
+  if (!main_thread_needs_layer_textures_)
+    return false;
+  if (texture_state_ == LAYER_TEXTURE_STATE_UNLOCKED)
+    return true;
+  DCHECK_EQ(texture_state_, LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD);
+  return false;
+}
+
+bool SchedulerStateMachine::ShouldActivatePendingTree() const {
+  // There is nothing to activate.
+  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)

[danakj, 2013/08/27 01:24:02]

What if the active tree hasn't been drawn because it can't?  I suppose then we'd
draw and abort to make active_tree_has_been_drawn be true, pushing this
activation out a vsync.

Why not check can_draw here, or even PendingDrawsShouldBeAborted() - name seems
out of place, but when that's true we won't draw, right? Would it make sense to
shorten that cycle and make this more direct?

[brianderson, 2013/08/27 01:45:37]

It is true that the draw will be aborted to unblock the activation. But the
aborted draw is not tied to vsync and the activation (forced or not) is not tied
to vsync either, so everything should happen very quickly.
I initially used PendingDrawsShouldBeAborted to force activations, but that
results in checkerboarding in many cases, which I describe in the comments at
the end of PendingDrawsShouldBeAborted.

[danakj, 2013/08/27 02:03:47]

Ah, so it's like PendingDrawsShouldBeAborted() == ForcePendingActivations() ||
UnableToDraw(). And then here we could allow activation when we haven't drawn if
UnableToDraw()?

I'm not sure where your next patches take this logic, so do you think that would
be better or worse than what's here now?

+    return false;
+
+  // If we want to force activation, do so ASAP.
+  if (PendingActivationsShouldBeForced())
+    return true;
+
+  // At this point, only activate if we are ready to activate.
+  return pending_tree_is_ready_for_activation_;
 }
 
 bool SchedulerStateMachine::ShouldUpdateVisibleTiles() const {
   if (!settings_.impl_side_painting)
     return false;
   if (HasUpdatedVisibleTilesThisFrame())
     return false;
 
-  return ShouldAttemptTreeActivation() || ShouldDraw() ||
+  // There's no reason to check for tiles if we don't have an output surface.
+  if (!HasInitializedOutputSurface())
+    return false;
+
+  return ShouldActivatePendingTree() || ShouldDraw() ||

[danakj, 2013/08/27 01:24:02]

Can you comment explaining why these bools are used for this method? Maybe split
the 3 up with a comment for each?

[brianderson, 2013/08/27 01:45:37]

Thanks for making me do this exercise! ShouldActivatePendingTree is not needed
anymore because we don't need to poll for activation anymore (like when it was
called SHouldAttemptTreeActivation).

          swap_used_incomplete_tile_;
 }
 
 bool SchedulerStateMachine::ShouldSendBeginFrameToMainThread() const {
   if (!needs_commit_)
     return false;
 
   // Only send BeginFrame to the main thread when idle.
   if (commit_state_ != COMMIT_STATE_IDLE)
     return false;
@@ skipping 19 matching lines @@
   if (!visible_)
     return false;
 
   // We shouldn't normally accept commits if there isn't an OutputSurface.
   if (!HasInitializedOutputSurface())
     return false;
 
   return true;
 }
 
-bool SchedulerStateMachine::ShouldAcquireLayerTexturesForMainThread() const {
-  if (!main_thread_needs_layer_textures_)
-    return false;
-  if (texture_state_ == LAYER_TEXTURE_STATE_UNLOCKED)
-    return true;
-  DCHECK_EQ(texture_state_, LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD);
-  return false;
-}
-
 SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
   if (ShouldAcquireLayerTexturesForMainThread())
     return ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD;
 
   switch (commit_state_) {
-    case COMMIT_STATE_IDLE: {
-      if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE &&
-          needs_forced_redraw_)
-        return ACTION_DRAW_FORCED;
-      if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE &&
-          needs_forced_commit_)
-        // TODO(enne): Should probably drop the active tree on force commit.
-        return has_pending_tree_ ? ACTION_NONE
-                                 : ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD;
-      if (output_surface_state_ == OUTPUT_SURFACE_LOST && can_start_)
-        return ACTION_BEGIN_OUTPUT_SURFACE_CREATION;
-      if (output_surface_state_ == OUTPUT_SURFACE_CREATING)
-        return ACTION_NONE;
+    case COMMIT_STATE_IDLE:
       if (ShouldUpdateVisibleTiles())
         return ACTION_UPDATE_VISIBLE_TILES;
-      if (ShouldAttemptTreeActivation())
-        return ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED;
+      if (ShouldActivatePendingTree())
+        return ACTION_ACTIVATE_PENDING_TREE;
       if (ShouldDraw()) {
         return needs_forced_redraw_ ? ACTION_DRAW_FORCED
                                     : ACTION_DRAW_IF_POSSIBLE;
       }
       if (ShouldSendBeginFrameToMainThread())
         return ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD;
+      if (ShouldBeginOutputSurfaceCreation())
+        return ACTION_BEGIN_OUTPUT_SURFACE_CREATION;
       return ACTION_NONE;
-    }
+
     case COMMIT_STATE_FRAME_IN_PROGRESS:
       if (ShouldUpdateVisibleTiles())
         return ACTION_UPDATE_VISIBLE_TILES;
-      if (ShouldAttemptTreeActivation())
-        return ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED;
+      if (ShouldActivatePendingTree())
+        return ACTION_ACTIVATE_PENDING_TREE;
       if (ShouldDraw()) {
         return needs_forced_redraw_ ? ACTION_DRAW_FORCED
                                     : ACTION_DRAW_IF_POSSIBLE;
       }
       return ACTION_NONE;
 
     case COMMIT_STATE_READY_TO_COMMIT:
       return ACTION_COMMIT;
 
-    case COMMIT_STATE_WAITING_FOR_FIRST_DRAW: {
+    case COMMIT_STATE_WAITING_FOR_FIRST_DRAW:
       if (ShouldUpdateVisibleTiles())
         return ACTION_UPDATE_VISIBLE_TILES;
-      if (ShouldAttemptTreeActivation())
-        return ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED;
+      if (ShouldActivatePendingTree())
+        return ACTION_ACTIVATE_PENDING_TREE;
       if (ShouldDraw()) {
         if (needs_forced_redraw_)
           return ACTION_DRAW_FORCED;
         else if (PendingDrawsShouldBeAborted())
           return ACTION_DRAW_AND_SWAP_ABORT;
         else
           return ACTION_DRAW_IF_POSSIBLE;
       }
       return ACTION_NONE;
-    }
 
     case COMMIT_STATE_WAITING_FOR_FIRST_FORCED_DRAW:
       if (ShouldUpdateVisibleTiles())
         return ACTION_UPDATE_VISIBLE_TILES;
-      if (ShouldAttemptTreeActivation())
-        return ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED;
-      if (needs_forced_redraw_)
+      if (ShouldActivatePendingTree())
+        return ACTION_ACTIVATE_PENDING_TREE;
+      if (ShouldDraw())
         return ACTION_DRAW_FORCED;
       return ACTION_NONE;
   }
   NOTREACHED();
   return ACTION_NONE;
 }
 
 void SchedulerStateMachine::UpdateState(Action action) {
   switch (action) {
     case ACTION_NONE:
       return;
 
     case ACTION_UPDATE_VISIBLE_TILES:
       last_frame_number_where_update_visible_tiles_was_called_ =
           current_frame_number_;
       return;
 
-    case ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED:
-      last_frame_number_where_tree_activation_attempted_ =
-          current_frame_number_;
+    case ACTION_ACTIVATE_PENDING_TREE:
+      UpdateStateOnActivation();
       return;
 
     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_);
       }
       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;
 
     case ACTION_COMMIT: {
       bool commit_was_aborted = false;
-      HandleCommitInternal(commit_was_aborted);
+      UpdateStateOnCommit(commit_was_aborted);
       return;
     }
 
     case ACTION_DRAW_FORCED:
     case ACTION_DRAW_IF_POSSIBLE: {
       bool did_swap = true;
       UpdateStateOnDraw(did_swap);
       return;
     }
 
     case ACTION_DRAW_AND_SWAP_ABORT: {
       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;
       return;
 
     case ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD:
       texture_state_ = LAYER_TEXTURE_STATE_ACQUIRED_BY_MAIN_THREAD;
       main_thread_needs_layer_textures_ = false;
       return;
   }
 }
 
+void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
+  commit_count_++;
+
+  // If we are impl-side-painting but the commit was aborted, then we behave
+  // 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 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 state if we have a new active tree to draw.

[danakj, 2013/08/27 01:24:02]

, or the active tree was unchanged but we're going to do a readback.

[brianderson, 2013/08/27 01:45:37]

Done.

+  if (!has_pending_tree_ &&
+      (!commit_was_aborted || expect_immediate_begin_frame_for_main_thread_)) {
+      needs_redraw_ = true;

[danakj, 2013/08/27 01:24:02]

2 space indent

[brianderson, 2013/08/27 01:45:37]

Done.

+      active_tree_has_been_drawn_ = false;
+  }
+
+  // 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;
+  }
+
+  // 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_)
+    texture_state_ = LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD;
+  else
+    texture_state_ = LAYER_TEXTURE_STATE_UNLOCKED;
+}
+
+void SchedulerStateMachine::UpdateStateOnActivation() {
+  if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
+    output_surface_state_ = OUTPUT_SURFACE_ACTIVE;
+
+  has_pending_tree_ = false;
+  pending_tree_is_ready_for_activation_ = false;
+  active_tree_has_been_drawn_ = false;
+  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_);
     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) {
     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)
     swap_used_incomplete_tile_ = false;
 }
 
 void SchedulerStateMachine::SetMainThreadNeedsLayerTextures() {
   DCHECK(!main_thread_needs_layer_textures_);
   DCHECK_NE(texture_state_, LAYER_TEXTURE_STATE_ACQUIRED_BY_MAIN_THREAD);
   main_thread_needs_layer_textures_ = true;
 }
 
 bool SchedulerStateMachine::BeginFrameNeededToDrawByImplThread() const {
-  // TODO(brianderson): Remove this hack once the Scheduler controls activation,
-  // otherwise we can get stuck in OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION.
-  // It lies that we actually need to draw, but we won't actually draw
-  // if we can't. This will cause WebView to potentially have corruption
-  // in the first few frames, but this workaround is being removed soon.
-  if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
-    return true;
-
   // 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_)
     return true;
 
   if (visible_ && swap_used_incomplete_tile_)
     return true;
 
   return needs_redraw_ && visible_ && HasInitializedOutputSurface();
 }
 
 bool SchedulerStateMachine::ProactiveBeginFrameWantedByImplThread() const {
   // Do not be proactive when invisible.

[danakj, 2013/08/27 01:24:02]

can you update this comment to explain the output surface part?

[brianderson, 2013/08/27 01:45:37]

Done.

-  if (!visible_ || output_surface_state_ != OUTPUT_SURFACE_ACTIVE)
+  if (!visible_ || !HasInitializedOutputSurface())
     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_);
 }
 
 void SchedulerStateMachine::DidEnterBeginFrame(const BeginFrameArgs& args) {
   current_frame_number_++;
@@ skipping 48 matching lines @@
          (expect_immediate_begin_frame_for_main_thread_ &&
           commit_state_ != COMMIT_STATE_IDLE))
       << *AsValue();
   commit_state_ = COMMIT_STATE_READY_TO_COMMIT;
 }
 
 void SchedulerStateMachine::BeginFrameAbortedByMainThread(bool did_handle) {
   DCHECK_EQ(commit_state_, COMMIT_STATE_FRAME_IN_PROGRESS);
   if (did_handle) {
     bool commit_was_aborted = true;
-    HandleCommitInternal(commit_was_aborted);
+    UpdateStateOnCommit(commit_was_aborted);
   } else if (expect_immediate_begin_frame_for_main_thread_) {
     expect_immediate_begin_frame_for_main_thread_ = false;
   } else {
     commit_state_ = COMMIT_STATE_IDLE;
     SetNeedsCommit();
   }
 }
 
 void SchedulerStateMachine::DidLoseOutputSurface() {
   if (output_surface_state_ == OUTPUT_SURFACE_LOST ||
       output_surface_state_ == OUTPUT_SURFACE_CREATING)
     return;
   output_surface_state_ = OUTPUT_SURFACE_LOST;
   needs_redraw_ = false;
 }
 
-void SchedulerStateMachine::SetHasPendingTree(bool has_pending_tree) {
-  if (has_pending_tree_ && !has_pending_tree) {
-    // There is a new active tree.
-    if (output_surface_state_ == OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION)
-      output_surface_state_ = OUTPUT_SURFACE_ACTIVE;
-
-    needs_redraw_ = true;
-  }
-  has_pending_tree_ = has_pending_tree;
+void SchedulerStateMachine::NotifyReadyToActivate() {
+  if (has_pending_tree_)
+    pending_tree_is_ready_for_activation_ = true;
 }
 
 void SchedulerStateMachine::SetCanDraw(bool can) { can_draw_ = can; }
 
 void SchedulerStateMachine::DidCreateAndInitializeOutputSurface() {
   DCHECK_EQ(output_surface_state_, OUTPUT_SURFACE_CREATING);
   output_surface_state_ = OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT;
 
   if (did_create_and_initialize_first_output_surface_) {
     // TODO(boliu): See if we can remove this when impl-side painting is always
@@ skipping 17 matching lines @@
   NOTREACHED();
   return false;
 }
 
 void SchedulerStateMachine::SetMaximumNumberOfFailedDrawsBeforeDrawIsForced(
     int num_draws) {
   maximum_number_of_failed_draws_before_draw_is_forced_ = num_draws;
 }
 
 }  // namespace cc