Revert 213338 "cc: Allow the main thread to cancel commits"

Broke WebViewInteractiveTest.EditCommandsNoMenu on Mac

[1288:27139:0723/221703:FATAL:layer_impl.cc(286)] Check failed: TotalScrollOffset().y() <= max_scroll_offset_.y() (62 vs. 47)

> cc: Allow the main thread to cancel commits
> 
> Add a new SetNeedsUpdateLayers that triggers the commit flow, but is
> abortable if update layers doesn't actually make any changes.  This
> allows the main thread to abort a begin frame.  This happens in the case
> of scroll updates from the compositor thread or invalidations.
> 
> There was previously an abort begin frame call for when a visibility
> message and a begin frame message were posted simultaneously, but it
> incorrectly applied the scrolls and scales without informing the
> compositor thread that these had already been consumed.  To fix this,
> the abort message passes back a boolean about whether or not the
> commit was aborted (and needed to be sent again) or was handled
> (and the scrolls and scales processed).
> 
> To avoid a deluge of begin frames (in the commit sense) from the
> scheduler, the scheduler has been adjusted to wait until the next begin
> frame (in the vsync signal sense) so that these calls can be throttled.
> Otherwise, the scheduler will just keep trying to begin frame.
> 
> R=brianderson@chromium.org, danakj@chromium.org
> BUG=256381
> 
> Review URL: https://chromiumcodereview.appspot.com/19106007

TBR=enne@chromium.org

Review URL: https://codereview.chromium.org/19519019

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

6 files changed, 29 insertions, 157 deletions

diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 010cc99..5e397bd 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -85,9 +85,9 @@ void Scheduler::FinishCommit() {
   ProcessScheduledActions();
 }
 
