[cc/metric] Some sanity checks in the code.

Add some DCHECK()s in the code to validate some assumptions (e.g. make
sure the reporters are terminated predictably), and update tests
accordingly.

BUG=none

Change-Id: Id01bd571ad1d59b90c297f71097edcf3e6edc59b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2128448
Auto-Submit: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Behdad Bakhshinategh <behdadb@chromium.org>
Commit-Queue: Behdad Bakhshinategh <behdadb@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760220}

cc/metrics/compositor_frame_reporter.cc

@@ -329,8 +329,7 @@ void CompositorFrameReporter::MissedDeadlineFrame() {
 }
 
 void CompositorFrameReporter::TerminateReporter() {
-  if (frame_termination_status_ == FrameTerminationStatus::kUnknown)
-    TerminateFrame(FrameTerminationStatus::kUnknown, base::TimeTicks::Now());
+  DCHECK_NE(frame_termination_status_, FrameTerminationStatus::kUnknown);
   DCHECK_EQ(current_stage_.start_time, base::TimeTicks());
   bool report_compositor_latency = false;
   bool report_event_latency = false;

cc/metrics/compositor_frame_reporting_controller.cc

@@ -79,6 +79,7 @@ void CompositorFrameReportingController::WillBeginImplFrame(
 
 void CompositorFrameReportingController::WillBeginMainFrame(
     const viz::BeginFrameArgs& args) {
+  const auto now = Now();
   if (reporters_[PipelineStage::kBeginImplFrame]) {
     // We need to use .get() below because operator<< in std::unique_ptr is a
     // C++20 feature.
@@ -87,10 +88,22 @@ void CompositorFrameReportingController::WillBeginMainFrame(
     DCHECK_EQ(reporters_[PipelineStage::kBeginImplFrame]->frame_id_,
               args.frame_id);
     reporters_[PipelineStage::kBeginImplFrame]->StartStage(
-        StageType::kSendBeginMainFrameToCommit, Now());
+        StageType::kSendBeginMainFrameToCommit, now);
     AdvanceReporterStage(PipelineStage::kBeginImplFrame,
                          PipelineStage::kBeginMainFrame);
   } else {
+    auto& old_reporter = reporters_[PipelineStage::kBeginMainFrame];
+    if (old_reporter) {
+      DCHECK(old_reporter->did_abort_main_frame());
+      if (old_reporter->did_not_produce_frame()) {
+        old_reporter->TerminateFrame(
+            FrameTerminationStatus::kDidNotProduceFrame,
+            old_reporter->did_not_produce_frame_time());
+      } else {
+        old_reporter->TerminateFrame(
+            FrameTerminationStatus::kReplacedByNewReporter, now);
+      }
+    }
     // In this case we have already submitted the ImplFrame, but we received
     // beginMain frame before next BeginImplFrame (Not reached the ImplFrame
     // deadline yet). So will start a new reporter at BeginMainFrame.
@@ -99,7 +112,7 @@ void CompositorFrameReportingController::WillBeginMainFrame(
             &active_trackers_, args.frame_id,
             args.frame_time + (args.interval * 1.5),
             latency_ukm_reporter_.get(), should_report_metrics_);
-    reporter->StartStage(StageType::kSendBeginMainFrameToCommit, Now());
+    reporter->StartStage(StageType::kSendBeginMainFrameToCommit, now);
     reporters_[PipelineStage::kBeginMainFrame] = std::move(reporter);
   }
 }
@@ -356,7 +369,9 @@ bool CompositorFrameReportingController::CanSubmitImplFrame(
   if (!reporters_[PipelineStage::kBeginImplFrame])
     return false;
   auto& reporter = reporters_[PipelineStage::kBeginImplFrame];
-  return (reporter->frame_id_ == id && reporter->did_finish_impl_frame());
+  DCHECK_EQ(reporter->frame_id_, id);
+  DCHECK(reporter->did_finish_impl_frame());
+  return true;
 }
 
 bool CompositorFrameReportingController::CanSubmitMainFrame(
@@ -364,8 +379,8 @@ bool CompositorFrameReportingController::CanSubmitMainFrame(
   if (!reporters_[PipelineStage::kBeginMainFrame])
     return false;
   auto& reporter = reporters_[PipelineStage::kBeginMainFrame];
-  return (reporter->frame_id_ == id && reporter->did_finish_impl_frame() &&
-          reporter->did_abort_main_frame());
+  DCHECK(reporter->did_finish_impl_frame());
+  return reporter->frame_id_ == id && reporter->did_abort_main_frame();
 }
 
 std::unique_ptr<CompositorFrameReporter>

cc/metrics/compositor_frame_reporting_controller_unittest.cc

@@ -108,11 +108,13 @@ class CompositorFrameReportingControllerTest : public testing::Test {
     if (!reporting_controller_.reporters()
              [CompositorFrameReportingController::PipelineStage::kActivate])
       SimulateActivate();
-    CHECK(reporting_controller_.reporters()
-              [CompositorFrameReportingController::PipelineStage::kActivate]);
-    reporting_controller_.DidSubmitCompositorFrame(frame_token, current_id_,
-                                                   last_activated_id_,
-                                                   std::move(events_metrics));
+    auto& reporter =
+        reporting_controller_.reporters()
+            [CompositorFrameReportingController::PipelineStage::kActivate];
+    DCHECK(reporter);
+    reporting_controller_.DidSubmitCompositorFrame(
+        frame_token, reporter->frame_id_, last_activated_id_,
+        std::move(events_metrics));
   }
 
   void SimulatePresentCompositorFrame() {
@@ -510,9 +512,6 @@ TEST_F(CompositorFrameReportingControllerTest, MainFrameAborted2) {
   viz::BeginFrameId current_id_2(1, 2);
   viz::BeginFrameArgs args_2 = SimulateBeginFrameArgs(current_id_2);
 
-  viz::BeginFrameId current_id_3(1, 3);
-  viz::BeginFrameArgs args_3 = SimulateBeginFrameArgs(current_id_3);
-
   reporting_controller_.WillBeginImplFrame(args_1);
   reporting_controller_.OnFinishImplFrame(current_id_1);
   reporting_controller_.WillBeginMainFrame(args_1);
@@ -543,44 +542,6 @@ TEST_F(CompositorFrameReportingControllerTest, MainFrameAborted2) {
   histogram_tester.ExpectTotalCount(
       "CompositorLatency.SubmitCompositorFrameToPresentationCompositorFrame",
       2);
-  reporting_controller_.DidSubmitCompositorFrame(2, current_id_2, current_id_1,
-                                                 {});
-  reporting_controller_.DidPresentCompositorFrame(2, details);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.DroppedFrame.BeginImplFrameToSendBeginMainFrame", 0);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.BeginImplFrameToSendBeginMainFrame", 2);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.SendBeginMainFrameToCommit", 2);
-  histogram_tester.ExpectTotalCount("CompositorLatency.Commit", 1);
-  histogram_tester.ExpectTotalCount("CompositorLatency.EndCommitToActivation",
-                                    1);
-  histogram_tester.ExpectTotalCount("CompositorLatency.Activation", 1);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.EndActivateToSubmitCompositorFrame", 2);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.SubmitCompositorFrameToPresentationCompositorFrame",
-      2);
-  reporting_controller_.WillBeginImplFrame(args_3);
-  reporting_controller_.OnFinishImplFrame(current_id_3);
-  reporting_controller_.DidSubmitCompositorFrame(3, current_id_3, current_id_1,
-                                                 {});
-  reporting_controller_.DidPresentCompositorFrame(3, details);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.DroppedFrame.BeginImplFrameToSendBeginMainFrame", 0);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.BeginImplFrameToSendBeginMainFrame", 3);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.SendBeginMainFrameToCommit", 2);
-  histogram_tester.ExpectTotalCount("CompositorLatency.Commit", 1);
-  histogram_tester.ExpectTotalCount("CompositorLatency.EndCommitToActivation",
-                                    1);
-  histogram_tester.ExpectTotalCount("CompositorLatency.Activation", 1);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.EndActivateToSubmitCompositorFrame", 3);
-  histogram_tester.ExpectTotalCount(
-      "CompositorLatency.SubmitCompositorFrameToPresentationCompositorFrame",
-      3);
 }
 
 TEST_F(CompositorFrameReportingControllerTest, LongMainFrame) {
@@ -744,42 +705,33 @@ TEST_F(CompositorFrameReportingControllerTest, BlinkBreakdown) {
   blink_breakdown->composite_commit = base::TimeDelta::FromMicroseconds(2);
   blink_breakdown->update_layers = base::TimeDelta::FromMicroseconds(1);
 
-  SimulateActivate();
   SimulateCommit(std::move(blink_breakdown));
+  SimulateActivate();
   SimulatePresentCompositorFrame();
 
   histogram_tester.ExpectTotalCount(
       "CompositorLatency.SendBeginMainFrameToCommit", 1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.HandleInputEvents",
-      base::TimeDelta::FromMicroseconds(10).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.HandleInputEvents", 10, 1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.Animate",
-      base::TimeDelta::FromMicroseconds(9).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.Animate", 9, 1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.StyleUpdate",
-      base::TimeDelta::FromMicroseconds(8).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.StyleUpdate", 8, 1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.LayoutUpdate",
-      base::TimeDelta::FromMicroseconds(7).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.LayoutUpdate", 7, 1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.Prepaint",
-      base::TimeDelta::FromMicroseconds(6).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.Prepaint", 6, 1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.Composite",
-      base::TimeDelta::FromMicroseconds(5).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.Composite", 5, 1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.Paint",
-      base::TimeDelta::FromMicroseconds(4).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.Paint", 4, 1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.ScrollingCoordinator",
-      base::TimeDelta::FromMicroseconds(3).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.ScrollingCoordinator", 3,
+      1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.CompositeCommit",
-      base::TimeDelta::FromMicroseconds(2).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.CompositeCommit", 2, 1);
   histogram_tester.ExpectUniqueSample(
-      "CompositorLatency.SendBeginMainFrameToCommit.UpdateLayers",
-      base::TimeDelta::FromMicroseconds(1).InMilliseconds(), 1);
+      "CompositorLatency.SendBeginMainFrameToCommit.UpdateLayers", 1, 1);
   histogram_tester.ExpectTotalCount(
       "CompositorLatency.SendBeginMainFrameToCommit.BeginMainSentToStarted", 1);
 }