cc: Add an ACTION_DRAW_AND_SWAP_ABORT to help clear the pipeline

Adding this action will allow us to flush the pending/active tree
pipeline in corner cases without crazy branching in the Scheduler.

It also allows us to:
1) Maintain the invariant that we only send BeginFrame to the
     main thread when we are in COMMIT_STATE_IDLE, which helps
     simplify the Scheduler a bit more.
2) Simplify ShouldAcquireLayerTexturesForMainThread and fix
     a potential bug where main thread locks might race
     impl-side draws when impl-side painting.

BUG=276579
BUG=277093

Review URL: https://chromiumcodereview.appspot.com/22884014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218930 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 100 insertions, 74 deletions

diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 0cf594f..3e8833f 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -229,6 +229,10 @@ void Scheduler::ProcessScheduledActions() {
       case SchedulerStateMachine::ACTION_DRAW_FORCED:
         DrawAndSwapForced();
         break;
+      case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT:
+        // No action is actually performed, but this allows the state machine to
+        // advance out of its waiting to draw state without actually drawing.
+        break;
       case SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
         client_->ScheduledActionBeginOutputSurfaceCreation();
         break;
@@ -243,7 +247,7 @@ void Scheduler::ProcessScheduledActions() {
 }
 
 bool Scheduler::WillDrawIfNeeded() const {
-  return !state_machine_.DrawSuspendedUntilCommit();
+  return !state_machine_.PendingDrawsShouldBeAborted();
 }
 
 }  // namespace cc
diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
index f7f8d2da..0014796 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -101,6 +101,8 @@ const char* SchedulerStateMachine::ActionToString(Action action) {
       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:
@@ -221,37 +223,43 @@ void SchedulerStateMachine::SetPostCommitFlags() {
   texture_state_ = LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD;
 }
 
-bool SchedulerStateMachine::DrawSuspendedUntilCommit() const {
+bool SchedulerStateMachine::PendingDrawsShouldBeAborted() const {
+  // These are all the cases where, if we do not abort draws to make
+  // forward progress, we might deadlock with the main thread.
   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)
     return true;
   return false;
 }
 
-bool SchedulerStateMachine::ScheduledToDraw() const {
-  if (!needs_redraw_)
-    return false;
-  if (DrawSuspendedUntilCommit())
-    return false;
-  return true;
-}
-
 bool SchedulerStateMachine::ShouldDraw() const {
+  // Always handle forced draws ASAP.
   if (needs_forced_redraw_)
     return true;
 
-  if (!ScheduledToDraw())
-    return false;
-  if (!inside_begin_frame_)
-    return false;
+  // If we are going to abort draws, we should do so ASAP.
+  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_;
+  }
+
+  // After this line, we only want to draw once per frame.
   if (HasDrawnThisFrame())
     return false;
-  if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE)
+
+  // We currently only draw within the BeginFrame.
+  if (!inside_begin_frame_)
     return false;
-  return true;
+
+  return needs_redraw_;
 }
 
 bool SchedulerStateMachine::ShouldAttemptTreeActivation() const {
@@ -275,12 +283,6 @@ bool SchedulerStateMachine::ShouldAcquireLayerTexturesForMainThread() const {
   if (texture_state_ == LAYER_TEXTURE_STATE_UNLOCKED)
     return true;
   DCHECK_EQ(texture_state_, LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD);
-  // Transfer the lock from impl thread to main thread immediately if the
-  // impl thread is not even scheduled to draw. Guards against deadlocking.
-  if (!ScheduledToDraw())
-    return true;
-  if (!BeginFrameNeededToDrawByImplThread())
-    return true;
   return false;
 }
 
@@ -341,21 +343,14 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
         return ACTION_UPDATE_VISIBLE_TILES;
       if (ShouldAttemptTreeActivation())
         return ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED;
-      if (ShouldDraw() || output_surface_state_ == OUTPUT_SURFACE_LOST) {
-        return needs_forced_redraw_ ? ACTION_DRAW_FORCED
-                                    : ACTION_DRAW_IF_POSSIBLE;
+      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;
       }
-      // 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 =
-          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;
       return ACTION_NONE;
     }
 
