[cc] Calculate the correct latest_confirmed_sequence_number in cc::Scheduler.

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"
 #include "base/trace_event/trace_event_argument.h"
 #include "base/values.h"
+#include "cc/output/begin_frame_args.h"
 
 namespace cc {
 
 namespace {
 // Surfaces and CompositorTimingHistory don't support more than 1 pending swap.
 const int kMaxPendingSubmitFrames = 1;
 }  // namespace
 
 SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
     : settings_(settings),
       compositor_frame_sink_state_(COMPOSITOR_FRAME_SINK_NONE),
       begin_impl_frame_state_(BEGIN_IMPL_FRAME_STATE_IDLE),
       begin_main_frame_state_(BEGIN_MAIN_FRAME_STATE_IDLE),
       forced_redraw_state_(FORCED_REDRAW_STATE_IDLE),
+      begin_frame_source_id_(0),
+      begin_frame_sequence_number_(BeginFrameArgs::kStartingFrameNumber),
+      last_begin_frame_sequence_number_begin_main_frame_sent_(
+          BeginFrameArgs::kInvalidFrameNumber),
+      last_begin_frame_sequence_number_pending_tree_was_fresh_(
+          BeginFrameArgs::kInvalidFrameNumber),
+      last_begin_frame_sequence_number_active_tree_was_fresh_(
+          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),
       invalidate_compositor_frame_sink_funnel_(false),
       prepare_tiles_funnel_(0),
@@ skipping 170 matching lines @@
 
   state->BeginDictionary("minor_state");
   state->SetInteger("commit_count", commit_count_);
   state->SetInteger("current_frame_number", current_frame_number_);
   state->SetInteger("last_frame_number_submit_performed",
                     last_frame_number_submit_performed_);
   state->SetInteger("last_frame_number_draw_performed",
                     last_frame_number_draw_performed_);
   state->SetInteger("last_frame_number_begin_main_frame_sent",
                     last_frame_number_begin_main_frame_sent_);
+  state->SetInteger("begin_frame_source_id", begin_frame_source_id_);
+  state->SetInteger("begin_frame_sequence_number",
+                    begin_frame_sequence_number_);
+  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->SetInteger("funnel: prepare_tiles_funnel", prepare_tiles_funnel_);
   state->SetBoolean("funnel: invalidate_compositor_frame_sink_funnel",
                     invalidate_compositor_frame_sink_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",
@@ skipping 340 matching lines @@
 
 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_);
   begin_main_frame_state_ = BEGIN_MAIN_FRAME_STATE_SENT;
   needs_begin_main_frame_ = false;
   send_begin_main_frame_funnel_ = 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::WillCommit(bool commit_has_no_updates) {
   DCHECK(!has_pending_tree_ ||
          (settings_.main_frame_before_activation_enabled &&
           commit_has_no_updates));
   commit_count_++;
   last_commit_had_no_updates_ = commit_has_no_updates;
   begin_main_frame_state_ = BEGIN_MAIN_FRAME_STATE_IDLE;
 
-  if (!commit_has_no_updates) {
-    // Pending tree only exists if commit had updates.
+  if (commit_has_no_updates) {
+    // Pending tree might still exist from prior commit.
+    if (has_pending_tree_) {
+      DCHECK(settings_.main_frame_before_activation_enabled);
+      last_begin_frame_sequence_number_pending_tree_was_fresh_ =
+          last_begin_frame_sequence_number_begin_main_frame_sent_;
+    } else {
+      last_begin_frame_sequence_number_active_tree_was_fresh_ =
+          last_begin_frame_sequence_number_begin_main_frame_sent_;

[brianderson, 2017/02/14 23:58:56]

Do you also need to do:
if (...active_tree_was_fresh_ == ...compositor_frame_was_fresh_)
  ...compositor_frame_was_fresh_ = ...begin_main_frame_sent_

[Eric Seckler, 2017/02/17 00:07:26]

Brian and I chatted about why this works, even if impl thread redraws are
pending at the same time. Essentially, we assume that:
(a) any imp-side needs_redraw_=true changes are aligned with BeginFrames. This
isn't completely true at the moment, but work is underway to ensure this.
(b) a main-thread commit without updates has considered the effects of any
(impl-side) inputs/animations/etc that were pending at the time of
BeginMainFrame. That is, any such effects also did not cause updates.

Given this, it is fine to update the compositor frame freshness if it previously
was as fresh as the active tree.

Added the branch and also an expectation to the state machine tests.

+    }
+  } else {
+    // We have a new pending tree.
     has_pending_tree_ = true;
     pending_tree_is_ready_for_activation_ = false;
+    last_begin_frame_sequence_number_pending_tree_was_fresh_ =
+        last_begin_frame_sequence_number_begin_main_frame_sent_;
     wait_for_ready_to_draw_ = settings_.commit_to_active_tree;
   }
 
   // Update state related to forced draws.
   if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_COMMIT) {
     forced_redraw_state_ = has_pending_tree_
                                ? FORCED_REDRAW_STATE_WAITING_FOR_ACTIVATION
                                : FORCED_REDRAW_STATE_WAITING_FOR_DRAW;
   }
 
@@ skipping 11 matching lines @@
       COMPOSITOR_FRAME_SINK_WAITING_FOR_FIRST_ACTIVATION)
     compositor_frame_sink_state_ = COMPOSITOR_FRAME_SINK_ACTIVE;
 
   if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_ACTIVATION)
     forced_redraw_state_ = FORCED_REDRAW_STATE_WAITING_FOR_DRAW;
 
   has_pending_tree_ = false;
   pending_tree_is_ready_for_activation_ = false;
   active_tree_needs_first_draw_ = true;
   needs_redraw_ = true;
