Revert of Making scheduler run ANIMATE after a COMMIT (instead of...

Revert of Making scheduler run ANIMATE after a COMMIT (instead of LayerTreeHostImpl). (patchset #4 id:50001 of https://codereview.chromium.org/621823003/)

Reason for revert:
Some sites become "unresponsive" i.e. they work but have no ui changes. and the user need to switch between tabs in order of getting the actual page.

Original issue's description:
> Making scheduler run ANIMATE after a COMMIT (instead of LayerTreeHostImpl).
>
> This moves the logic from the LayerTreeHostImpl into the scheduler which means
> only ScheduledActionAnimate now updates animation state.
>
> The further extends the work in http://crrev.com/206793003
>
> BUG=346230
>
> Committed: https://crrev.com/7fa5729cf6ac490cc3b257b7eb8093dcd1285e3a
> Cr-Commit-Position: refs/heads/master@{#298396}

TBR=ajuma@chromium.org,skyostil@google.com,brianderson@chromium.org,danakj@chromium.org,skyostil@chromium.org,mithro@mithis.com
NOTREECHECKS=true
NOTRY=true
BUG=346230

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

Cr-Commit-Position: refs/heads/master@{#299441}

cc/scheduler/scheduler_state_machine.cc

@@ -43,7 +43,6 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       has_pending_tree_(false),
       pending_tree_is_ready_for_activation_(false),
       active_tree_needs_first_draw_(false),
-      did_commit_after_animating_(false),
       did_create_and_initialize_first_output_surface_(false),
       impl_latency_takes_priority_(false),
       skip_next_begin_main_frame_to_reduce_latency_(false),
@@ -226,7 +225,6 @@ void SchedulerStateMachine::AsValueInto(base::debug::TracedValue* state,
                     pending_tree_is_ready_for_activation_);
   state->SetBoolean("active_tree_needs_first_draw",
                     active_tree_needs_first_draw_);
-  state->SetBoolean("did_commit_after_animating", did_commit_after_animating_);
   state->SetBoolean("did_create_and_initialize_first_output_surface",
                     did_create_and_initialize_first_output_surface_);
   state->SetBoolean("impl_latency_takes_priority",
@@ -253,10 +251,6 @@ void SchedulerStateMachine::AdvanceCurrentFrameNumber() {
   skip_next_begin_main_frame_to_reduce_latency_ = false;
 }
 
-bool SchedulerStateMachine::HasAnimatedThisFrame() const {
-  return last_frame_number_animate_performed_ == current_frame_number_;
-}
-
 bool SchedulerStateMachine::HasSentBeginMainFrameThisFrame() const {
   return current_frame_number_ ==
          last_frame_number_begin_main_frame_sent_;
@@ -347,11 +341,6 @@ bool SchedulerStateMachine::ShouldDraw() const {
   if (PendingDrawsShouldBeAborted())
     return active_tree_needs_first_draw_;
 
-  // If a commit has occurred after the animate call, we need to call animate
-  // again before we should draw.
-  if (did_commit_after_animating_)
-    return false;
-
   // After this line, we only want to send a swap request once per frame.
   if (HasRequestedSwapThisFrame())
     return false;
@@ -423,8 +412,7 @@ bool SchedulerStateMachine::ShouldAnimate() const {
   if (!can_draw_)
     return false;
 
-  // If a commit occurred after our last call, we need to do animation again.
-  if (HasAnimatedThisFrame() && !did_commit_after_animating_)
+  if (last_frame_number_animate_performed_ == current_frame_number_)
     return false;
 
   if (begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING &&
@@ -568,7 +556,6 @@ void SchedulerStateMachine::UpdateState(Action action) {
     case ACTION_ANIMATE:
       last_frame_number_animate_performed_ = current_frame_number_;
       needs_animate_ = false;
-      did_commit_after_animating_ = false;
       // TODO(skyostil): Instead of assuming this, require the client to tell
       // us.
       SetNeedsRedraw();
@@ -624,9 +611,6 @@ void SchedulerStateMachine::UpdateState(Action action) {
 void SchedulerStateMachine::UpdateStateOnCommit(bool commit_was_aborted) {
   commit_count_++;
 
-  if (!commit_was_aborted && HasAnimatedThisFrame())
-    did_commit_after_animating_ = true;
-
   if (commit_was_aborted || settings_.main_frame_before_activation_enabled) {
     commit_state_ = COMMIT_STATE_IDLE;
   } else {

cc/scheduler/scheduler_state_machine.h

@@ -260,7 +260,6 @@ class CC_EXPORT SchedulerStateMachine {
   bool ShouldManageTiles() const;
 
   void AdvanceCurrentFrameNumber();
-  bool HasAnimatedThisFrame() const;
   bool HasSentBeginMainFrameThisFrame() const;
   bool HasUpdatedVisibleTilesThisFrame() const;
   bool HasRequestedSwapThisFrame() const;
@@ -308,7 +307,6 @@ class CC_EXPORT SchedulerStateMachine {
   bool has_pending_tree_;
   bool pending_tree_is_ready_for_activation_;
   bool active_tree_needs_first_draw_;
-  bool did_commit_after_animating_;
   bool did_create_and_initialize_first_output_surface_;
   bool impl_latency_takes_priority_;
   bool skip_next_begin_main_frame_to_reduce_latency_;

cc/scheduler/scheduler_state_machine_unittest.cc

@@ -1754,39 +1754,6 @@ TEST(SchedulerStateMachineTest, TestAnimateBeforeCommit) {
       SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
 }
 
-TEST(SchedulerStateMachineTest, TestAnimateAfterCommitBeforeDraw) {
-  SchedulerSettings settings;
-  settings.impl_side_painting = true;
-  StateMachine state(settings);
-  state.SetCanStart();
-  state.UpdateState(state.NextAction());
-  state.CreateAndInitializeOutputSurfaceWithActivatedCommit();
-  state.SetVisible(true);
-  state.SetCanDraw(true);
-
-  // Check that animations are updated before we start a commit.
-  state.SetNeedsAnimate();
-  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
-  state.SetNeedsCommit();
-  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_NONE);
-  EXPECT_TRUE(state.BeginFrameNeeded());
-
-  state.OnBeginImplFrame(CreateBeginFrameArgsForTesting());
-  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_ANIMATE);
-  EXPECT_ACTION_UPDATE_STATE(
-      SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME);
-
-  state.NotifyBeginMainFrameStarted();
-  state.NotifyReadyToCommit();
-  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_COMMIT);
-
-  state.OnBeginImplFrameDeadlinePending();
-  state.OnBeginImplFrameDeadline();
-  EXPECT_ACTION_UPDATE_STATE(SchedulerStateMachine::ACTION_ANIMATE);
-  EXPECT_ACTION_UPDATE_STATE(
-      SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE);
-}
-
 TEST(SchedulerStateMachineTest, TestSetNeedsAnimateAfterAnimate) {
   SchedulerSettings settings;
   settings.impl_side_painting = true;

cc/trees/layer_tree_host_impl.cc

@@ -381,10 +381,6 @@ void LayerTreeHostImpl::BeginCommit() {
 void LayerTreeHostImpl::CommitComplete() {
   TRACE_EVENT0("cc", "LayerTreeHostImpl::CommitComplete");
 
-  // Ask to be animated if there are animations
-  if (needs_animate_layers())
-    SetNeedsAnimate();
-
   if (pending_tree_)
     pending_tree_->ApplyScrollDeltasSinceBeginMainFrame();
   sync_tree()->set_needs_update_draw_properties();

cc/trees/thread_proxy.cc

@@ -108,6 +108,7 @@ ThreadProxy::CompositorThreadOnly::CompositorThreadOnly(
       inside_draw(false),
       input_throttled_until_commit(false),
       animations_frozen_until_next_draw(false),
+      did_commit_after_animating(false),
       smoothness_priority_expiration_notifier(
           proxy->ImplThreadTaskRunner(),
           base::Bind(&ThreadProxy::RenewTreePriority, base::Unretained(proxy)),
@@ -943,6 +944,7 @@ void ThreadProxy::ScheduledActionAnimate() {
         impl().layer_tree_host_impl->CurrentBeginFrameArgs().frame_time;
   }
   impl().layer_tree_host_impl->Animate(impl().animation_time);
+  impl().did_commit_after_animating = false;
 }
 
 void ThreadProxy::ScheduledActionCommit() {
@@ -960,6 +962,7 @@ void ThreadProxy::ScheduledActionCommit() {
     impl().animation_time = std::max(
         impl().animation_time, blocked_main().last_monotonic_frame_begin_time);
   }
+  impl().did_commit_after_animating = true;
 
   blocked_main().main_thread_inside_commit = true;
   impl().layer_tree_host_impl->BeginCommit();
@@ -1028,6 +1031,11 @@ DrawResult ThreadProxy::DrawSwapInternal(bool forced_draw) {
   impl().timing_history.DidStartDrawing();
   base::AutoReset<bool> mark_inside(&impl().inside_draw, true);
 
+  if (impl().did_commit_after_animating) {
+    impl().layer_tree_host_impl->Animate(impl().animation_time);
+    impl().did_commit_after_animating = false;
+  }
+
   if (impl().layer_tree_host_impl->pending_tree())
     impl().layer_tree_host_impl->pending_tree()->UpdateDrawProperties();