cc: Allow the main thread to cancel commits

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 cb14cadf52acd3beeb3a7f91f6b7c89c8f4308f1..13a1d35ee78bac4155ddf5dafd294aa7743fa562 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -15,6 +15,7 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       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_check_for_completed_tile_uploads_called_(-1),
@@ -184,7 +185,7 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
     return ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD;
 
   switch (commit_state_) {
-    case COMMIT_STATE_IDLE:
+    case COMMIT_STATE_IDLE: {
       if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE &&
           needs_forced_redraw_)
         return ACTION_DRAW_FORCED;
@@ -205,14 +206,18 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
         return needs_forced_redraw_ ? ACTION_DRAW_FORCED
                                     : ACTION_DRAW_IF_POSSIBLE;
       }
-      if (needs_commit_ &&
-          ((visible_ && output_surface_state_ == OUTPUT_SURFACE_ACTIVE)
-           || needs_forced_commit_))
+      bool can_commit_this_frame =
+          visible_ &&
+          current_frame_number_ >
+              last_frame_number_where_begin_frame_sent_to_main_thread_;
+      if (needs_commit_ && ((can_commit_this_frame &&
+                             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;
       return ACTION_NONE;
-
+    }
     case COMMIT_STATE_FRAME_IN_PROGRESS:
       if (ShouldCheckForCompletedTileUploads())
         return ACTION_CHECK_FOR_COMPLETED_TILE_UPLOADS;
@@ -239,7 +244,11 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
       // COMMIT_STATE_WAITING_FOR_FIRST_DRAW wants to enforce a draw. If
       // can_draw_ is false or textures are not available, proceed to the next
       // step (similar as in COMMIT_STATE_IDLE).
-      bool can_commit = visible_ || needs_forced_commit_;
+      bool can_commit =
+          needs_forced_commit_ ||
+          (visible_ &&
+           current_frame_number_ >
+               last_frame_number_where_begin_frame_sent_to_main_thread_);
       if (needs_commit_ && can_commit && DrawSuspendedUntilCommit())
         return has_pending_tree_ ? ACTION_NONE
                                  : ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD;
@@ -276,10 +285,16 @@ void SchedulerStateMachine::UpdateState(Action action) {
 
     case ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD:
       DCHECK(!has_pending_tree_);
-      DCHECK(visible_ || needs_forced_commit_);
+      if (!needs_forced_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:
@@ -367,12 +382,12 @@ bool SchedulerStateMachine::ProactiveBeginFrameWantedByImplThread() const {
 }
 
 void SchedulerStateMachine::DidEnterBeginFrame(const BeginFrameArgs& args) {
+  current_frame_number_++;
   inside_begin_frame_ = true;
   last_begin_frame_args_ = args;
 }
 
 void SchedulerStateMachine::DidLeaveBeginFrame() {
-  current_frame_number_++;
   inside_begin_frame_ = false;
 }
 
@@ -422,10 +437,16 @@ void SchedulerStateMachine::FinishCommit() {
   commit_state_ = COMMIT_STATE_READY_TO_COMMIT;
 }
 
-void SchedulerStateMachine::BeginFrameAbortedByMainThread() {
+void SchedulerStateMachine::BeginFrameAbortedByMainThread(bool did_handle) {
   DCHECK_EQ(commit_state_, COMMIT_STATE_FRAME_IN_PROGRESS);
   if (expect_immediate_begin_frame_for_main_thread_) {
     expect_immediate_begin_frame_for_main_thread_ = false;
+  } else if (did_handle) {
+    commit_state_ = COMMIT_STATE_IDLE;
+    if (needs_forced_redraw_after_next_commit_) {
+      needs_forced_redraw_after_next_commit_ = false;

[danakj, 2013/07/18 21:57:23]

What about the other states changed in ACTION_COMMIT? The texture acquired
state, or the last_frame_where_draw_failed?  I feel like these should share a
code path for this.

[enne (OOO), 2013/07/19 01:23:51]

Unfortunately, there's not that much sharing.

Aborting sets commit state idle (rather than commit state waiting for first
[forced?] draw).  I think this is correct, as an aborted commit doesn't add any
new information to the compositor thread so getting a "did commit and draw"
notification in this case is incorrect. 

There's no SetNeedRedraw for !impl-side painting, which I think is also correct
as there hasn't been any damage.

Since nothing has changed, there's no need to reset the frame number for
last_frame_number_where_draw_was_called_.

Maybe I don't understand texture_state_ enough, but because nothing has changed
on the compositor thread, draw/abort commit/draw seems no different than
draw/draw/draw where the texture_state_ continues to be TEXTURE_STATE_UNLOCKED_
in both cases.

That leaves needs_forced_redraw_after_next_commit and my new
needs_redraw_after_next_commit.  I'll put them both in a helper function, and
you can see what you think.

[danakj, 2013/07/22 17:16:20]

I'd say they are different, because drawing is blocked until a commit happens
and an aborted commit should still count and unblock the impl thread.

If the texture_state_ is LAYER_TEXTURE_STATE_ACQUIRED_BY_MAIN_THREAD, then the
aborted commit does not change the texture_state_ and we will stay in
DrawSuspendedUntilCommit() state.

I think this would end up DCHECKing in ShouldAcquireLayerTexturesForMainThread()
actually.

Can you add a unit test that does acquire layer textures, aborts a commit, and
makes sure the impl thread continues to draw?

[enne (OOO), 2013/07/22 19:02:43]

Done.

+      needs_forced_redraw_ = true;
+    }
   } else {
     commit_state_ = COMMIT_STATE_IDLE;
     SetNeedsCommit();