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

cc/scheduler/scheduler_state_machine.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/scheduler/scheduler_state_machine.h"
 
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
@@ skipping 24 matching lines @@
           BeginFrameArgs::kInvalidFrameNumber),
       last_begin_frame_sequence_number_compositor_frame_was_fresh_(
           BeginFrameArgs::kInvalidFrameNumber),
       commit_count_(0),
       current_frame_number_(0),
       last_frame_number_submit_performed_(-1),
       last_frame_number_draw_performed_(-1),
       last_frame_number_begin_main_frame_sent_(-1),
       last_frame_number_invalidate_compositor_frame_sink_performed_(-1),
       draw_funnel_(false),
-      send_begin_main_frame_funnel_(true),
+      did_send_begin_main_frame_for_current_frame_(true),
       invalidate_compositor_frame_sink_funnel_(false),
       impl_side_invalidation_funnel_(false),
+      did_notify_begin_main_frame_not_sent_(false),
+      did_commit_during_frame_(false),
       prepare_tiles_funnel_(0),
       consecutive_checkerboard_animations_(0),
       pending_submit_frames_(0),
       submit_frames_with_current_compositor_frame_sink_(0),
       needs_redraw_(false),
       needs_prepare_tiles_(false),
       needs_begin_main_frame_(false),
       needs_one_begin_impl_frame_(false),
       visible_(false),
       begin_frame_source_paused_(false),
@@ skipping 129 matching lines @@
     case ACTION_DRAW_ABORT:
       return "ACTION_DRAW_ABORT";
     case ACTION_BEGIN_COMPOSITOR_FRAME_SINK_CREATION:
       return "ACTION_BEGIN_COMPOSITOR_FRAME_SINK_CREATION";
     case ACTION_PREPARE_TILES:
       return "ACTION_PREPARE_TILES";
     case ACTION_INVALIDATE_COMPOSITOR_FRAME_SINK:
       return "ACTION_INVALIDATE_COMPOSITOR_FRAME_SINK";
     case ACTION_PERFORM_IMPL_SIDE_INVALIDATION:
       return "ACTION_PERFORM_IMPL_SIDE_INVALIDATION";
+    case ACTION_NOTIFY_BEGIN_MAIN_FRAME_NOT_SENT:
+      return "ACTION_NOTIFY_BEGIN_MAIN_FRAME_NOT_SENT";
   }
   NOTREACHED();
   return "???";
 }
 
 std::unique_ptr<base::trace_event::ConvertableToTraceFormat>
 SchedulerStateMachine::AsValue() const {
   std::unique_ptr<base::trace_event::TracedValue> state(
       new base::trace_event::TracedValue());
   AsValueInto(state.get());
@@ skipping 30 matching lines @@
   state->SetInteger("last_begin_frame_sequence_number_begin_main_frame_sent",
                     last_begin_frame_sequence_number_begin_main_frame_sent_);
   state->SetInteger("last_begin_frame_sequence_number_pending_tree_was_fresh",
                     last_begin_frame_sequence_number_pending_tree_was_fresh_);
   state->SetInteger("last_begin_frame_sequence_number_active_tree_was_fresh",
                     last_begin_frame_sequence_number_active_tree_was_fresh_);
   state->SetInteger(
       "last_begin_frame_sequence_number_compositor_frame_was_fresh",
       last_begin_frame_sequence_number_compositor_frame_was_fresh_);
   state->SetBoolean("funnel: draw_funnel", draw_funnel_);
-  state->SetBoolean("funnel: send_begin_main_frame_funnel",
-                    send_begin_main_frame_funnel_);
+  state->SetBoolean("funnel: did_send_begin_main_frame",
+                    did_send_begin_main_frame_for_current_frame_);
+  state->SetBoolean("funnel: did_notify_begin_main_frame_not_sent",
+                    did_notify_begin_main_frame_not_sent_);
+  state->SetBoolean("funnel: did_commit_during_frame",
+                    did_commit_during_frame_);
   state->SetInteger("funnel: prepare_tiles_funnel", prepare_tiles_funnel_);
   state->SetBoolean("funnel: invalidate_compositor_frame_sink_funnel",
                     invalidate_compositor_frame_sink_funnel_);
   state->SetBoolean("funnel: impl_side_invalidation_funnel",
                     impl_side_invalidation_funnel_);
   state->SetInteger("consecutive_checkerboard_animations",
                     consecutive_checkerboard_animations_);
   state->SetInteger("pending_submit_frames_", pending_submit_frames_);
   state->SetInteger("submit_frames_with_current_compositor_frame_sink",
                     submit_frames_with_current_compositor_frame_sink_);
@@ skipping 162 matching lines @@
     return false;
 
   // If we want to force activation, do so ASAP.
   if (PendingActivationsShouldBeForced())
     return true;
 
   // At this point, only activate if we are ready to activate.
   return pending_tree_is_ready_for_activation_;
 }
 
