[cc/metrics] Defer resetting state when frames pause.

It is possible that a frame sequence can pause while in the middle
of processing a frame. When this happens, the state-reset should be
deferred until processing the frame ends, so that other notifications
for the frame (e.g. 'no damage') can be handled appropriately.

BUG=1035382, 1021963

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

cc/metrics/frame_sequence_tracker.cc

@@ -438,6 +438,12 @@ void FrameSequenceTracker::ReportBeginImplFrame(
     impl_frames_.insert(args.frame_id);
 #endif
 
+  if (reset_all_state_) {
+    begin_impl_frame_data_ = {};
+    begin_main_frame_data_ = {};
+    reset_all_state_ = false;
+  }
+
   TRACKER_TRACE_STREAM << "b(" << args.frame_id.sequence_number << ")";
   DCHECK(!frame_had_no_compositor_damage_) << TRACKER_DCHECK_MSG;
   DCHECK(!compositor_frame_submitted_) << TRACKER_DCHECK_MSG;
@@ -538,6 +544,21 @@ void FrameSequenceTracker::ReportFrameEnd(const viz::BeginFrameArgs& args) {
 
   TRACKER_TRACE_STREAM << "e(" << args.frame_id.sequence_number << ")";
 
+  bool should_ignore_sequence =
+      ShouldIgnoreSequence(args.frame_id.sequence_number);
+  if (reset_all_state_) {
+    begin_impl_frame_data_ = {};
+    begin_main_frame_data_ = {};
+    reset_all_state_ = false;
+  }
+
+  if (should_ignore_sequence) {
+#if DCHECK_IS_ON()
+    is_inside_frame_ = false;
+#endif
+    return;
+  }
+
   // It is possible that the compositor claims there was no damage from the
   // compositor, but before the frame ends, it submits a compositor frame (e.g.
   // with some damage from main). In such cases, the compositor is still
@@ -554,13 +575,6 @@ void FrameSequenceTracker::ReportFrameEnd(const viz::BeginFrameArgs& args) {
   frame_had_no_compositor_damage_ = false;
   compositor_frame_submitted_ = false;
 
-  if (ShouldIgnoreSequence(args.frame_id.sequence_number)) {
-#if DCHECK_IS_ON()
-    is_inside_frame_ = false;
-#endif
-    return;
-  }
-
 #if DCHECK_IS_ON()
   DCHECK(is_inside_frame_) << TRACKER_DCHECK_MSG;
   DCHECK_EQ(last_started_impl_sequence_, last_processed_impl_sequence_)
@@ -714,11 +728,12 @@ void FrameSequenceTracker::ReportMainFrameCausedNoDamage(
 }
 
 void FrameSequenceTracker::PauseFrameProduction() {
-  // Reset the states, so that the tracker ignores the vsyncs until the next
-  // received begin-frame.
-  begin_impl_frame_data_ = {0, 0, 0};
-  begin_main_frame_data_ = {0, 0, 0};
+  // The states need to be reset, so that the tracker ignores the vsyncs until
+  // the next received begin-frame. However, defer doing that until the frame
+  // ends (or a new frame starts), so that in case a frame is in-progress,
+  // subsequent notifications for that frame can be handled correctly.
   TRACKER_TRACE_STREAM << 'R';
+  reset_all_state_ = true;
 }
 
 void FrameSequenceTracker::UpdateTrackedFrameData(TrackedFrameData* frame_data,

cc/metrics/frame_sequence_tracker.h

@@ -382,6 +382,9 @@ class CC_EXPORT FrameSequenceTracker {
   // Keeps track of whether a CompositorFrame is submitted during the frame.
   bool compositor_frame_submitted_ = false;
 
+  // Keeps track of whether the frame-states should be reset.
+  bool reset_all_state_ = false;
+
   // A frame that is ignored at ReportSubmitFrame should never be presented.
   // TODO(xidachen): this should not be necessary. Some webview tests seem to
   // present a frame even if it is ignored by ReportSubmitFrame.

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -681,4 +681,19 @@ TEST_F(FrameSequenceTrackerTest, NoCompositorDamageSubmitFrame) {
   EXPECT_EQ(MainThroughput().frames_produced, 1u);
 }
 
+TEST_F(FrameSequenceTrackerTest, SequenceStateResetsDuringFrame) {
+  const char sequence[] = "b(1)Rn(1)e(1)";
+  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);
+
+  GenerateSequence("b(2)s(1)e(2)P(1)b(4)");
+  EXPECT_EQ(ImplThroughput().frames_expected, 3u);
+  EXPECT_EQ(MainThroughput().frames_expected, 0u);
+  EXPECT_EQ(ImplThroughput().frames_produced, 1u);
+  EXPECT_EQ(MainThroughput().frames_produced, 0u);
+}
+
 }  // namespace cc