[throughput] Fix a OOM crash in FrameSequenceTracker

There are OOM crashes in FrameSequenceTracker::ReportSubmitFrame,
where the ignored_frame_token_ is getting too big.

This could only happen if a tracker is somehow never terminated,
and thus not getting removed.

This CL is a temp fix that if a tracker is scheduled for termination,
and still receive 3 frame submissions, then we terminate this
tracker right away.

Following this, we will add debug strings to find out why is it
not terminated.

Bug: 1070209
Change-Id: I5a28cb0f52abf44e76211b87f8d8a6b3c1657fdd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2154498Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761084}

cc/metrics/frame_sequence_tracker.cc

@@ -219,6 +219,20 @@ void FrameSequenceTracker::ReportSubmitFrame(
     const viz::BeginFrameAck& ack,
     const viz::BeginFrameArgs& origin_args) {
   DCHECK_NE(termination_status_, TerminationStatus::kReadyForTermination);
+
+  // TODO(crbug.com/1072482): find a proper way to terminate a tracker.
+  // Right now, we define a magical number |frames_to_terminate_tracker| = 3,
+  // which means that if this frame_token is more than 3 frames compared with
+  // the last submitted frame, then we assume that the last submitted frame is
+  // not going to be presented, and thus terminate this tracker.
+  const uint32_t frames_to_terminate_tracker = 3;
+  if (termination_status_ == TerminationStatus::kScheduledForTermination &&
+      viz::FrameTokenGT(frame_token,
+                        last_submitted_frame_ + frames_to_terminate_tracker)) {
+    termination_status_ = TerminationStatus::kReadyForTermination;
+    return;
+  }
+
   if (ShouldIgnoreBeginFrameSource(ack.frame_id.source_id) ||
       ShouldIgnoreSequence(ack.frame_id.sequence_number)) {
     ignored_frame_tokens_.insert(frame_token);

cc/metrics/frame_sequence_tracker_collection.cc

@@ -165,6 +165,10 @@ void FrameSequenceTrackerCollection::NotifySubmitFrame(
     tracker->ReportSubmitFrame(frame_token, has_missing_content, ack,
                                origin_args);
   }
+
+  // TODO(crbug.com/1072482): find a proper way to terminate a tracker. Please
+  // refer to details in FrameSequenceTracker::ReportSubmitFrame
+  DestroyTrackers();
 }
 
 void FrameSequenceTrackerCollection::NotifyFrameEnd(

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -1671,6 +1671,21 @@ TEST_F(FrameSequenceTrackerTest, IgnoredFrameTokensRemovedAtPresentation1) {
   EXPECT_EQ(IgnoredFrameTokens().size(), 0u);
 }
 
+// Test the case where the frame tokens wraps around the 32-bit max value.
+TEST_F(FrameSequenceTrackerTest, IgnoredFrameTokensRemovedAtPresentation2) {
+  GenerateSequence("b(5)");
+  auto args = CreateBeginFrameArgs(1u, 1u);
+  // Ack to an impl frame that doesn't exist in this tracker.
+  collection_.NotifySubmitFrame(UINT32_MAX, /*has_missing_content=*/false,
+                                viz::BeginFrameAck(args, true), args);
+  EXPECT_EQ(IgnoredFrameTokens().size(), 1u);
+
+  args = CreateBeginFrameArgs(1u, 5u);
+  collection_.NotifySubmitFrame(1, false, viz::BeginFrameAck(args, true), args);
+  GenerateSequence("e(5,0)P(1)");
+  EXPECT_TRUE(IgnoredFrameTokens().empty());
+}
+
 TEST_F(FrameSequenceTrackerTest, TerminationWithNullPresentationTimeStamp) {
   GenerateSequence("b(1)s(1)");
   collection_.StopSequence(FrameSequenceTrackerType::kTouchScroll);
@@ -1684,19 +1699,27 @@ TEST_F(FrameSequenceTrackerTest, TerminationWithNullPresentationTimeStamp) {
   EXPECT_EQ(NumberOfRemovalTrackers(), 0u);
 }
 
-// Test the case where the frame tokens wraps around the 32-bit max value.
-TEST_F(FrameSequenceTrackerTest, IgnoredFrameTokensRemovedAtPresentation2) {
-  GenerateSequence("b(5)");
+// Test that a tracker is terminated after 3 submitted frames, remove this
+// once crbug.com/1072482 is fixed.
+TEST_F(FrameSequenceTrackerTest, TerminationAfterThreeSubmissions1) {
+  GenerateSequence("b(1)s(1)e(1,0)");
+  collection_.StopSequence(FrameSequenceTrackerType::kTouchScroll);
+  EXPECT_EQ(NumberOfRemovalTrackers(), 1u);
+  GenerateSequence("b(2)s(2)e(2,0)b(3)s(3)e(3,0)b(4)s(4)e(4,0)b(5)s(5)e(5,0)");
+  EXPECT_EQ(NumberOfRemovalTrackers(), 0u);
+}
+
+TEST_F(FrameSequenceTrackerTest, TerminationAfterThreeSubmissions2) {
+  GenerateSequence("b(1)");
   auto args = CreateBeginFrameArgs(1u, 1u);
   // Ack to an impl frame that doesn't exist in this tracker.
   collection_.NotifySubmitFrame(UINT32_MAX, /*has_missing_content=*/false,
                                 viz::BeginFrameAck(args, true), args);
-  EXPECT_EQ(IgnoredFrameTokens().size(), 1u);
-
-  args = CreateBeginFrameArgs(1u, 5u);
-  collection_.NotifySubmitFrame(1, false, viz::BeginFrameAck(args, true), args);
-  GenerateSequence("e(5,0)P(1)");
-  EXPECT_TRUE(IgnoredFrameTokens().empty());
+  GenerateSequence("e(1,0)");
+  collection_.StopSequence(FrameSequenceTrackerType::kTouchScroll);
+  EXPECT_EQ(NumberOfRemovalTrackers(), 1u);
+  GenerateSequence("b(2)s(1)e(2,0)b(3)s(2)e(3,0)b(4)s(3)e(4,0)");
+  EXPECT_EQ(NumberOfRemovalTrackers(), 0u);
 }
 
 TEST_F(FrameSequenceTrackerTest, OffScreenMainDamage1) {