Commit 3144187e authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

[throughput] Fix a DCHECK crash

Currently we hit a DCHECK in report histogram. The problem is that
for certain type of throughput data (such as CompositorAnimation +
main thread), we do not report it. In this case, we do not reset the
throughput data and it kept accumulated and eventually the number
of expected frames reaches a threshold and caused crash.

This CL fixes the crash and added test.

Bug: 1052848
Change-Id: Ib150872f950f0901a6bce5f10e683e08644c5809
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2061398Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742947}
parent 8fab0b6c
......@@ -120,6 +120,12 @@ TEST(FrameSequenceMetricsTest, IrrelevantMetricsNotReported) {
"CompositorAnimation",
0u);
// Not reported, but the data should be reset.
EXPECT_EQ(first.impl_throughput().frames_expected, 0u);
EXPECT_EQ(first.impl_throughput().frames_produced, 0u);
EXPECT_EQ(first.main_throughput().frames_expected, 0u);
EXPECT_EQ(first.main_throughput().frames_produced, 0u);
FrameSequenceMetrics second(FrameSequenceTrackerType::kRAF, nullptr);
second.impl_throughput().frames_expected = 120;
second.impl_throughput().frames_produced = 80;
......
......@@ -232,10 +232,10 @@ void FrameSequenceMetrics::ReportMetrics() {
frames_checkerboarded_ = 0;
}
// Reset the metrics that have already been reported.
if (impl_throughput_percent.has_value())
// Reset the metrics that reach reporting threshold.
if (impl_throughput_.frames_expected >= kMinFramesForThroughputMetric)
impl_throughput_ = {};
if (main_throughput_percent.has_value())
if (main_throughput_.frames_expected >= kMinFramesForThroughputMetric)
main_throughput_ = {};
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment