[throughput] Correctly compute expected frames for aggregated throughput

Right now, the |frames_expected| for the aggregated throughput is
simply the same as the |frames_expected| of the impl throughput.
However, there are some edge cases where this could be wrong, such
as the newly added test. In this case, we add jank in the script,
the jank is long enough such that exactly half of the expected
main frames gets presented on screen. In this case, half of the
impl frames reports no damage, and thus the |frames_expected| for
the aggregated throughput is only half of what it should be.

This CL fixes the problem and add a test case. It could causes
some changes to UMA for the:
Graphics.Smoothness.PercentDroppedFrame.*

TBR=sadrul@chromium.org

Bug: None
Change-Id: I3040a2db8a90e7d35ea2785204039b7c764e5435
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2234063
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779912}

cc/metrics/frame_sequence_metrics.cc

@@ -202,6 +202,8 @@ void FrameSequenceMetrics::ComputeAggregatedThroughput() {
 void FrameSequenceMetrics::ReportMetrics() {
   DCHECK_LE(impl_throughput_.frames_produced, impl_throughput_.frames_expected);
   DCHECK_LE(main_throughput_.frames_produced, main_throughput_.frames_expected);
+  DCHECK_LE(aggregated_throughput_.frames_produced,
+            aggregated_throughput_.frames_expected);
 
   // Terminates |trace_data_| for all types of FrameSequenceTracker.
   trace_data_.Terminate();
@@ -217,8 +219,6 @@ void FrameSequenceMetrics::ReportMetrics() {
     return;
   }
 
-  ComputeAggregatedThroughput();
-
   // Report the throughput metrics.
   base::Optional<int> impl_throughput_percent = ThroughputData::ReportHistogram(
       this, ThreadType::kCompositor,

cc/metrics/frame_sequence_metrics_unittest.cc

@@ -31,6 +31,7 @@ TEST(FrameSequenceMetricsTest, AggregatedThroughputClearedAfterReport) {
                              nullptr);
   first.impl_throughput().frames_expected = 200u;
   first.impl_throughput().frames_produced = 190u;
+  first.aggregated_throughput().frames_expected = 170u;
   first.aggregated_throughput().frames_produced = 150u;
 
   first.ReportMetrics();
@@ -49,11 +50,12 @@ TEST(FrameSequenceMetricsTest, ComputeUniversalThroughputAtDestruction) {
 
   metric->impl_throughput().frames_expected = 200u;
   metric->impl_throughput().frames_produced = 190u;
+  metric->aggregated_throughput().frames_expected = 170u;
   metric->aggregated_throughput().frames_produced = 150u;
 
   metric = nullptr;
   DCHECK(reporter.current_universal_throughput().has_value());
-  EXPECT_EQ(reporter.current_universal_throughput().value(), 75);
+  EXPECT_EQ(reporter.current_universal_throughput().value(), 88);
 }
 
 // Test that ThroughputUkmReporter::ReportThroughputUkm isn't called for the
@@ -67,6 +69,7 @@ TEST(FrameSequenceMetricsTest, UniversalNotReportUkmAtRenderer) {
 
   metric->impl_throughput().frames_expected = 200u;
   metric->impl_throughput().frames_produced = 190u;
+  metric->aggregated_throughput().frames_expected = 170u;
   metric->aggregated_throughput().frames_produced = 150u;
   metric->ReportMetrics();
 

cc/metrics/frame_sequence_tracker.cc

@@ -128,6 +128,8 @@ void FrameSequenceTracker::ReportBeginImplFrame(
                          args.frame_id.sequence_number);
   impl_throughput().frames_expected +=
       begin_impl_frame_data_.previous_sequence_delta;
+  aggregated_throughput().frames_expected +=
+      begin_impl_frame_data_.previous_sequence_delta;
 #if DCHECK_IS_ON()
   ++impl_throughput().frames_received;
 #endif
@@ -365,6 +367,8 @@ void FrameSequenceTracker::ReportFrameEnd(
       NOTREACHED() << TRACKER_DCHECK_MSG;
 #endif
     begin_impl_frame_data_.previous_sequence = 0;
+    if (!IsExpectingMainFrame())
+      --aggregated_throughput().frames_expected;
   }
   // last_submitted_frame_ == 0 means the last impl frame has been presented.
   if (termination_status_ == TerminationStatus::kScheduledForTermination &&
@@ -654,6 +658,13 @@ bool FrameSequenceTracker::ShouldIgnoreSequence(
   return sequence_number != begin_impl_frame_data_.previous_sequence;
 }
 
+bool FrameSequenceTracker::IsExpectingMainFrame() const {
+  bool last_main_not_processed =
+      begin_main_frame_data_.previous_sequence != 0 &&
+      begin_main_frame_data_.previous_sequence != last_processed_main_sequence_;
+  return !main_frames_.empty() || last_main_not_processed;
+}
+
 std::unique_ptr<base::trace_event::TracedValue>
 FrameSequenceMetrics::ThroughputData::ToTracedValue(
     const ThroughputData& impl,

cc/metrics/frame_sequence_tracker.h

@@ -160,6 +160,8 @@ class CC_EXPORT FrameSequenceTracker {
 
   bool ShouldIgnoreSequence(uint64_t sequence_number) const;
 
+  bool IsExpectingMainFrame() const;
+
   const int custom_sequence_id_;
 
   TerminationStatus termination_status_ = TerminationStatus::kActive;

cc/metrics/throughput_ukm_reporter.cc

@@ -69,8 +69,6 @@ void ThroughputUkmReporter::ComputeUniversalThroughput(
     FrameSequenceMetrics* metrics) {
   last_impl_percent_ = metrics->impl_throughput().DroppedFramePercent();
   last_main_percent_ = metrics->main_throughput().DroppedFramePercent();
-  metrics->aggregated_throughput().frames_expected =
-      metrics->impl_throughput().frames_expected;
   last_aggregated_percent_ =
       metrics->aggregated_throughput().DroppedFramePercent();
   current_universal_throughput_ = 100 - last_aggregated_percent_.value();

chrome/test/data/perf/throughput_test_cases/main-animations-half-presented.html

+<!doctype html>
+<!--
+Copyright 2019 The Chromium Authors. All rights reserved.
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+-->
+<html>
+<head>
+  <meta charset="utf-8">
+  <meta name="viewport" content="width=device-width">
+  <title>JS Bin</title>
+  <style>
+    #mainanim {
+      width: 200px;
+      height: 200px;
+      background-color: red;
+      animation: expand 2s infinite
+    }
+    @keyframes expand {
+      0% { width: 200px; }
+      100% { width: 500px; }
+    }
+  </style>
+</head>
+<body>
+<div id='mainanim'></div>
+
+</body>
+<script>
+var startTime = new Date();
+var frameNum = 0;
+function jank() {
+  frameNum = frameNum + 1;
+  // Make the jank long enough such that there is one main frame presented for
+  // every two main frames.
+  while ((new Date() - startTime) < 2 * 16.67 * frameNum + 20) {}
+  requestAnimationFrame(jank);
+}
+requestAnimationFrame(jank);
+</script>
+</html>

tools/perf/page_sets/rendering/throughput_test_cases.py

@@ -140,6 +140,13 @@ class OffScreenMainSixtyJank(ThroughputMetricStory):
          'main-animations-throughput.html?jank&offscreen#60')
 
 
+class MainAnimationsHalfPresented(ThroughputMetricStory):
+  BASE_NAME = 'main_animations_half_presented'
+  SUPPORTED_PLATFORMS = platforms.ALL_PLATFORMS
+  URL = ('file://../../../../chrome/test/data/perf/throughput_test_cases/'
+         'main-animations-half-presented.html')
+
+
 class ThroughputScrolling(ThroughputMetricStory):
   ABSTRACT_STORY = True
   URL = ('file://../../../../chrome/test/data/perf/throughput_test_cases/'