[debug] Add DumpWithoutCrashing in FrameSequenceTracker

We are seeing OOM crashes from FrameSequenceTracker, and suspect that
a tracker may never be terminated.

This CL adds some debugging instrumentation to investigate what
happens after a tracker is scheduled to terminate.

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

cc/metrics/frame_sequence_tracker.cc

@@ -5,6 +5,8 @@
 #include "cc/metrics/frame_sequence_tracker.h"
 
 #include "base/bind.h"
+#include "base/debug/crash_logging.h"
+#include "base/debug/dump_without_crashing.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/trace_event/trace_event.h"
@@ -85,6 +87,7 @@ FrameSequenceTracker::FrameSequenceTracker(
 FrameSequenceTracker::~FrameSequenceTracker() = default;
 
 void FrameSequenceTracker::ScheduleTerminate() {
+  TRACKER_TRACE_STREAM << "t";
   // If the last frame has ended and there is no frame awaiting presentation,
   // then it is ready to terminate.
   if (!is_inside_frame_ && last_submitted_frame_ == 0)
@@ -230,15 +233,25 @@ void FrameSequenceTracker::ReportSubmitFrame(
   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
+  // Right now, we define a magical number |frames_to_terminate_tracker| = 10,
+  // which means that if this frame_token is more than 10 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;
+  const uint32_t frames_to_terminate_tracker = 10;
   if (termination_status_ == TerminationStatus::kScheduledForTermination &&
       last_submitted_frame_ != 0 &&
       viz::FrameTokenGT(frame_token,
                         last_submitted_frame_ + frames_to_terminate_tracker)) {
+#if DCHECK_IS_ON()
+    std::string full_str = frame_sequence_trace_.str();
+    std::string crash_str = full_str.size() < 255
+                                ? full_str
+                                : full_str.substr(full_str.size() - 255, 255);
+    static auto* crash_key = base::debug::AllocateCrashKeyString(
+        "tracker-sequence", base::debug::CrashKeySize::Size256);
+    base::debug::SetCrashKeyString(crash_key, crash_str);
+    base::debug::DumpWithoutCrashing();
+#endif
     termination_status_ = TerminationStatus::kReadyForTermination;
     return;
   }

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -1699,17 +1699,20 @@ TEST_F(FrameSequenceTrackerTest, TerminationWithNullPresentationTimeStamp) {
   EXPECT_EQ(NumberOfRemovalTrackers(), 0u);
 }
 
-// Test that a tracker is terminated after 3 submitted frames, remove this
+// Test that a tracker is terminated after 10 submitted frames, remove this
 // once crbug.com/1072482 is fixed.
-TEST_F(FrameSequenceTrackerTest, TerminationAfterThreeSubmissions1) {
+TEST_F(FrameSequenceTrackerTest, TerminationAfterTenSubmissions1) {
   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)");
+  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)b(6)s(6)e(6,0)b("
+      "7)s(7)e(7,0)b(8)s(8)e(8,0)b(9)s(9)e(9,0)b(10)s(10)e(10,0)b(11)s(11)e(11,"
+      "0)b(12)s(12)e(12,0)");
   EXPECT_EQ(NumberOfRemovalTrackers(), 0u);
 }
 
-TEST_F(FrameSequenceTrackerTest, TerminationAfterThreeSubmissions2) {
+TEST_F(FrameSequenceTrackerTest, TerminationAfterTenSubmissions2) {
   GenerateSequence("b(1)");
   auto args = CreateBeginFrameArgs(1u, 1u);
   // Ack to an impl frame that doesn't exist in this tracker.
@@ -1718,16 +1721,19 @@ TEST_F(FrameSequenceTrackerTest, TerminationAfterThreeSubmissions2) {
   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)");
+  GenerateSequence(
+      "b(2)s(1)e(2,0)b(3)s(2)e(3,0)b(4)s(3)e(4,0)b(5)s(4)e(5,0)b(6)s(5)e(6,0)b("
+      "7)s(6)e(7,0)b(8)s(7)e(8,0)b(9)s(8)e(9,0)b(10)s(9)e(10,0)b(11)s(10)e(11,"
+      "0)");
   EXPECT_EQ(NumberOfRemovalTrackers(), 0u);
 }
 
-TEST_F(FrameSequenceTrackerTest, TerminationAfterThreeSubmissions3) {
+TEST_F(FrameSequenceTrackerTest, TerminationAfterTenSubmissions3) {
   GenerateSequence(
       "b(1)s(1)e(1,0)P(1)b(2)s(2)e(2,0)P(2)b(3)s(3)e(3,0)P(3)b(4)");
   collection_.StopSequence(FrameSequenceTrackerType::kTouchScroll);
   EXPECT_EQ(NumberOfRemovalTrackers(), 1u);
-  GenerateSequence("s(4)");
+  GenerateSequence("s(11)");
   EXPECT_EQ(NumberOfRemovalTrackers(), 1u);
 }