[throughput] Correct main frame presentation logic

When a frame is presented on screen, the frame token may not exactly
match the awaiting-presented main frame token. In this case, our
current code won't count as main frame produced. But instead, we
should count this frame as main frame produced, as long as the
presented frame token is >= any main frame token that is waiting to
be presented.

Bug: 1055848
Change-Id: I5e0fb3e65a6581c8348e5e074cac6fc9a746356f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2075040
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748441}

cc/metrics/frame_sequence_tracker.cc

@@ -634,6 +634,7 @@ void FrameSequenceTracker::ReportSubmitFrame(
 
   TRACKER_TRACE_STREAM << "s(" << frame_token << ")";
   had_impl_frame_submitted_between_commits_ = true;
+
   const bool main_changes_after_sequence_started =
       first_received_main_sequence_ &&
       origin_args.frame_id.sequence_number >= first_received_main_sequence_;
@@ -774,18 +775,21 @@ void FrameSequenceTracker::ReportFramePresented(
       last_submitted_frame_ = 0;
   }
 
-  while (!main_frames_.empty() &&
-         !viz::FrameTokenGT(main_frames_.front(), frame_token)) {
-    if (was_presented && main_frames_.front() == frame_token) {
+  if (was_presented) {
+    // This presentation includes the visual update from all main frame tokens
+    // <= |frame_token|.
+    const unsigned size_before_erase = main_frames_.size();
+    while (!main_frames_.empty() &&
+           !viz::FrameTokenGT(main_frames_.front(), frame_token)) {
+      main_frames_.pop_front();
+    }
+    if (main_frames_.size() < size_before_erase) {
       DCHECK_LT(main_throughput().frames_produced,
                 main_throughput().frames_expected)
           << TRACKER_DCHECK_MSG;
       ++main_throughput().frames_produced;
     }
-    main_frames_.pop_front();
-  }
 
-  if (was_presented) {
     if (checkerboarding_.last_frame_had_checkerboarding) {
       DCHECK(!checkerboarding_.last_frame_timestamp.is_null())
           << TRACKER_DCHECK_MSG;

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -885,6 +885,36 @@ TEST_F(FrameSequenceTrackerTest, OffScreenMainDamage10) {
   EXPECT_EQ(MainThroughput().frames_expected, 0u);
 }
 
+// A presentation with a frame token that is > the main frame token submitted.
+TEST_F(FrameSequenceTrackerTest, MainThreadPresentWithNonMatchedToken) {
+  const char sequence[] = "b(1)B(0,1)E(1)s(1)S(1)e(1,0)b(2)s(2)S(1)e(2,1)P(2)";
+  GenerateSequence(sequence);
+  EXPECT_EQ(MainThroughput().frames_expected, 1u);
+  EXPECT_EQ(MainThroughput().frames_produced, 1u);
+}
+
+TEST_F(FrameSequenceTrackerTest, CoalescedMainThreadPresent) {
+  const char sequence[] =
+      "b(1)B(0,1)E(1)s(1)S(1)e(1,1)b(2)B(1,2)E(2)s(2)S(2)e(2,2)P(2)";
+  GenerateSequence(sequence);
+  EXPECT_EQ(MainThroughput().frames_expected, 2u);
+  EXPECT_EQ(MainThroughput().frames_produced, 1u);
+}
+
+TEST_F(FrameSequenceTrackerTest, MainThreadPresentWithNullTimeStamp) {
+  const char sequence[] = "b(1)B(0,1)E(1)s(1)S(1)e(1,1)";
+  GenerateSequence(sequence);
+  collection_.NotifyFramePresented(
+      1, {base::TimeTicks(), viz::BeginFrameArgs::DefaultInterval(), 0});
+  EXPECT_EQ(MainThroughput().frames_expected, 1u);
+  // No presentation, no main frame produced.
+  EXPECT_EQ(MainThroughput().frames_produced, 0u);
+  GenerateSequence("b(2)s(2)S(1)e(2,0)P(2)");
+  EXPECT_EQ(MainThroughput().frames_expected, 1u);
+  // The main frame update is caught up here.
+  EXPECT_EQ(MainThroughput().frames_produced, 1u);
+}
+
 TEST_F(FrameSequenceTrackerTest, TrackerTypeEncoding) {
   // The test begins with a kTouchScroll tracker
   EXPECT_EQ(NumberOfTrackers(), 1u);