-void Scheduler::BeginFrameAbortedByMainThread(bool did_handle) {
+void Scheduler::BeginFrameAbortedByMainThread() {
   TRACE_EVENT0("cc", "Scheduler::BeginFrameAbortedByMainThread");
-  state_machine_.BeginFrameAbortedByMainThread(did_handle);
+  state_machine_.BeginFrameAbortedByMainThread();
   ProcessScheduledActions();
 }
 
diff --git a/cc/scheduler/scheduler.h b/cc/scheduler/scheduler.h
index 230f20d..48a3bb2 100644
--- a/cc/scheduler/scheduler.h
+++ b/cc/scheduler/scheduler.h
@@ -86,7 +86,7 @@ class CC_EXPORT Scheduler {
   void DidSwapUseIncompleteTile();
 
   void FinishCommit();
-  void BeginFrameAbortedByMainThread(bool did_handle);
+  void BeginFrameAbortedByMainThread();
 
   void DidLoseOutputSurface();
   void DidCreateAndInitializeOutputSurface();
diff --git a/cc/scheduler/scheduler_state_machine.cc b/cc/scheduler/scheduler_state_machine.cc
index 94100b8..eed9f15 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -15,7 +15,6 @@ 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_update_visible_tiles_was_called_(-1),
@@ -25,7 +24,6 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       swap_used_incomplete_tile_(false),
       needs_forced_redraw_(false),
       needs_forced_redraw_after_next_commit_(false),
-      needs_redraw_after_next_commit_(false),
       needs_commit_(false),
       needs_forced_commit_(false),
       expect_immediate_begin_frame_for_main_thread_(false),
@@ -118,19 +116,6 @@ bool SchedulerStateMachine::HasUpdatedVisibleTilesThisFrame() const {
          last_frame_number_where_update_visible_tiles_was_called_;
 }
 
-void SchedulerStateMachine::SetPostCommitFlags() {
-  // This post-commit work is common to both completed and aborted commits.
-  if (needs_forced_redraw_after_next_commit_) {
-    needs_forced_redraw_after_next_commit_ = false;
-    needs_forced_redraw_ = true;
-  }
-  if (needs_redraw_after_next_commit_) {
-    needs_redraw_after_next_commit_ = false;
-    needs_redraw_ = true;
-  }
-  texture_state_ = LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD;
-}
-
 bool SchedulerStateMachine::DrawSuspendedUntilCommit() const {
   if (!can_draw_)
     return true;
@@ -199,7 +184,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;
@@ -220,18 +205,14 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
         return needs_forced_redraw_ ? ACTION_DRAW_FORCED
                                     : ACTION_DRAW_IF_POSSIBLE;
       }
-      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_))
+      if (needs_commit_ &&
+          ((visible_ && 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 (ShouldUpdateVisibleTiles())
         return ACTION_UPDATE_VISIBLE_TILES;
@@ -258,11 +239,7 @@ 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 =
-          needs_forced_commit_ ||
-          (visible_ &&
-           current_frame_number_ >
-               last_frame_number_where_begin_frame_sent_to_main_thread_);
+      bool can_commit = visible_ || needs_forced_commit_;
       if (needs_commit_ && can_commit && DrawSuspendedUntilCommit())
         return has_pending_tree_ ? ACTION_NONE
                                  : ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD;
@@ -299,16 +276,10 @@ void SchedulerStateMachine::UpdateState(Action action) {
 
     case ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD:
       DCHECK(!has_pending_tree_);
-      if (!needs_forced_commit_) {
-        DCHECK(visible_);
-        DCHECK_GT(current_frame_number_,
-                  last_frame_number_where_begin_frame_sent_to_main_thread_);
-      }
+      DCHECK(visible_ || needs_forced_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;
 
     case ACTION_COMMIT:
@@ -322,7 +293,13 @@ void SchedulerStateMachine::UpdateState(Action action) {
         needs_redraw_ = true;
       if (draw_if_possible_failed_)
         last_frame_number_where_draw_was_called_ = -1;
-      SetPostCommitFlags();
+
+      if (needs_forced_redraw_after_next_commit_) {
+        needs_forced_redraw_after_next_commit_ = false;
+        needs_forced_redraw_ = true;
+      }
+
+      texture_state_ = LAYER_TEXTURE_STATE_ACQUIRED_BY_IMPL_THREAD;
       return;
 
     case ACTION_DRAW_FORCED:
@@ -390,12 +367,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;
 }
 
@@ -445,13 +422,10 @@ void SchedulerStateMachine::FinishCommit() {
   commit_state_ = COMMIT_STATE_READY_TO_COMMIT;
 }
 
-void SchedulerStateMachine::BeginFrameAbortedByMainThread(bool did_handle) {
+void SchedulerStateMachine::BeginFrameAbortedByMainThread() {
   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;
-    SetPostCommitFlags();
   } else {
     commit_state_ = COMMIT_STATE_IDLE;
     SetNeedsCommit();
@@ -484,7 +458,6 @@ void SchedulerStateMachine::DidCreateAndInitializeOutputSurface() {
     // sort themselves out with a commit.
     needs_redraw_ = false;
   }
-  needs_redraw_after_next_commit_ = true;
   did_create_and_initialize_first_output_surface_ = true;
 }
 
diff --git a/cc/scheduler/scheduler_state_machine.h b/cc/scheduler/scheduler_state_machine.h
index 5c7d5b5..5a29830d 100644
--- a/cc/scheduler/scheduler_state_machine.h
+++ b/cc/scheduler/scheduler_state_machine.h
@@ -79,7 +79,7 @@ class CC_EXPORT SchedulerStateMachine {
 
   // Indicates that the system has entered and left a BeginFrame callback.
   // The scheduler will not draw more than once in a given BeginFrame
-  // callback nor send more than one BeginFrame message.
+  // callback.
   void DidEnterBeginFrame(const BeginFrameArgs& args);
   void DidLeaveBeginFrame();
   bool inside_begin_frame() const { return inside_begin_frame_; }
@@ -120,9 +120,8 @@ class CC_EXPORT SchedulerStateMachine {
 
   // Call this only in response to receiving an
   // ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD from NextAction if the client
-  // rejects the begin frame message.  If did_handle is false, then
-  // another commit will be retried soon.
-  void BeginFrameAbortedByMainThread(bool did_handle);
+  // rejects the begin frame message.
+  void BeginFrameAbortedByMainThread();
 
   // Request exclusive access to the textures that back single buffered
   // layers on behalf of the main thread. Upon acquisition,
@@ -169,7 +168,6 @@ class CC_EXPORT SchedulerStateMachine {
   bool HasDrawnThisFrame() const;
   bool HasAttemptedTreeActivationThisFrame() const;
   bool HasUpdatedVisibleTilesThisFrame() const;
-  void SetPostCommitFlags();
 
   const SchedulerSettings settings_;
 
@@ -177,7 +175,6 @@ class CC_EXPORT SchedulerStateMachine {
   int commit_count_;
 
   int current_frame_number_;
-  int last_frame_number_where_begin_frame_sent_to_main_thread_;
   int last_frame_number_where_draw_was_called_;
   int last_frame_number_where_tree_activation_attempted_;
   int last_frame_number_where_update_visible_tiles_was_called_;
@@ -187,7 +184,6 @@ class CC_EXPORT SchedulerStateMachine {
   bool swap_used_incomplete_tile_;
   bool needs_forced_redraw_;
   bool needs_forced_redraw_after_next_commit_;
-  bool needs_redraw_after_next_commit_;
   bool needs_commit_;
   bool needs_forced_commit_;
   bool expect_immediate_begin_frame_for_main_thread_;
diff --git a/cc/scheduler/scheduler_state_machine_unittest.cc b/cc/scheduler/scheduler_state_machine_unittest.cc
index c7210eb..7036459 100644
--- a/cc/scheduler/scheduler_state_machine_unittest.cc
+++ b/cc/scheduler/scheduler_state_machine_unittest.cc
@@ -718,7 +718,7 @@ TEST(SchedulerStateMachineTest, TestGoesInvisibleBeforeFinishCommit) {
 
   // Become invisible and abort the main thread's begin frame.
   state.SetVisible(false);
-  state.BeginFrameAbortedByMainThread(false);
+  state.BeginFrameAbortedByMainThread();
 
   // We should now be back in the idle state as if we didn't start a frame at
   // all.
@@ -728,14 +728,8 @@ TEST(SchedulerStateMachineTest, TestGoesInvisibleBeforeFinishCommit) {
   // Become visible again.
   state.SetVisible(true);
 
-  // Although we have aborted on this frame and haven't cancelled the commit
-  // (i.e. need another), don't send another begin frame yet.
+  // We should be beginning a frame now.
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_IDLE, state.CommitState());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  EXPECT_TRUE(state.NeedsCommit());
-
-  // Start a new frame.
-  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
             state.NextAction());
 
@@ -747,52 +741,6 @@ TEST(SchedulerStateMachineTest, TestGoesInvisibleBeforeFinishCommit) {
             state.CommitState());
 }
 
-TEST(SchedulerStateMachineTest, AbortBeginFrameAndCancelCommit) {
-  SchedulerSettings default_scheduler_settings;
-  StateMachine state(default_scheduler_settings);
-  state.SetCanStart();
-  state.UpdateState(state.NextAction());
-  state.DidCreateAndInitializeOutputSurface();
-  state.SetVisible(true);
-  state.SetCanDraw(true);
-
-  // Get into a begin frame / commit state.
-  state.SetNeedsCommit();
-  EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
-            state.NextAction());
-  state.UpdateState(
-      SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD);
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
-            state.CommitState());
-  EXPECT_FALSE(state.NeedsCommit());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-
-  // Abort the commit, cancelling future commits.
-  state.BeginFrameAbortedByMainThread(true);
-
-  // Verify that another commit doesn't start on the same frame.
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_IDLE, state.CommitState());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  EXPECT_FALSE(state.NeedsCommit());
-
-  // Start a new frame; draw because this is the first frame since output
-  // surface init'd.
-  state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
-  state.DidLeaveBeginFrame();
-
-  // Verify another commit doesn't start on another frame either.
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_IDLE, state.CommitState());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  EXPECT_FALSE(state.NeedsCommit());
-
-  // Verify another commit can start if requested, though.
-  state.SetNeedsCommit();
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_IDLE, state.CommitState());
-  EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
-            state.NextAction());
-}
-
 TEST(SchedulerStateMachineTest, TestFirstContextCreation) {
   SchedulerSettings default_scheduler_settings;
   StateMachine state(default_scheduler_settings);
@@ -868,23 +816,15 @@ TEST(SchedulerStateMachineTest,
 
   // Recreate the context
   state.DidCreateAndInitializeOutputSurface();
-  EXPECT_FALSE(state.RedrawPending());
 
   // When the context is recreated, we should begin a commit
   EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
             state.NextAction());
   state.UpdateState(state.NextAction());
-  EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_FRAME_IN_PROGRESS,
-            state.CommitState());
-  state.FinishCommit();
-  EXPECT_EQ(SchedulerStateMachine::ACTION_COMMIT, state.NextAction());
-  state.UpdateState(state.NextAction());
-  // Finishing the first commit after initializing an output surface should
-  // automatically cause a redraw.
-  EXPECT_TRUE(state.RedrawPending());
 
   // Once the context is recreated, whether we draw should be based on
   // SetCanDraw.
+  state.SetNeedsRedraw(true);
   state.DidEnterBeginFrame(BeginFrameArgs::CreateForTesting());
   EXPECT_EQ(SchedulerStateMachine::ACTION_DRAW_IF_POSSIBLE, state.NextAction());
   state.SetCanDraw(false);
@@ -1252,7 +1192,7 @@ TEST(SchedulerStateMachineTest,
 
   // Become invisible and abort the main thread's begin frame.
   state.SetVisible(false);
-  state.BeginFrameAbortedByMainThread(false);
+  state.BeginFrameAbortedByMainThread();
 
   // Should be back in the idle state, but needing a commit.
   EXPECT_EQ(SchedulerStateMachine::COMMIT_STATE_IDLE, state.CommitState());
@@ -1355,37 +1295,5 @@ TEST(SchedulerStateMachineTest, ReportIfNotDrawingFromAcquiredTextures) {
   EXPECT_FALSE(state.DrawSuspendedUntilCommit());
 }
 
-TEST(SchedulerStateMachineTest, AcquireTexturesWithAbort) {
-  SchedulerSettings default_scheduler_settings;
-  SchedulerStateMachine state(default_scheduler_settings);
-  state.SetCanStart();
-  state.UpdateState(state.NextAction());
-  state.DidCreateAndInitializeOutputSurface();
-  state.SetCanDraw(true);
-  state.SetVisible(true);
-
-  state.SetMainThreadNeedsLayerTextures();
-  EXPECT_EQ(
-      SchedulerStateMachine::ACTION_ACQUIRE_LAYER_TEXTURES_FOR_MAIN_THREAD,
-      state.NextAction());
-  state.UpdateState(state.NextAction());
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
-
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-
-  state.SetNeedsCommit();
-  EXPECT_EQ(SchedulerStateMachine::ACTION_SEND_BEGIN_FRAME_TO_MAIN_THREAD,
-            state.NextAction());
-  state.UpdateState(state.NextAction());
-  EXPECT_TRUE(state.DrawSuspendedUntilCommit());
-
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-
-  state.BeginFrameAbortedByMainThread(true);
-
-  EXPECT_EQ(SchedulerStateMachine::ACTION_NONE, state.NextAction());
-  EXPECT_FALSE(state.DrawSuspendedUntilCommit());
-}
-
 }  // namespace
 }  // namespace cc
diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc
index 930111c..0a3c250 100644
--- a/cc/scheduler/scheduler_unittest.cc
+++ b/cc/scheduler/scheduler_unittest.cc
@@ -378,17 +378,12 @@ TEST(SchedulerTest, VisibilitySwitchWithTextureAcquisition) {
                        client);
   client.Reset();
 
-  // Already sent a begin frame on this current frame, so wait.
-  scheduler->SetVisible(true);
-  EXPECT_EQ(0, client.num_actions_());
-  client.Reset();
-
   // Regaining visibility with textures acquired by main thread while
   // compositor is waiting for first draw should result in a request
   // for a new frame in order to escape a deadlock.
-  scheduler->BeginFrame(BeginFrameArgs::CreateForTesting());
-  EXPECT_ACTION("ScheduledActionSendBeginFrameToMainThread", client, 0, 2);
-  EXPECT_ACTION("SetNeedsBeginFrameOnImplThread", client, 1, 2);
+  scheduler->SetVisible(true);
+  EXPECT_SINGLE_ACTION("ScheduledActionSendBeginFrameToMainThread", client);
+  client.Reset();
 }
 
 class SchedulerClientThatsetNeedsDrawInsideDraw : public FakeSchedulerClient {