From 1c1af9fcab8ffaf3fdc3177130f035a99fa96049 Mon Sep 17 00:00:00 2001
From: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri, 20 Dec 2019 03:52:46 +0000
Subject: [PATCH] [cc/metrics] Some more debug checks.

Add DCHECK() to verify that every impl-frame started is also terminated
as expected: either a frame is submitted, or no-damage is explicitly
reported.

Also, add 'R' to the debug-log for a reset, and always report 'e' for
end, even for an invalid sequence-number.

BUG=1035382

Change-Id: I1aeab1453fa0c1b80a02b2f1795ab898be71a060
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1976912
Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Auto-Submit: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726688}
---
 cc/metrics/frame_sequence_tracker.cc | 14 +++++++++++---
 cc/metrics/frame_sequence_tracker.h  |  3 +++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/cc/metrics/frame_sequence_tracker.cc b/cc/metrics/frame_sequence_tracker.cc
index 9fec811e43dede..c359f3e9134639 100644
--- a/cc/metrics/frame_sequence_tracker.cc
+++ b/cc/metrics/frame_sequence_tracker.cc
@@ -432,6 +432,7 @@ void FrameSequenceTracker::ReportBeginImplFrame(
 #if DCHECK_IS_ON()
   DCHECK(!is_inside_frame_) << TRACKER_DCHECK_MSG;
   is_inside_frame_ = true;
+  last_started_impl_sequence_ = args.frame_id.sequence_number;
 
   if (args.type == viz::BeginFrameArgs::NORMAL)
     impl_frames_.insert(args.frame_id);
@@ -489,6 +490,7 @@ void FrameSequenceTracker::ReportSubmitFrame(
 
 #if DCHECK_IS_ON()
   DCHECK(is_inside_frame_) << TRACKER_DCHECK_MSG;
+  last_processed_impl_sequence_ = ack.frame_id.sequence_number;
 #endif
   if (first_submitted_frame_ == 0)
     first_submitted_frame_ = frame_token;
@@ -508,7 +510,7 @@ void FrameSequenceTracker::ReportSubmitFrame(
   if (!ShouldIgnoreBeginFrameSource(origin_args.frame_id.source_id) &&
       main_changes_after_sequence_started && main_changes_include_new_changes &&
       !main_change_had_no_damage) {
-    TRACKER_TRACE_STREAM << 'S';
+    TRACKER_TRACE_STREAM << "S(" << origin_args.frame_id.sequence_number << ")";
 
     last_submitted_main_sequence_ = origin_args.frame_id.sequence_number;
     main_frames_.push_back(frame_token);
@@ -529,13 +531,15 @@ void FrameSequenceTracker::ReportFrameEnd(const viz::BeginFrameArgs& args) {
   if (ShouldIgnoreBeginFrameSource(args.frame_id.source_id))
     return;
 
+  TRACKER_TRACE_STREAM << "e(" << args.frame_id.sequence_number << ")";
   if (ShouldIgnoreSequence(args.frame_id.sequence_number)) {
     is_inside_frame_ = false;
     return;
   }
 
-  TRACKER_TRACE_STREAM << "e(" << args.frame_id.sequence_number << ")";
   DCHECK(is_inside_frame_) << TRACKER_DCHECK_MSG;
+  DCHECK_EQ(last_started_impl_sequence_, last_processed_impl_sequence_)
+      << TRACKER_DCHECK_MSG;
   is_inside_frame_ = false;
 #endif
 }
@@ -639,6 +643,9 @@ void FrameSequenceTracker::ReportImplFrameCausedNoDamage(
   // dispatched for this frame-sequence. In such cases, ignore this call.
   if (ShouldIgnoreSequence(ack.frame_id.sequence_number))
     return;
+#if DCHECK_IS_ON()
+  last_processed_impl_sequence_ = ack.frame_id.sequence_number;
+#endif
 
   TRACKER_TRACE_STREAM << 'n';
   DCHECK_GT(impl_throughput().frames_expected, 0u) << TRACKER_DCHECK_MSG;
@@ -683,6 +690,7 @@ void FrameSequenceTracker::PauseFrameProduction() {
   // received begin-frame.
   begin_impl_frame_data_ = {0, 0, 0};
   begin_main_frame_data_ = {0, 0, 0};
+  TRACKER_TRACE_STREAM << 'R';
 }
 
 void FrameSequenceTracker::UpdateTrackedFrameData(TrackedFrameData* frame_data,
@@ -703,7 +711,7 @@ void FrameSequenceTracker::UpdateTrackedFrameData(TrackedFrameData* frame_data,
 bool FrameSequenceTracker::ShouldIgnoreBeginFrameSource(
     uint64_t source_id) const {
   if (begin_impl_frame_data_.previous_source == 0)
-    return false;
+    return source_id == viz::BeginFrameArgs::kManualSourceId;
   return source_id != begin_impl_frame_data_.previous_source;
 }
 
diff --git a/cc/metrics/frame_sequence_tracker.h b/cc/metrics/frame_sequence_tracker.h
index 7e7aa3e6eb1499..7ad95a00d1752c 100644
--- a/cc/metrics/frame_sequence_tracker.h
+++ b/cc/metrics/frame_sequence_tracker.h
@@ -394,6 +394,9 @@ class CC_EXPORT FrameSequenceTracker {
   // when DCHECK is on.
   std::stringstream frame_sequence_trace_;
 
+  uint64_t last_started_impl_sequence_ = 0;
+  uint64_t last_processed_impl_sequence_ = 0;
+
   // If ReportBeginImplFrame is never called on a arg, then ReportBeginMainFrame
   // should ignore that arg.
   base::flat_set<viz::BeginFrameId> impl_frames_;