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

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 91ae485850381b5ce41b3eb509a11d06deb5ba09..296d2becfad9d673d8168e8f7a9f122fbeaed4a4 100644
--- a/cc/scheduler/scheduler_state_machine.cc
+++ b/cc/scheduler/scheduler_state_machine.cc
@@ -10,6 +10,7 @@
 #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 {
 
@@ -24,6 +25,16 @@ SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
       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),
@@ -214,6 +225,18 @@ void SchedulerStateMachine::AsValueInto(
                     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_);
@@ -574,6 +597,8 @@ void SchedulerStateMachine::WillSendBeginMainFrame() {
   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) {
@@ -584,10 +609,22 @@ void SchedulerStateMachine::WillCommit(bool commit_has_no_updates) {
   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;
   }
 
@@ -619,6 +656,8 @@ void SchedulerStateMachine::WillActivate() {
   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() {
@@ -815,7 +854,62 @@ bool SchedulerStateMachine::ProactiveBeginFrameWanted() const {
   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_++;
 
@@ -844,6 +938,8 @@ void SchedulerStateMachine::OnBeginImplFrameDeadline() {
     if (prepare_tiles_funnel_ > 0)
       prepare_tiles_funnel_--;
   }
+
+  UpdateBeginFrameSequenceNumbersForBeginFrameDeadline();
 }
 
 void SchedulerStateMachine::OnBeginImplFrameIdle() {
@@ -976,6 +1072,8 @@ void SchedulerStateMachine::DidSubmitCompositorFrame() {
 
   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() {