cc: Do not use current frame number in state machine decisions.

This allows the scheduler to evolve to a state where it's able to
perform multiple actions of the same type in a given frame. One example
of this is webview where we can stop receiving BeginFrames but still
have to draw any number of times the system asks us to.

BUG=439275

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

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

cc/scheduler/scheduler_state_machine.cc

@@ -26,6 +26,10 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       last_frame_number_swap_performed_(-1),
       last_frame_number_swap_requested_(-1),
       last_frame_number_begin_main_frame_sent_(-1),
+      animate_funnel_(false),
+      perform_swap_funnel_(false),
+      request_swap_funnel_(false),
+      send_begin_main_frame_funnel_(false),
       prepare_tiles_funnel_(0),
       consecutive_checkerboard_animations_(0),
       max_pending_swaps_(1),
@@ -41,7 +45,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),
@@ -171,7 +174,6 @@ void SchedulerStateMachine::AsValueInto(
   state->BeginDictionary("minor_state");
   state->SetInteger("commit_count", commit_count_);
   state->SetInteger("current_frame_number", current_frame_number_);
-
   state->SetInteger("last_frame_number_animate_performed",
                     last_frame_number_animate_performed_);
   state->SetInteger("last_frame_number_swap_performed",
@@ -180,8 +182,12 @@ void SchedulerStateMachine::AsValueInto(
                     last_frame_number_swap_requested_);
   state->SetInteger("last_frame_number_begin_main_frame_sent",
                     last_frame_number_begin_main_frame_sent_);
-
-  state->SetInteger("prepare_tiles_funnel", prepare_tiles_funnel_);
+  state->SetBoolean("funnel: animate_funnel", animate_funnel_);
+  state->SetBoolean("funnel: perform_swap_funnel", perform_swap_funnel_);
+  state->SetBoolean("funnel: request_swap_funnel", request_swap_funnel_);
+  state->SetBoolean("funnel: send_begin_main_frame_funnel",
+                    send_begin_main_frame_funnel_);
+  state->SetInteger("funnel: prepare_tiles_funnel", prepare_tiles_funnel_);
   state->SetInteger("consecutive_checkerboard_animations",
                     consecutive_checkerboard_animations_);
   state->SetInteger("max_pending_swaps_", max_pending_swaps_);
@@ -198,7 +204,6 @@ void SchedulerStateMachine::AsValueInto(
                     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",
@@ -218,6 +223,11 @@ void SchedulerStateMachine::AsValueInto(
 void SchedulerStateMachine::AdvanceCurrentFrameNumber() {
   current_frame_number_++;
 
+  animate_funnel_ = false;
+  perform_swap_funnel_ = false;
+  request_swap_funnel_ = false;
+  send_begin_main_frame_funnel_ = false;
+
   // "Drain" the PrepareTiles funnel.
   if (prepare_tiles_funnel_ > 0)
     prepare_tiles_funnel_--;
@@ -227,23 +237,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_;
-}
-
-bool SchedulerStateMachine::HasSwappedThisFrame() const {
-  return current_frame_number_ == last_frame_number_swap_performed_;
-}
-
-bool SchedulerStateMachine::HasRequestedSwapThisFrame() const {
-  return current_frame_number_ == last_frame_number_swap_requested_;
-}
-
 bool SchedulerStateMachine::PendingDrawsShouldBeAborted() const {
   // These are all the cases where we normally cannot or do not want to draw
   // but, if needs_redraw_ is true and we do not draw to make forward progress,
@@ -316,17 +309,14 @@ bool SchedulerStateMachine::ShouldDraw() const {
   if (PendingDrawsShouldBeAborted())
     return active_tree_needs_first_draw_;
 
-  // Don't draw if we are waiting on the first commit after a surface.
-  if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE)
-    return false;
-
-  // If a commit has occurred after the animate call, we need to call animate
-  // again before we should draw.
-  if (did_commit_after_animating_)
+  // Do not draw too many times in a single frame. It's okay that we don't check
+  // this before checking for aborted draws because aborted draws do not request
+  // a swap.
+  if (request_swap_funnel_)
     return false;
 
-  // After this line, we only want to send a swap request once per frame.
-  if (HasRequestedSwapThisFrame())
+  // Don't draw if we are waiting on the first commit after a surface.
+  if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE)
     return false;
 
   // Do not queue too many swaps.
@@ -365,12 +355,12 @@ bool SchedulerStateMachine::ShouldActivatePendingTree() const {
 }
 
 bool SchedulerStateMachine::ShouldAnimate() const {
-  // Don't animate if we are waiting on the first commit after a surface.
-  if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE)
+  // Do not animate too many times in a single frame.
+  if (animate_funnel_)
     return false;
 
-  // If a commit occurred after our last call, we need to do animation again.
-  if (HasAnimatedThisFrame() && !did_commit_after_animating_)
+  // Don't animate if we are waiting on the first commit after a surface.
+  if (output_surface_state_ != OUTPUT_SURFACE_ACTIVE)
     return false;
 
   if (begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING &&
@@ -381,6 +371,10 @@ bool SchedulerStateMachine::ShouldAnimate() const {
 }
 
 bool SchedulerStateMachine::CouldSendBeginMainFrame() const {
+  // Do not send begin main frame too many times in a single frame.
+  if (send_begin_main_frame_funnel_)
+    return false;
+
   if (!needs_commit_)
     return false;
 
@@ -424,10 +418,6 @@ bool SchedulerStateMachine::ShouldSendBeginMainFrame() const {
   if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_COMMIT)
     return true;
 
-  // After this point, we only start a commit once per frame.
-  if (HasSentBeginMainFrameThisFrame())
-    return false;
-
   // We shouldn't normally accept commits if there isn't an OutputSurface.
   if (!HasInitializedOutputSurface())
     return false;
@@ -436,7 +426,7 @@ bool SchedulerStateMachine::ShouldSendBeginMainFrame() const {
   // TODO(brianderson): Remove this restriction to improve throughput.
   bool just_swapped_in_deadline =
       begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE &&
-      HasSwappedThisFrame();
+      perform_swap_funnel_;
   if (pending_swaps_ >= max_pending_swaps_ && !just_swapped_in_deadline)
     return false;
 
@@ -513,9 +503,10 @@ void SchedulerStateMachine::UpdateState(Action action) {
       return;
 
     case ACTION_ANIMATE:
+      DCHECK(!animate_funnel_);
       last_frame_number_animate_performed_ = current_frame_number_;
+      animate_funnel_ = true;
       needs_animate_ = false;
-      did_commit_after_animating_ = false;
       // TODO(skyostil): Instead of assuming this, require the client to tell
       // us.
       SetNeedsRedraw();
@@ -525,8 +516,10 @@ void SchedulerStateMachine::UpdateState(Action action) {
       DCHECK(!has_pending_tree_ ||
              settings_.main_frame_before_activation_enabled);
       DCHECK(visible_);
+      DCHECK(!send_begin_main_frame_funnel_);
       commit_state_ = COMMIT_STATE_BEGIN_MAIN_FRAME_SENT;
       needs_commit_ = false;
+      send_begin_main_frame_funnel_ = true;
       last_frame_number_begin_main_frame_sent_ =
           current_frame_number_;
       return;
@@ -571,8 +564,8 @@ void SchedulerStateMachine::UpdateState(Action action) {
 void SchedulerStateMachine::UpdateStateOnCommit(bool commit_has_no_updates) {
   commit_count_++;
 
-  if (!commit_has_no_updates && HasAnimatedThisFrame())
-    did_commit_after_animating_ = true;
+  if (!commit_has_no_updates)
+    animate_funnel_ = false;
 
   if (commit_has_no_updates || settings_.main_frame_before_activation_enabled) {
     commit_state_ = COMMIT_STATE_IDLE;
@@ -651,8 +644,11 @@ void SchedulerStateMachine::UpdateStateOnDraw(bool did_request_swap) {
   needs_redraw_ = false;
   active_tree_needs_first_draw_ = false;
 
-  if (did_request_swap)
+  if (did_request_swap) {
+    DCHECK(!request_swap_funnel_);
+    request_swap_funnel_ = true;
     last_frame_number_swap_requested_ = current_frame_number_;
+  }
 }
 
 void SchedulerStateMachine::UpdateStateOnPrepareTiles() {
@@ -767,7 +763,7 @@ bool SchedulerStateMachine::ProactiveBeginFrameWanted() const {
   // SetNeedsBeginFrame requests, which may propagate to the BeginImplFrame
   // provider and get sampled at an inopportune time, delaying the next
   // BeginImplFrame.
-  if (HasRequestedSwapThisFrame())
+  if (request_swap_funnel_)
     return true;
 
   return false;
@@ -861,7 +857,7 @@ bool SchedulerStateMachine::MainThreadIsInHighLatencyMode() const {
 
   // If we just sent a BeginMainFrame and haven't hit the deadline yet, the main
   // thread is in a low latency mode.
-  if (HasSentBeginMainFrameThisFrame() &&
+  if (send_begin_main_frame_funnel_ &&
       (begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING ||
        begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME))
     return false;
@@ -885,8 +881,8 @@ bool SchedulerStateMachine::MainThreadIsInHighLatencyMode() const {
     // Even if there's a new active tree to draw at the deadline or we've just
     // swapped it, it may have been triggered by a previous BeginImplFrame, in
     // which case the main thread is in a high latency mode.
-    return (active_tree_needs_first_draw_ || HasSwappedThisFrame()) &&
-           !HasSentBeginMainFrameThisFrame();
+    return (active_tree_needs_first_draw_ || perform_swap_funnel_) &&
+           !send_begin_main_frame_funnel_;
   }
 
   // If the active tree needs its first draw in any other state, we know the
@@ -927,7 +923,9 @@ void SchedulerStateMachine::SetMaxSwapsPending(int max) {
 void SchedulerStateMachine::DidSwapBuffers() {
   pending_swaps_++;
   DCHECK_LE(pending_swaps_, max_pending_swaps_);
+  DCHECK(!perform_swap_funnel_);
 
+  perform_swap_funnel_ = true;
   last_frame_number_swap_performed_ = current_frame_number_;
 }
 

cc/scheduler/scheduler_state_machine.h

@@ -278,10 +278,6 @@ class CC_EXPORT SchedulerStateMachine {
   bool ShouldPrepareTiles() const;
 
   void AdvanceCurrentFrameNumber();
-  bool HasAnimatedThisFrame() const;
-  bool HasSentBeginMainFrameThisFrame() const;
-  bool HasRequestedSwapThisFrame() const;
-  bool HasSwappedThisFrame() const;
 
   void UpdateStateOnCommit(bool commit_had_no_updates);
   void UpdateStateOnActivation();
@@ -295,6 +291,7 @@ class CC_EXPORT SchedulerStateMachine {
   CommitState commit_state_;
   ForcedRedrawOnTimeoutState forced_redraw_state_;
 
+  // These are used for tracing only.
   int commit_count_;
   int current_frame_number_;
   int last_frame_number_animate_performed_;
@@ -302,11 +299,18 @@ class CC_EXPORT SchedulerStateMachine {
   int last_frame_number_swap_requested_;
   int last_frame_number_begin_main_frame_sent_;
 
+  // These are used to ensure that an action only happens once per frame,
+  // deadline, etc.
+  bool animate_funnel_;
+  bool perform_swap_funnel_;
+  bool request_swap_funnel_;
+  bool send_begin_main_frame_funnel_;
   // prepare_tiles_funnel_ is "filled" each time PrepareTiles is called
   // and "drained" on each BeginImplFrame. If the funnel gets too full,
   // we start throttling ACTION_PREPARE_TILES such that we average one
   // PrepareTiles per BeginImplFrame.
   int prepare_tiles_funnel_;
+
   int consecutive_checkerboard_animations_;
   int max_pending_swaps_;
   int pending_swaps_;
@@ -321,7 +325,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_;