+  last_begin_frame_sequence_number_active_tree_was_fresh_ =
+      last_begin_frame_sequence_number_pending_tree_was_fresh_;
 }
 
 void SchedulerStateMachine::WillDrawInternal() {
   // If a new active tree is pending after the one we are about to draw,
   // the main thread is in a high latency mode.
   // main_thread_missed_last_deadline_ is here in addition to
   // OnBeginImplFrameIdle for cases where the scheduler aborts draws outside
   // of the deadline.
   main_thread_missed_last_deadline_ = CommitPending() || has_pending_tree_;
 
@@ skipping 176 matching lines @@
     return true;
 
   // If the last commit was aborted because of early out (no updates), we should
   // still want a begin frame in case there is a commit coming again.
   if (last_commit_had_no_updates_)
     return true;
 
   return false;
 }
 
-void SchedulerStateMachine::OnBeginImplFrame() {
+void SchedulerStateMachine::OnBeginFrameDroppedNotObserving(
+    uint32_t source_id,
+    uint64_t sequence_number) {
+  DCHECK(!BeginFrameNeeded());
+  DCHECK_EQ(BEGIN_IMPL_FRAME_STATE_IDLE, begin_impl_frame_state_);
+
+  // Confirms the dropped BeginFrame, since we don't have updates.
+  UpdateBeginFrameSequenceNumbersForBeginFrame(source_id, sequence_number);
+  UpdateBeginFrameSequenceNumbersForBeginFrameDeadline();
+}
+
+void SchedulerStateMachine::UpdateBeginFrameSequenceNumbersForBeginFrame(
+    uint32_t source_id,
+    uint64_t sequence_number) {
+  if (source_id != begin_frame_source_id_) {
+    begin_frame_source_id_ = source_id;
+    begin_frame_sequence_number_ = sequence_number;
+
+    // Reset freshness sequence numbers.
+    last_begin_frame_sequence_number_begin_main_frame_sent_ =
+        BeginFrameArgs::kInvalidFrameNumber;
+    last_begin_frame_sequence_number_active_tree_was_fresh_ =
+        BeginFrameArgs::kInvalidFrameNumber;
+    last_begin_frame_sequence_number_pending_tree_was_fresh_ =
+        BeginFrameArgs::kInvalidFrameNumber;
+    last_begin_frame_sequence_number_compositor_frame_was_fresh_ =
+        BeginFrameArgs::kInvalidFrameNumber;
+  } else {
+    DCHECK_GE(sequence_number, begin_frame_sequence_number_);

[brianderson, 2017/02/14 22:51:08]

DCHECK_GT?

[Eric Seckler, 2017/02/14 23:43:45]

This requires that we set begin_frame_sequence_number_ = kInvalidFrameNumber
initially (currently its kStartingFrameNumber). Let's see if that works on the
bots :)

+    begin_frame_sequence_number_ = sequence_number;
+  }
+}
+
+void SchedulerStateMachine::
+    UpdateBeginFrameSequenceNumbersForBeginFrameDeadline() {
+  // Update frame numbers for freshness in case no updates were necessary.
+  if (begin_main_frame_state_ == BEGIN_MAIN_FRAME_STATE_IDLE &&

[brianderson, 2017/02/14 22:51:09]

Can you restructure this with early returns to reduce nesting?

[Eric Seckler, 2017/02/14 23:43:45]

Done.

+      !needs_begin_main_frame_) {
+    if (has_pending_tree_) {
+      last_begin_frame_sequence_number_pending_tree_was_fresh_ =
+          begin_frame_sequence_number_;
+    } else {
+      last_begin_frame_sequence_number_active_tree_was_fresh_ =
+          begin_frame_sequence_number_;
+
+      if (!needs_redraw_)
+        last_begin_frame_sequence_number_compositor_frame_was_fresh_ =
+            begin_frame_sequence_number_;
+    }
+  }
+}
+
+void SchedulerStateMachine::OnBeginImplFrame(uint32_t source_id,
+                                             uint64_t sequence_number) {
+  UpdateBeginFrameSequenceNumbersForBeginFrame(source_id, sequence_number);
+
   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;
   invalidate_compositor_frame_sink_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;
 
   // Clear funnels for any actions we perform during the deadline.
   draw_funnel_ = false;
 
   // Allow one PrepareTiles per draw for synchronous compositor.
   if (settings_.using_synchronous_renderer_compositor) {
     if (prepare_tiles_funnel_ > 0)
       prepare_tiles_funnel_--;
   }
+
+  UpdateBeginFrameSequenceNumbersForBeginFrameDeadline();
 }
 
 void SchedulerStateMachine::OnBeginImplFrameIdle() {
   begin_impl_frame_state_ = BEGIN_IMPL_FRAME_STATE_IDLE;
 
   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_ =
@@ skipping 112 matching lines @@
 void SchedulerStateMachine::DidSubmitCompositorFrame() {
   TRACE_EVENT_ASYNC_BEGIN1("cc", "Scheduler:pending_submit_frames", this,
                            "pending_frames", pending_submit_frames_);
   DCHECK_LT(pending_submit_frames_, kMaxPendingSubmitFrames);
 
   pending_submit_frames_++;
   submit_frames_with_current_compositor_frame_sink_++;
 
   did_submit_in_last_frame_ = true;
   last_frame_number_submit_performed_ = current_frame_number_;
+  last_begin_frame_sequence_number_compositor_frame_was_fresh_ =
+      last_begin_frame_sequence_number_active_tree_was_fresh_;
 }
 
 void SchedulerStateMachine::DidReceiveCompositorFrameAck() {
   TRACE_EVENT_ASYNC_END1("cc", "Scheduler:pending_submit_frames", this,
                          "pending_frames", pending_submit_frames_);
   pending_submit_frames_--;
 }
 
 void SchedulerStateMachine::SetTreePrioritiesAndScrollState(
     TreePriority tree_priority,
@@ skipping 114 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