+bool SchedulerStateMachine::ShouldNotifyBeginMainFrameNotSent() const {

[brianderson, 2017/04/07 22:03:16]

Is it okay that we'll be sending this at the start of every frame while the
compositor is active?

Should blink explicitly request this signal?

[Dan Elphick, 2017/04/10 15:53:22]

We're aware this might cause some problems, but we'd like to assess the impact
first and then add the extra complexity later if/when we determine this is
having a negative impact.

Assuming it is a problem, I think we can modify the renderer scheduler to signal
the cc scheduler that it wants this signal whenever the idle queue changes from
empty to non-empty and vice versa.

+  // This method returns true if most of the conditions for sending a
+  // BeginMainFrame are met, but one is not actually requested. This gives the
+  // main thread the chance to do something else.
+
+  // Don't notify if a BeginMainFrame has already been requested.
+  if (needs_begin_main_frame_ || did_send_begin_main_frame_for_current_frame_)

[brianderson, 2017/04/07 22:03:16]

I think the did_send_begin_main_frame_for_current_frame_ check can be removed
since it is redundant with the combination of checks on did_commit_during_frame_
and begin_main_frame_state_.

[Dan Elphick, 2017/04/10 15:53:22]

It is actually necessary as it's the flag is initialized to true which stops it
sending this notification straight away (which caused every single state machine
test to fail).

However I can remove this check and fix that by just tweaking the initialization
of did_notify_begin_main_frame_not_sent_ to true.

+    return false;
+
+  // Only notify when we're visible.
+  if (!visible_)
+    return false;
+
+  // There are no BeginImplFrames while BeginFrameSource is paused, meaning
+  // the scheduler should send SendBeginMainFrameNotExpectedSoon instead,
+  // indicating a longer period of inactivity.
+  if (begin_frame_source_paused_)
+    return false;
+
+  // Do not notify that no BeginMainFrame was sent too many times in a single
+  // frame.
+  if (did_notify_begin_main_frame_not_sent_)
+    return false;
+
+  // Do not notify if a commit happened during this frame as the main thread
+  // will already be active and does not need to be woken up to make further
+  // actions. (This occurs if the main frame was scheduled but didn't complete
+  // before the vsync deadline).
+  if (did_commit_during_frame_)
+    return false;
+
+  // If a BeginMainFrame is in progress, there's no need to activate it.
+  if (begin_main_frame_state_ != BEGIN_MAIN_FRAME_STATE_IDLE)

[brianderson, 2017/04/07 22:03:16]

Group this with the needs_begin_main_frame_ check on line 445.

[Dan Elphick, 2017/04/10 15:53:22]

Done.

+    return false;
+
+  return true;
+}
+
 bool SchedulerStateMachine::CouldSendBeginMainFrame() const {
   if (!needs_begin_main_frame_)
     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::ShouldSendBeginMainFrame() const {
   if (!CouldSendBeginMainFrame())
     return false;
 
   // Do not send begin main frame too many times in a single frame or before
   // the first BeginFrame.
-  if (send_begin_main_frame_funnel_)
+  if (did_send_begin_main_frame_for_current_frame_)
     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.
@@ skipping 123 matching lines @@
   if (ShouldPerformImplSideInvalidation())
     return ACTION_PERFORM_IMPL_SIDE_INVALIDATION;
   if (ShouldPrepareTiles())
     return ACTION_PREPARE_TILES;
   if (ShouldSendBeginMainFrame())
     return ACTION_SEND_BEGIN_MAIN_FRAME;
   if (ShouldInvalidateCompositorFrameSink())
     return ACTION_INVALIDATE_COMPOSITOR_FRAME_SINK;
   if (ShouldBeginCompositorFrameSinkCreation())
     return ACTION_BEGIN_COMPOSITOR_FRAME_SINK_CREATION;
+  if (ShouldNotifyBeginMainFrameNotSent())
+    return ACTION_NOTIFY_BEGIN_MAIN_FRAME_NOT_SENT;
   return ACTION_NONE;
 }
 
 bool SchedulerStateMachine::ShouldPerformImplSideInvalidation() const {
   if (!needs_impl_side_invalidation_)
     return false;
 
   if (!CouldCreatePendingTree())
     return false;
 
@@ skipping 65 matching lines @@
   if (!HasInitializedCompositorFrameSink())
     return false;
 
   return true;
 }
 
 void SchedulerStateMachine::WillSendBeginMainFrame() {
   DCHECK(!has_pending_tree_ || settings_.main_frame_before_activation_enabled);
   DCHECK(visible_);
   DCHECK(!begin_frame_source_paused_);
-  DCHECK(!send_begin_main_frame_funnel_);
+  DCHECK(!did_send_begin_main_frame_for_current_frame_);
   begin_main_frame_state_ = BEGIN_MAIN_FRAME_STATE_SENT;
   needs_begin_main_frame_ = false;
-  send_begin_main_frame_funnel_ = true;
+  did_send_begin_main_frame_for_current_frame_ = true;
   last_frame_number_begin_main_frame_sent_ = current_frame_number_;
   last_begin_frame_sequence_number_begin_main_frame_sent_ =
       begin_frame_sequence_number_;
 }
 
+void SchedulerStateMachine::WillNotifyBeginMainFrameNotSent() {
+  DCHECK(visible_);
+  DCHECK(!begin_frame_source_paused_);
+  DCHECK(!did_notify_begin_main_frame_not_sent_);
+  did_notify_begin_main_frame_not_sent_ = true;
+}
+
 void SchedulerStateMachine::WillCommit(bool commit_has_no_updates) {
   bool can_have_pending_tree =
       commit_has_no_updates &&
       (settings_.main_frame_before_activation_enabled ||
        current_pending_tree_is_impl_side_);
   DCHECK(!has_pending_tree_ || can_have_pending_tree);
   commit_count_++;
   last_commit_had_no_updates_ = commit_has_no_updates;
   begin_main_frame_state_ = BEGIN_MAIN_FRAME_STATE_IDLE;
+  did_commit_during_frame_ = true;
 
   if (commit_has_no_updates) {
     // Pending tree might still exist from prior commit.
     if (has_pending_tree_) {
       DCHECK(can_have_pending_tree);
       last_begin_frame_sequence_number_pending_tree_was_fresh_ =
           last_begin_frame_sequence_number_begin_main_frame_sent_;
     } else {
       if (last_begin_frame_sequence_number_compositor_frame_was_fresh_ ==
           last_begin_frame_sequence_number_active_tree_was_fresh_) {
@@ skipping 326 matching lines @@
 
   begin_impl_frame_state_ = BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME;
   current_frame_number_++;
 
   last_commit_had_no_updates_ = false;
   did_draw_in_last_frame_ = false;
   did_submit_in_last_frame_ = false;
   needs_one_begin_impl_frame_ = false;
 
   // Clear funnels for any actions we perform during the frame.
-  send_begin_main_frame_funnel_ = false;
+  did_notify_begin_main_frame_not_sent_ = false;
+  did_send_begin_main_frame_for_current_frame_ = false;
+  did_commit_during_frame_ = false;
   invalidate_compositor_frame_sink_funnel_ = false;
   impl_side_invalidation_funnel_ = false;
 
   // "Drain" the PrepareTiles funnel.
   if (prepare_tiles_funnel_ > 0)
     prepare_tiles_funnel_--;
 }
 
 void SchedulerStateMachine::OnBeginImplFrameDeadline() {
   begin_impl_frame_state_ = BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE;
@@ skipping 17 matching lines @@
   skip_next_begin_main_frame_to_reduce_latency_ = false;
 
   // If a new or undrawn active tree is pending after the deadline,
   // then the main thread is in a high latency mode.
   main_thread_missed_last_deadline_ =
       CommitPending() || has_pending_tree_ || active_tree_needs_first_draw_;
 
   // If we're entering a state where we won't get BeginFrames set all the
   // funnels so that we don't perform any actions that we shouldn't.
   if (!BeginFrameNeeded())
-    send_begin_main_frame_funnel_ = true;
+    did_send_begin_main_frame_for_current_frame_ = true;
 
   // Synchronous compositor finishes BeginFrames before triggering their
   // deadline. Therefore, we update sequence numbers when becoming idle, before
   // the Scheduler sends its BeginFrameAck.
   if (settings_.using_synchronous_renderer_compositor)
     UpdateBeginFrameSequenceNumbersForBeginFrameDeadline();
 }
 
 SchedulerStateMachine::BeginImplFrameDeadlineMode
 SchedulerStateMachine::CurrentBeginImplFrameDeadlineMode() const {
@@ skipping 236 matching lines @@
     case COMPOSITOR_FRAME_SINK_ACTIVE:
     case COMPOSITOR_FRAME_SINK_WAITING_FOR_FIRST_COMMIT:
     case COMPOSITOR_FRAME_SINK_WAITING_FOR_FIRST_ACTIVATION:
       return true;
   }
   NOTREACHED();
   return false;
 }
 
 }  // namespace cc