[throughput] Fix handling delayed no-damage from main.

It is possible for the compositor to process multiple frames while
main-thread handles a single frame and responds with 'no damage'. Fix
the tracker to handle such cases correctly.

BUG=1021963

Change-Id: Id662be6ba4ca85171f541d893ab5bb093c236f66
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1995469
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730554}

cc/metrics/frame_sequence_tracker.cc

@@ -489,9 +489,6 @@ void FrameSequenceTracker::ReportBeginMainFrame(
   if (args.type == viz::BeginFrameArgs::NORMAL) {
     DCHECK(impl_frames_.contains(args.frame_id));
   }
-  last_started_main_sequence_ = args.frame_id.sequence_number;
-  DCHECK_EQ(last_started_impl_sequence_, last_started_main_sequence_)
-      << TRACKER_DCHECK_MSG;
 #endif
 
   UpdateTrackedFrameData(&begin_main_frame_data_, args.frame_id.source_id,
@@ -725,8 +722,10 @@ void FrameSequenceTracker::ReportMainFrameCausedNoDamage(
   TRACKER_TRACE_STREAM << "N(" << begin_main_frame_data_.previous_sequence
                        << "," << args.frame_id.sequence_number << ")";
 
-  if (ShouldIgnoreSequence(args.frame_id.sequence_number))
+  if (!first_received_main_sequence_ ||
+      first_received_main_sequence_ > args.frame_id.sequence_number) {
     return;
+  }
 
   if (last_no_main_damage_sequence_ == args.frame_id.sequence_number)
     return;
@@ -742,12 +741,6 @@ void FrameSequenceTracker::ReportMainFrameCausedNoDamage(
 
   if (begin_main_frame_data_.previous_sequence == args.frame_id.sequence_number)
     begin_main_frame_data_.previous_sequence = 0;
-
-#if DCHECK_IS_ON()
-  DCHECK_EQ(last_started_main_sequence_, args.frame_id.sequence_number)
-      << TRACKER_DCHECK_MSG;
-  last_started_main_sequence_ = 0;
-#endif
 }
 
 void FrameSequenceTracker::PauseFrameProduction() {

cc/metrics/frame_sequence_tracker.h

@@ -407,8 +407,6 @@ class CC_EXPORT FrameSequenceTracker {
   // when DCHECK is on.
   std::stringstream frame_sequence_trace_;
 
-  uint64_t last_started_main_sequence_ = 0;
-
   // If ReportBeginImplFrame is never called on a arg, then ReportBeginMainFrame
   // should ignore that arg.
   base::flat_set<viz::BeginFrameId> impl_frames_;

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -663,6 +663,25 @@ TEST_F(FrameSequenceTrackerTest, MultipleNoDamageNotificationsFromMain) {
   EXPECT_EQ(MainThroughput().frames_produced, 0u);
 }
 
+TEST_F(FrameSequenceTrackerTest, DelayedMainFrameNoDamage) {
+  const char sequence[] = "b(1)B(0,1)n(1)e(1)b(2)n(2)e(2)b(3)N(0,1)n(3)e(3)";
+  GenerateSequence(sequence);
+  EXPECT_EQ(ImplThroughput().frames_expected, 0u);
+  EXPECT_EQ(MainThroughput().frames_expected, 0u);
+  EXPECT_EQ(ImplThroughput().frames_produced, 0u);
+  EXPECT_EQ(MainThroughput().frames_produced, 0u);
+}
+
+TEST_F(FrameSequenceTrackerTest, DelayedMainFrameNoDamageFromOlderFrame) {
+  // Start a sequence, and receive a 'no damage' from an earlier frame.
+  const char second_sequence[] = "b(2)B(0,2)N(2,1)n(2)N(2,2)e(2)";
+  GenerateSequence(second_sequence);
+  EXPECT_EQ(ImplThroughput().frames_expected, 0u);
+  EXPECT_EQ(MainThroughput().frames_expected, 0u);
+  EXPECT_EQ(ImplThroughput().frames_produced, 0u);
+  EXPECT_EQ(MainThroughput().frames_produced, 0u);
+}
+
 TEST_F(FrameSequenceTrackerTest, StateResetDuringSequence) {
   const char sequence[] = "b(1)B(0,1)n(1)N(1,1)Re(1)b(2)n(2)e(2)";
   GenerateSequence(sequence);