@@ -416,23 +411,17 @@ void SchedulerStateMachine::UpdateState(Action action) {
       return;
 
     case ACTION_DRAW_FORCED:
-    case ACTION_DRAW_IF_POSSIBLE:
-      needs_redraw_ = false;
-      needs_forced_redraw_ = false;
-      draw_if_possible_failed_ = false;
-      swap_used_incomplete_tile_ = false;
-      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;
+    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);
@@ -447,6 +436,27 @@ void SchedulerStateMachine::UpdateState(Action action) {
   }
 }
 
+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;
+
+  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);
diff --git a/cc/scheduler/scheduler_state_machine.h b/cc/scheduler/scheduler_state_machine.h
index 95d5fa1..5e0cf5b 100644
--- a/cc/scheduler/scheduler_state_machine.h
+++ b/cc/scheduler/scheduler_state_machine.h
@@ -74,6 +74,7 @@ class CC_EXPORT SchedulerStateMachine {
     ACTION_ACTIVATE_PENDING_TREE_IF_NEEDED,
     ACTION_DRAW_IF_POSSIBLE,
     ACTION_DRAW_FORCED,
+    ACTION_DRAW_AND_SWAP_ABORT,
     ACTION_BEGIN_OUTPUT_SURFACE_CREATION,
     ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD,
   };
@@ -165,13 +166,11 @@ class CC_EXPORT SchedulerStateMachine {
   // Exposed for testing purposes.
   void SetMaximumNumberOfFailedDrawsBeforeDrawIsForced(int num_draws);
 
-  // False if drawing is not being prevented, true if drawing won't happen
-  // for some reason, such as not being visible.
-  bool DrawSuspendedUntilCommit() const;
+  // True if we need to abort draws to make forward progress.
+  bool PendingDrawsShouldBeAborted() const;
 
  protected:
   bool ShouldDrawForced() const;
-  bool ScheduledToDraw() const;
   bool ShouldDraw() const;
   bool ShouldAttemptTreeActivation() const;
   bool ShouldAcquireLayerTexturesForMainThread() const;
@@ -180,6 +179,7 @@ class CC_EXPORT SchedulerStateMachine {
   bool HasAttemptedTreeActivationThisFrame() const;
   bool HasUpdatedVisibleTilesThisFrame() const;
   void SetPostCommitFlags();
+  void UpdateStateOnDraw(bool did_swap);
 
   const SchedulerSettings settings_;
 
diff --git a/cc/scheduler/scheduler_state_machine_unittest.cc b/cc/scheduler/scheduler_state_machine_unittest.cc
index fc7fe83..c9454ce 100644
--- a/cc/scheduler/scheduler_state_machine_unittest.cc
+++ b/cc/scheduler/scheduler_state_machine_unittest.cc
@@ -449,12 +449,12 @@ TEST(SchedulerStateMachineTest, TestNextActionDrawsOnBeginFrame) {
 
       // Case 1: needs_commit=false.
       EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
-      EXPECT_EQ(expected_action, state.NextAction());
+      EXPECT_EQ(expected_action, state.NextAction()) << *state.AsValue();
 
       // Case 2: needs_commit=true.
       state.SetNeedsCommit();
       EXPECT_TRUE(state.BeginFrameNeededToDrawByImplThread());
-      EXPECT_EQ(expected_action, state.NextAction());
+      EXPECT_EQ(expected_action, state.NextAction()) << *state.AsValue();
     }
   }
 }
@@ -529,6 +529,9 @@ TEST(SchedulerStateMachineTest,
   state.SetNeedsRedraw(true);
   state.SetVisible(true);
   state.SetCanDraw(false);
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT,
+            state.NextAction());
+  state.UpdateState(state.NextAction());
   EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
             state.NextAction());
 }
@@ -888,7 +891,8 @@ TEST(SchedulerStateMachineTest,
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
   state.SetCanDraw(false);
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT,
+            state.NextAction());
   state.SetCanDraw(true);
   state.DidLeaveBeginFrame();
 }
@@ -931,7 +935,8 @@ TEST(SchedulerStateMachineTest, TestContextLostWhileCommitInProgress) {
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
             state.CommitState());
 
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT,
+            state.NextAction());
   state.UpdateState(state.NextAction());
 
   // Expect to be told to begin context recreation, independent of
@@ -984,7 +989,8 @@ TEST(SchedulerStateMachineTest,
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
             state.CommitState());
 
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT,
+            state.NextAction());
   state.UpdateState(state.NextAction());
 
   // Expect to be told to begin context recreation, independent of
