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

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 e2c66e87e555a1364012e3ca2630e2a382cd82f5..29f1ba00f559d52397e2b3ba47e970a4b7d78962 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/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),
@@ -180,7 +183,11 @@ 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->SetBoolean("animate_funnel", animate_funnel_);
+  state->SetBoolean("perform_swap_funnel", perform_swap_funnel_);
+  state->SetBoolean("request_swap_funnel", request_swap_funnel_);
+  state->SetBoolean("send_begin_main_frame_funnel",
+                    send_begin_main_frame_funnel_);
   state->SetInteger("prepare_tiles_funnel", prepare_tiles_funnel_);
   state->SetInteger("consecutive_checkerboard_animations",
                     consecutive_checkerboard_animations_);
@@ -198,7 +205,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 +224,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 +238,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,
@@ -320,13 +314,8 @@ bool SchedulerStateMachine::ShouldDraw() const {
   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_)
-    return false;
-
   // After this line, we only want to send a swap request once per frame.
-  if (HasRequestedSwapThisFrame())
+  if (request_swap_funnel_)
     return false;
 
   // Do not queue too many swaps.
@@ -370,7 +359,7 @@ bool SchedulerStateMachine::ShouldAnimate() const {
     return false;
 
   // If a commit occurred after our last call, we need to do animation again.
-  if (HasAnimatedThisFrame() && !did_commit_after_animating_)
+  if (animate_funnel_)
     return false;
 
   if (begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING &&
@@ -425,7 +414,7 @@ bool SchedulerStateMachine::ShouldSendBeginMainFrame() const {
     return true;
 
   // After this point, we only start a commit once per frame.
-  if (HasSentBeginMainFrameThisFrame())
+  if (send_begin_main_frame_funnel_)
     return false;
 
   // We shouldn't normally accept commits if there isn't an OutputSurface.
@@ -436,7 +425,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;
 
@@ -514,8 +503,8 @@ void SchedulerStateMachine::UpdateState(Action action) {
 
     case ACTION_ANIMATE:
       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();
@@ -527,6 +516,7 @@ void SchedulerStateMachine::UpdateState(Action action) {
       DCHECK(visible_);
       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 +561,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;

[brianderson, 2015/03/19 23:31:43]

Neat way of getting rid of did_commit_after_animating_.

[sunnyps, 2015/03/20 01:13:44]

Acknowledged.

 
   if (commit_has_no_updates || settings_.main_frame_before_activation_enabled) {
     commit_state_ = COMMIT_STATE_IDLE;
@@ -651,8 +641,10 @@ void SchedulerStateMachine::UpdateStateOnDraw(bool did_request_swap) {
   needs_redraw_ = false;
   active_tree_needs_first_draw_ = false;
 
-  if (did_request_swap)
+  if (did_request_swap) {
+    request_swap_funnel_ = true;

[brianderson, 2015/03/19 23:31:43]

For all the places where we set the funnels to true, should we DCHECK that they
were previously false?

[sunnyps, 2015/03/20 01:13:44]

Done.

     last_frame_number_swap_requested_ = current_frame_number_;
+  }
 }
 
 void SchedulerStateMachine::UpdateStateOnPrepareTiles() {
@@ -790,7 +782,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;
@@ -884,7 +876,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;
@@ -908,8 +900,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
@@ -951,6 +943,7 @@ void SchedulerStateMachine::DidSwapBuffers() {
   pending_swaps_++;
   DCHECK_LE(pending_swaps_, max_pending_swaps_);
 
+  perform_swap_funnel_ = true;
   last_frame_number_swap_performed_ = current_frame_number_;
 }