Compositor_frame_reporter code cleanup

For each frame there is only one blink_breakdown and viz_breakdown, so
From all stage_data in the stage_history_ only one has viz_breakdown,
and only one has blink breakdown. This means that the breakdowns can be
part of the reporter instead of the stage_data.

Bug=none

Change-Id: I1448ecd539a7cb480fef5d0744efc76ff517b5dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1940386Reviewed-by: Mohsen Izadi <mohsen@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Behdad Bakhshinategh <behdadb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720278}

cc/metrics/compositor_frame_reporter.cc

@@ -210,17 +210,17 @@ void CompositorFrameReporter::OnAbortBeginMainFrame() {
 
 void CompositorFrameReporter::SetBlinkBreakdown(
     std::unique_ptr<BeginMainFrameMetrics> blink_breakdown) {
+  DCHECK(blink_breakdown_.paint.is_zero());
   if (blink_breakdown)
-    current_stage_.blink_breakdown = *blink_breakdown;
+    blink_breakdown_ = *blink_breakdown;
   else
-    current_stage_.blink_breakdown = BeginMainFrameMetrics();
+    blink_breakdown_ = BeginMainFrameMetrics();
 }
 
 void CompositorFrameReporter::SetVizBreakdown(
     const viz::FrameTimingDetails& viz_breakdown) {
-  DCHECK(current_stage_.viz_breakdown.received_compositor_frame_timestamp
-             .is_null());
-  current_stage_.viz_breakdown = viz_breakdown;
+  DCHECK(viz_breakdown_.received_compositor_frame_timestamp.is_null());
+  viz_breakdown_ = viz_breakdown;
 }
 
 void CompositorFrameReporter::TerminateReporter() {
@@ -277,31 +277,31 @@ void CompositorFrameReporter::ReportStageHistograms(bool missed_frame) const {
           : CompositorFrameReporter::MissedFrameReportTypes::kNonMissedFrame;
 
   for (const StageData& stage : stage_history_) {
-    ReportStageHistogramWithBreakdown(
-        report_type, FrameSequenceTrackerType::kMaxType, stage);
+    ReportStageHistogramWithBreakdown(report_type, stage);
 
     for (const auto& frame_sequence_tracker_type : *active_trackers_) {
       // Report stage breakdowns.
-      ReportStageHistogramWithBreakdown(report_type,
-                                        frame_sequence_tracker_type, stage);
+      ReportStageHistogramWithBreakdown(report_type, stage,
+                                        frame_sequence_tracker_type);
     }
   }
 }
 
 void CompositorFrameReporter::ReportStageHistogramWithBreakdown(
     CompositorFrameReporter::MissedFrameReportTypes report_type,
-    FrameSequenceTrackerType frame_sequence_tracker_type,
-    const CompositorFrameReporter::StageData& stage) const {
+    const CompositorFrameReporter::StageData& stage,
+    FrameSequenceTrackerType frame_sequence_tracker_type) const {
   base::TimeDelta stage_delta = stage.end_time - stage.start_time;
   ReportHistogram(report_type, frame_sequence_tracker_type,
                   static_cast<int>(stage.stage_type), stage_delta);
   switch (stage.stage_type) {
     case StageType::kSendBeginMainFrameToCommit: {
-      ReportBlinkBreakdown(report_type, frame_sequence_tracker_type, stage);
+      ReportBlinkBreakdowns(report_type, frame_sequence_tracker_type);
       break;
     }
     case StageType::kSubmitCompositorFrameToPresentationCompositorFrame: {
-      ReportVizBreakdown(report_type, frame_sequence_tracker_type, stage);
+      ReportVizBreakdowns(report_type, stage.start_time,
+                          frame_sequence_tracker_type);
       break;
     }
     default:
@@ -309,85 +309,61 @@ void CompositorFrameReporter::ReportStageHistogramWithBreakdown(
   }
 }
 
-void CompositorFrameReporter::ReportBlinkBreakdown(
+void CompositorFrameReporter::ReportBlinkBreakdowns(
     CompositorFrameReporter::MissedFrameReportTypes report_type,
-    FrameSequenceTrackerType frame_sequence_tracker_type,
-    const CompositorFrameReporter::StageData& stage) const {
-  ReportHistogram(report_type, frame_sequence_tracker_type,
-                  kBlinkBreakdownInitialIndex +
-                      static_cast<int>(BlinkBreakdown::kHandleInputEvents),
-                  stage.blink_breakdown.handle_input_events);
-  ReportHistogram(
-      report_type, frame_sequence_tracker_type,
-      kBlinkBreakdownInitialIndex + static_cast<int>(BlinkBreakdown::kAnimate),
-      stage.blink_breakdown.animate);
-  ReportHistogram(report_type, frame_sequence_tracker_type,
-                  kBlinkBreakdownInitialIndex +
-                      static_cast<int>(BlinkBreakdown::kStyleUpdate),
-                  stage.blink_breakdown.style_update);
-  ReportHistogram(report_type, frame_sequence_tracker_type,
-                  kBlinkBreakdownInitialIndex +
-                      static_cast<int>(BlinkBreakdown::kLayoutUpdate),
-                  stage.blink_breakdown.layout_update);
-  ReportHistogram(
-      report_type, frame_sequence_tracker_type,
-      kBlinkBreakdownInitialIndex + static_cast<int>(BlinkBreakdown::kPrepaint),
-      stage.blink_breakdown.prepaint);
-  ReportHistogram(report_type, frame_sequence_tracker_type,
-                  kBlinkBreakdownInitialIndex +
-                      static_cast<int>(BlinkBreakdown::kComposite),
-                  stage.blink_breakdown.composite);
-  ReportHistogram(
-      report_type, frame_sequence_tracker_type,
-      kBlinkBreakdownInitialIndex + static_cast<int>(BlinkBreakdown::kPaint),
-      stage.blink_breakdown.paint);
-  ReportHistogram(report_type, frame_sequence_tracker_type,
-                  kBlinkBreakdownInitialIndex +
-                      static_cast<int>(BlinkBreakdown::kScrollingCoordinator),
-                  stage.blink_breakdown.scrolling_coordinator);
-  ReportHistogram(report_type, frame_sequence_tracker_type,
-                  kBlinkBreakdownInitialIndex +
-                      static_cast<int>(BlinkBreakdown::kCompositeCommit),
-                  stage.blink_breakdown.composite_commit);
-  ReportHistogram(report_type, frame_sequence_tracker_type,
-                  kBlinkBreakdownInitialIndex +
-                      static_cast<int>(BlinkBreakdown::kUpdateLayers),
-                  stage.blink_breakdown.update_layers);
+    FrameSequenceTrackerType frame_sequence_tracker_type) const {
+  std::vector<std::pair<BlinkBreakdown, base::TimeDelta>> breakdowns = {
+      {BlinkBreakdown::kHandleInputEvents,
+       blink_breakdown_.handle_input_events},
+      {BlinkBreakdown::kAnimate, blink_breakdown_.animate},
+      {BlinkBreakdown::kStyleUpdate, blink_breakdown_.style_update},
+      {BlinkBreakdown::kLayoutUpdate, blink_breakdown_.layout_update},
+      {BlinkBreakdown::kPrepaint, blink_breakdown_.prepaint},
+      {BlinkBreakdown::kComposite, blink_breakdown_.composite},
+      {BlinkBreakdown::kPaint, blink_breakdown_.paint},
+      {BlinkBreakdown::kScrollingCoordinator,
+       blink_breakdown_.scrolling_coordinator},
+      {BlinkBreakdown::kCompositeCommit, blink_breakdown_.composite_commit},
+      {BlinkBreakdown::kUpdateLayers, blink_breakdown_.update_layers}};
+
+  for (const auto& pair : breakdowns) {
+    ReportHistogram(report_type, frame_sequence_tracker_type,
+                    kBlinkBreakdownInitialIndex + static_cast<int>(pair.first),
+                    pair.second);
+  }
 }
 
-void CompositorFrameReporter::ReportVizBreakdown(
+void CompositorFrameReporter::ReportVizBreakdowns(
     CompositorFrameReporter::MissedFrameReportTypes report_type,
-    FrameSequenceTrackerType frame_sequence_tracker_type,
-    const CompositorFrameReporter::StageData& stage) const {
+    base::TimeTicks start_time,
+    FrameSequenceTrackerType frame_sequence_tracker_type) const {
   // Check if viz_breakdown is set.
-  if (stage.viz_breakdown.received_compositor_frame_timestamp.is_null())
+  if (viz_breakdown_.received_compositor_frame_timestamp.is_null())
     return;
-
   base::TimeDelta submit_to_receive_compositor_frame_delta =
-      stage.viz_breakdown.received_compositor_frame_timestamp -
-      stage.start_time;
+      viz_breakdown_.received_compositor_frame_timestamp - start_time;
   ReportHistogram(
       report_type, frame_sequence_tracker_type,
       kVizBreakdownInitialIndex +
           static_cast<int>(VizBreakdown::kSubmitToReceiveCompositorFrame),
       submit_to_receive_compositor_frame_delta);
 
-  if (stage.viz_breakdown.draw_start_timestamp.is_null())
+  if (viz_breakdown_.draw_start_timestamp.is_null())
     return;
   base::TimeDelta received_compositor_frame_to_start_draw_delta =
-      stage.viz_breakdown.draw_start_timestamp -
-      stage.viz_breakdown.received_compositor_frame_timestamp;
+      viz_breakdown_.draw_start_timestamp -
+      viz_breakdown_.received_compositor_frame_timestamp;
   ReportHistogram(
       report_type, frame_sequence_tracker_type,
       kVizBreakdownInitialIndex +
           static_cast<int>(VizBreakdown::kReceivedCompositorFrameToStartDraw),
       received_compositor_frame_to_start_draw_delta);
 
-  if (stage.viz_breakdown.swap_timings.is_null())
+  if (viz_breakdown_.swap_timings.is_null())
     return;
   base::TimeDelta start_draw_to_swap_end_delta =
-      stage.viz_breakdown.swap_timings.swap_end -
-      stage.viz_breakdown.draw_start_timestamp;
+      viz_breakdown_.swap_timings.swap_end -
+      viz_breakdown_.draw_start_timestamp;
 
   ReportHistogram(report_type, frame_sequence_tracker_type,
                   kVizBreakdownInitialIndex +
@@ -395,7 +371,8 @@ void CompositorFrameReporter::ReportVizBreakdown(
                   start_draw_to_swap_end_delta);
 
   base::TimeDelta swap_end_to_presentation_compositor_frame_delta =
-      stage.end_time - stage.viz_breakdown.swap_timings.swap_end;
+      viz_breakdown_.presentation_feedback.timestamp -
+      viz_breakdown_.swap_timings.swap_end;
 
   ReportHistogram(
       report_type, frame_sequence_tracker_type,

cc/metrics/compositor_frame_reporter.h

@@ -136,13 +136,11 @@ class CC_EXPORT CompositorFrameReporter {
     return impl_frame_finish_time_;
   }
 
- protected:
+ private:
   struct StageData {
     StageType stage_type;
     base::TimeTicks start_time;
     base::TimeTicks end_time;
-    BeginMainFrameMetrics blink_breakdown;
-    viz::FrameTimingDetails viz_breakdown;
     StageData();
     StageData(StageType stage_type,
               base::TimeTicks start_time,
@@ -151,35 +149,35 @@ class CC_EXPORT CompositorFrameReporter {
     ~StageData();
   };
 
+  void TerminateReporter();
+  void EndCurrentStage(base::TimeTicks end_time);
+  void ReportStageHistograms(bool missed_frame) const;
+  void ReportStageHistogramWithBreakdown(
+      MissedFrameReportTypes report_type,
+      const StageData& stage,
+      FrameSequenceTrackerType frame_sequence_tracker_type =
+          FrameSequenceTrackerType::kMaxType) const;
+  void ReportBlinkBreakdowns(
+      MissedFrameReportTypes report_type,
+      FrameSequenceTrackerType frame_sequence_tracker_type) const;
+  void ReportVizBreakdowns(
+      MissedFrameReportTypes report_type,
+      const base::TimeTicks start_time,
+      FrameSequenceTrackerType frame_sequence_tracker_type) const;
+  void ReportHistogram(MissedFrameReportTypes report_type,
+                       FrameSequenceTrackerType intraction_type,
+                       const int stage_type_index,
+                       base::TimeDelta time_delta) const;
+
   StageData current_stage_;
+  BeginMainFrameMetrics blink_breakdown_;
+  viz::FrameTimingDetails viz_breakdown_;
 
   // Stage data is recorded here. On destruction these stages will be reported
   // to UMA if the termination status is |kPresentedFrame|. Reported data will
   // be divided based on the frame submission status.
   std::vector<StageData> stage_history_;
 
- private:
-  void TerminateReporter();
-  void EndCurrentStage(base::TimeTicks end_time);
-  void ReportStageHistograms(bool missed_frame) const;
-  void ReportStageHistogramWithBreakdown(
-      CompositorFrameReporter::MissedFrameReportTypes report_type,
-      FrameSequenceTrackerType frame_sequence_tracker_type,
-      const CompositorFrameReporter::StageData& stage) const;
-  void ReportBlinkBreakdown(
-      CompositorFrameReporter::MissedFrameReportTypes report_type,
-      FrameSequenceTrackerType frame_sequence_tracker_type,
-      const CompositorFrameReporter::StageData& stage) const;
-  void ReportVizBreakdown(
-      CompositorFrameReporter::MissedFrameReportTypes report_type,
-      FrameSequenceTrackerType frame_sequence_tracker_type,
-      const CompositorFrameReporter::StageData& stage) const;
-  void ReportHistogram(
-      CompositorFrameReporter::MissedFrameReportTypes report_type,
-      FrameSequenceTrackerType intraction_type,
-      const int stage_type_index,
-      base::TimeDelta time_delta) const;
-
   // Returns true if the stage duration is greater than |kAbnormalityPercentile|
   // of its RollingTimeDeltaHistory.
   base::TimeDelta GetStateNormalUpperLimit(const StageData& stage) const;