@@ -1096,6 +1102,9 @@ TEST(SchedulerStateMachineTest, TestFinishCommitWhenCommitInProgress) {
 
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_WAITING_FOR_FIRST_DRAW,
             state.CommitState());
+  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT,
+            state.NextAction());
+  state.UpdateState(state.NextAction());
 
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
 }
@@ -1298,26 +1307,29 @@ TEST(SchedulerStateMachineTest, ImmediateFinishCommitWhileCantDraw) {
 TEST(SchedulerStateMachineTest, ReportIfNotDrawing) {
   SchedulerSettings default_scheduler_settings;
   SchedulerStateMachine state(default_scheduler_settings);
+  state.SetCanStart();
+  state.UpdateState(state.NextAction());
+  state.DidCreateAndInitializeOutputSurface();
 
   state.SetCanDraw(true);
   state.SetVisible(true);
-  EXPECT_FALSE(state.DrawSuspendedUntilCommit());
+  EXPECT_FALSE(state.PendingDrawsShouldBeAborted());
 
   state.SetCanDraw(false);
   state.SetVisible(true);
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
+  EXPECT_TRUE(state.PendingDrawsShouldBeAborted());
 
   state.SetCanDraw(true);
   state.SetVisible(false);
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
+  EXPECT_TRUE(state.PendingDrawsShouldBeAborted());
 
   state.SetCanDraw(false);
   state.SetVisible(false);
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
+  EXPECT_TRUE(state.PendingDrawsShouldBeAborted());
 
   state.SetCanDraw(true);
   state.SetVisible(true);
-  EXPECT_FALSE(state.DrawSuspendedUntilCommit());
+  EXPECT_FALSE(state.PendingDrawsShouldBeAborted());
 }
 
 TEST(SchedulerStateMachineTest, ReportIfNotDrawingFromAcquiredTextures) {
@@ -1328,14 +1340,14 @@ TEST(SchedulerStateMachineTest, ReportIfNotDrawingFromAcquiredTextures) {
   state.DidCreateAndInitializeOutputSurface();
   state.SetCanDraw(true);
   state.SetVisible(true);
-  EXPECT_FALSE(state.DrawSuspendedUntilCommit());
+  EXPECT_FALSE(state.PendingDrawsShouldBeAborted());
 
   state.SetMainThreadNeedsLayerTextures();
   EXPECT_EQ(
       SchedulerStateMachine::ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD,
       state.NextAction());
   state.UpdateState(state.NextAction());
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
+  EXPECT_TRUE(state.PendingDrawsShouldBeAborted());
 
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
 
@@ -1344,17 +1356,17 @@ TEST(SchedulerStateMachineTest, ReportIfNotDrawingFromAcquiredTextures) {
             state.NextAction());
 
   state.UpdateState(state.NextAction());
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
+  EXPECT_TRUE(state.PendingDrawsShouldBeAborted());
 
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
 
   state.FinishCommit();
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
+  EXPECT_TRUE(state.PendingDrawsShouldBeAborted());
 
   EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
 
   state.UpdateState(state.NextAction());
-  EXPECT_FALSE(state.DrawSuspendedUntilCommit());
+  EXPECT_FALSE(state.PendingDrawsShouldBeAborted());
 }
 
 TEST(SchedulerStateMachineTest, AcquireTexturesWithAbort) {
@@ -1371,7 +1383,7 @@ TEST(SchedulerStateMachineTest, AcquireTexturesWithAbort) {
       SchedulerStateMachine::ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD,
       state.NextAction());
   state.UpdateState(state.NextAction());
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
+  EXPECT_TRUE(state.PendingDrawsShouldBeAborted());
 
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
 
@@ -1379,14 +1391,14 @@ TEST(SchedulerStateMachineTest, AcquireTexturesWithAbort) {
   EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
             state.NextAction());
   state.UpdateState(state.NextAction());
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
+  EXPECT_TRUE(state.PendingDrawsShouldBeAborted());
 
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
 
   state.BeginFrameAbortedByMainThread(true);
 
   EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  EXPECT_FALSE(state.DrawSuspendedUntilCommit());
+  EXPECT_FALSE(state.PendingDrawsShouldBeAborted());
 }
 
 }  // namespace