Create a new action triggered when a BeginMainFrame is not expected before vsync

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 e73508ce1ab1fb198c9b69ff0e65609a3e637e8e..94d24be78ed3f3a653d341c628c10167780f2e76 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -45,6 +45,7 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       send_begin_main_frame_funnel_(true),
       invalidate_compositor_frame_sink_funnel_(false),
       impl_side_invalidation_funnel_(false),
+      do_idle_work_funnel_(false),
       prepare_tiles_funnel_(0),
       consecutive_checkerboard_animations_(0),
       pending_submit_frames_(0),
@@ -194,6 +195,8 @@ const char* SchedulerStateMachine::ActionToString(Action action) {
       return "ACTION_INVALIDATE_COMPOSITOR_FRAME_SINK";
     case ACTION_PERFORM_IMPL_SIDE_INVALIDATION:
       return "ACTION_PERFORM_IMPL_SIDE_INVALIDATION";
+    case ACTION_DO_IDLE_WORK:
+      return "ACTION_DO_IDLE_WORK";
   }
   NOTREACHED();
   return "???";
@@ -246,6 +249,7 @@ void SchedulerStateMachine::AsValueInto(
   state->SetBoolean("funnel: draw_funnel", draw_funnel_);
   state->SetBoolean("funnel: send_begin_main_frame_funnel",
                     send_begin_main_frame_funnel_);
+  state->SetBoolean("funnel: do_idle_work_funnel", do_idle_work_funnel_);
   state->SetInteger("funnel: prepare_tiles_funnel", prepare_tiles_funnel_);
   state->SetBoolean("funnel: invalidate_compositor_frame_sink_funnel",
                     invalidate_compositor_frame_sink_funnel_);
@@ -428,6 +432,98 @@ bool SchedulerStateMachine::ShouldActivatePendingTree() const {
   return pending_tree_is_ready_for_activation_;
 }
 
+bool SchedulerStateMachine::CouldDoIdleWork() const {
+  if (needs_begin_main_frame_ || send_begin_main_frame_funnel_)

[Dan Elphick, 2017/03/21 11:23:51]

I made this change here because needs_begin_main_frame gets reset after it's
handled. So essentially we'd get idle work after every main frame.

I've tested the two cases we had though and they still work after this change.

[Sami, 2017/03/21 11:35:21]

I wonder if we should look at begin_main_frame_state_ instead? What states does
it cycle through? Basically we want to avoid doing any extra idle work if
there's a BeginMainFrame in any part of the pipeline I think (perhaps that means
we should look at pending trees too)

[Sami, 2017/03/21 11:36:39]

Just to clarify: the problem with looking at the funnel is that it's really a
rate limiting mechanism for BeginMainFrames, and if we decide to allow more
BeginMainFrames in the future it wouldn't be sufficient here anymore.

[Dan Elphick, 2017/03/21 15:56:25]

I've renamed the funnel as discussed in comments elsewhere.

+    return false;
+
+  // We can not perform commits if we are not visible.
+  if (!visible_)
+    return false;
+
+  // There are no BeginImplFrames while BeginFrameSource is paused,
+  // so should also stop BeginMainFrames.
+  if (begin_frame_source_paused_)
+    return false;
+
+  // Do not make a new commits when it is deferred.
+  if (defer_commits_)
+    return false;
+
+  return true;
+}
+
+bool SchedulerStateMachine::ShouldDoIdleWork() const {
+  if (!CouldDoIdleWork())
+    return false;
+
+  // Do not do idle work too many times in a single frame or before
+  // the first BeginFrame.
+  if (do_idle_work_funnel_)
+    return false;
+
+  // Only send BeginMainFrame when there isn't another commit pending already.
+  // Other parts of the state machine indirectly defer the BeginMainFrame
+  // by transitioning to WAITING commit states rather than going
+  // immediately to IDLE.
+  if (begin_main_frame_state_ != BEGIN_MAIN_FRAME_STATE_IDLE)
+    return false;
+
+  // MFBA is disabled and we are waiting for previous activation.
+  if (!settings_.main_frame_before_activation_enabled && has_pending_tree_)
+    return false;
+
+  // We are waiting for previous frame to be drawn, submitted and acked.
+  if (settings_.commit_to_active_tree &&
+      (active_tree_needs_first_draw_ || IsDrawThrottled())) {
+    return false;
+  }
+
+  // Don't send BeginMainFrame early if we are prioritizing the active tree
+  // because of ImplLatencyTakesPriority.
+  if (ImplLatencyTakesPriority() &&
+      (has_pending_tree_ || active_tree_needs_first_draw_)) {
+    return false;
+  }
+
+  // We should not send BeginMainFrame while we are in the idle state since we
+  // might have new user input arriving soon. It's okay to send BeginMainFrame
+  // for the synchronous compositor because the main thread is always high
+  // latency in that case.
+  // TODO(brianderson): Allow sending BeginMainFrame while idle when the main
+  // thread isn't consuming user input for non-synchronous compositor.
+  if (!settings_.using_synchronous_renderer_compositor &&
+      begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_IDLE) {
+    return false;
+  }
+
+  // We need a new commit for the forced redraw. This honors the
+  // single commit per interval because the result will be swapped to screen.
+  if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_COMMIT)
+    return true;
+
+  // We shouldn't normally accept commits if there isn't an CompositorFrameSink.
+  if (!HasInitializedCompositorFrameSink())
+    return false;
+
+  if (!settings_.main_frame_while_submit_frame_throttled_enabled) {
+    // Throttle the BeginMainFrames on CompositorFrameAck unless we just
+    // submitted a frame to potentially improve impl-thread latency over
+    // main-thread throughput.
+    // TODO(brianderson): Remove this restriction to improve throughput or
+    // make it conditional on ImplLatencyTakesPriority.
+    bool just_submitted_in_deadline =
+        begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE &&
+        did_submit_in_last_frame_;
+    if (IsDrawThrottled() && !just_submitted_in_deadline)
+      return false;
+  }
+
+  if (skip_next_begin_main_frame_to_reduce_latency_)
+    return false;
+
+  return true;
+}
+
 bool SchedulerStateMachine::CouldSendBeginMainFrame() const {
   if (!needs_begin_main_frame_)
     return false;
@@ -594,6 +690,8 @@ SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
     return ACTION_PREPARE_TILES;
   if (ShouldSendBeginMainFrame())
     return ACTION_SEND_BEGIN_MAIN_FRAME;
+  if (ShouldDoIdleWork())
+    return ACTION_DO_IDLE_WORK;
   if (ShouldInvalidateCompositorFrameSink())
     return ACTION_INVALIDATE_COMPOSITOR_FRAME_SINK;
   if (ShouldBeginCompositorFrameSinkCreation())
@@ -692,6 +790,14 @@ void SchedulerStateMachine::WillSendBeginMainFrame() {
       begin_frame_sequence_number_;
 }
 
+void SchedulerStateMachine::WillDoIdleWork() {
+  DCHECK(!has_pending_tree_ || settings_.main_frame_before_activation_enabled);
+  DCHECK(visible_);
+  DCHECK(!begin_frame_source_paused_);
+  DCHECK(!do_idle_work_funnel_);
+  do_idle_work_funnel_ = true;
+}
+
 void SchedulerStateMachine::WillCommit(bool commit_has_no_updates) {
   bool can_have_pending_tree =
       commit_has_no_updates &&
@@ -1047,6 +1153,7 @@ void SchedulerStateMachine::OnBeginImplFrame(uint32_t source_id,
   needs_one_begin_impl_frame_ = false;
 
   // Clear funnels for any actions we perform during the frame.
+  do_idle_work_funnel_ = false;
   send_begin_main_frame_funnel_ = false;
   invalidate_compositor_frame_sink_funnel_ = false;
   impl_side_invalidation_funnel_ = false;