[cc/metrics] Some debug checks in FrameSequenceTracker.

Explicitly notify the FrameSequenceTracker instances when handling a
frame terminates. This is used to validate that the expected sequence
of calls happen correctly, e.g. a new frame does not start before the
earlier frame has completely been processed.

BUG=1021963

Change-Id: I9465148c7478545723e2b3657fdd9b49cdc79d3d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902069
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723652}

cc/metrics/frame_sequence_tracker.cc

@@ -298,6 +298,13 @@ void FrameSequenceTrackerCollection::NotifySubmitFrame(
   }
 }
 
+void FrameSequenceTrackerCollection::NotifyFrameEnd(
+    const viz::BeginFrameArgs& args) {
+  for (auto& tracker : frame_trackers_) {
+    tracker.second->ReportFrameEnd(args);
+  }
+}
+
 void FrameSequenceTrackerCollection::NotifyFramePresented(
     uint32_t frame_token,
     const gfx::PresentationFeedback& feedback) {
@@ -395,12 +402,16 @@ void FrameSequenceTracker::ReportBeginImplFrame(
 
   if (ShouldIgnoreBeginFrameSource(args.source_id))
     return;
+#if DCHECK_IS_ON()
+  DCHECK(!is_inside_frame_) << TRACKER_DCHECK_MSG;
+  is_inside_frame_ = true;
+#endif
 
 #if DCHECK_IS_ON()
   if (args.type == viz::BeginFrameArgs::NORMAL)
     impl_frames_.insert(std::make_pair(args.source_id, args.sequence_number));
 #endif
-  TRACKER_TRACE_STREAM << 'b';
+  TRACKER_TRACE_STREAM << "b(" << args.sequence_number << ")";
   UpdateTrackedFrameData(&begin_impl_frame_data_, args.source_id,
                          args.sequence_number);
   impl_throughput().frames_expected +=
@@ -451,6 +462,9 @@ void FrameSequenceTracker::ReportSubmitFrame(
     return;
   }
 
+#if DCHECK_IS_ON()
+  DCHECK(is_inside_frame_) << TRACKER_DCHECK_MSG;
+#endif
   if (first_submitted_frame_ == 0)
     first_submitted_frame_ = frame_token;
   last_submitted_frame_ = frame_token;
@@ -474,6 +488,25 @@ void FrameSequenceTracker::ReportSubmitFrame(
   }
 }
 
+void FrameSequenceTracker::ReportFrameEnd(const viz::BeginFrameArgs& args) {
+#if DCHECK_IS_ON()
+  if (termination_status_ != TerminationStatus::kActive)
+    return;
+
+  if (ShouldIgnoreBeginFrameSource(args.source_id))
+    return;
+
+  if (ShouldIgnoreSequence(args.sequence_number)) {
+    is_inside_frame_ = false;
+    return;
+  }
+
+  TRACKER_TRACE_STREAM << "e(" << args.sequence_number << ")";
+  DCHECK(is_inside_frame_) << TRACKER_DCHECK_MSG;
+  is_inside_frame_ = false;
+#endif
+}
+
 void FrameSequenceTracker::ReportFramePresented(
     uint32_t frame_token,
     const gfx::PresentationFeedback& feedback) {

cc/metrics/frame_sequence_tracker.h

@@ -154,6 +154,7 @@ class CC_EXPORT FrameSequenceTrackerCollection {
                          bool has_missing_content,
                          const viz::BeginFrameAck& ack,
                          const viz::BeginFrameArgs& origin_args);
+  void NotifyFrameEnd(const viz::BeginFrameArgs& args);
 
   // Note that this notifies the trackers of the presentation-feedbacks, and
   // destroys any tracker that had been scheduled for destruction (using
@@ -238,6 +239,8 @@ class CC_EXPORT FrameSequenceTracker {
                          const viz::BeginFrameAck& ack,
                          const viz::BeginFrameArgs& origin_args);
 
+  void ReportFrameEnd(const viz::BeginFrameArgs& args);
+
   // Notifies the tracker of the presentation-feedback of a previously submitted
   // CompositorFrame with |frame_token|.
   void ReportFramePresented(uint32_t frame_token,
@@ -374,6 +377,8 @@ class CC_EXPORT FrameSequenceTracker {
   const base::TimeDelta time_delta_to_report_ = base::TimeDelta::FromSeconds(5);
 
 #if DCHECK_IS_ON()
+  bool is_inside_frame_ = false;
+
   // This stringstream represents a sequence of frame reporting activities on
   // the current tracker. Each letter can be one of the following:
   // {'B', 'N', 'b', 'n', 'S', 'P'}, where

cc/metrics/frame_sequence_tracker_unittest.cc

@@ -65,11 +65,13 @@ class FrameSequenceTrackerTest : public testing::Test {
       uint32_t frame_token = NextFrameToken();
       collection_.NotifySubmitFrame(frame_token, has_missing_content,
                                     viz::BeginFrameAck(args, true), args);
+      collection_.NotifyFrameEnd(args);
       return frame_token;
     } else {
       collection_.NotifyImplFrameCausedNoDamage(
           viz::BeginFrameAck(args, false));
       collection_.NotifyMainFrameCausedNoDamage(args);
+      collection_.NotifyFrameEnd(args);
     }
     return 0;
   }

cc/trees/layer_tree_host_impl.cc

@@ -2310,9 +2310,16 @@ bool LayerTreeHostImpl::DrawLayers(FrameData* frame) {
   }
 #endif
 
-  frame_trackers_.NotifySubmitFrame(
-      compositor_frame.metadata.frame_token, frame->has_missing_content,
-      frame->begin_frame_ack, frame->origin_begin_main_frame_args);
+  // In some cases (e.g. for android-webviews), the frame-submission happens
+  // outside of begin-impl frame pipeline. Avoid notifying the trackers in such
+  // cases.
+  if (impl_thread_phase_ == ImplThreadPhase::INSIDE_IMPL_FRAME) {
+    frame_trackers_.NotifySubmitFrame(
+        compositor_frame.metadata.frame_token, frame->has_missing_content,
+        frame->begin_frame_ack, frame->origin_begin_main_frame_args);
+  }
+
+
   if (!mutator_host_->NextFrameHasPendingRAF())
     frame_trackers_.StopSequence(FrameSequenceTrackerType::kRAF);
 
@@ -2725,6 +2732,7 @@ void LayerTreeHostImpl::DidFinishImplFrame() {
     frame_trackers_.NotifyMainFrameCausedNoDamage(
         current_begin_frame_tracker_.Current());
   }
+  frame_trackers_.NotifyFrameEnd(current_begin_frame_tracker_.Current());
   impl_thread_phase_ = ImplThreadPhase::IDLE;
   current_begin_frame_tracker_.Finish();
 }

third_party/blink/renderer/platform/graphics/video_frame_submitter.cc

@@ -207,6 +207,10 @@ void VideoFrameSubmitter::OnBeginFrame(
 
   frame_trackers_.NotifyBeginImplFrame(args);
 
+  base::ScopedClosureRunner end_frame(
+      base::BindOnce(&cc::FrameSequenceTrackerCollection::NotifyFrameEnd,
+                     base::Unretained(&frame_trackers_), args));
+
   // Don't call UpdateCurrentFrame() for MISSED BeginFrames. Also don't call it
   // after StopRendering() has been called (forbidden by API contract).
   viz::BeginFrameAck current_begin_frame_ack(args, false);