0

Revert "[cc/metrics] Introduce FrameSorter."

This reverts commit 8e01fd9782.

Reason for revert: crashing in WebView tests https://crbug.com/1145636

Original change's description:
> [cc/metrics] Introduce FrameSorter.
>
> The begin-frames do not always terminate in the same order as they
> start. For example, a frame that does not have any updates can terminate
> earlier than a previous frame that had an update and is awaiting
> presentation. This can make it tricky to measure dropped-frames in a
> sliding window. To make this easier, introduce a FrameSorter, that makes
> sure the sliding-window can process the frames in order, regardless of
> when they are terminated.
>
> BUG=1138552
>
> Change-Id: I2ae0e2413d64d0242aa3ba57f9d1b3d2c1f4354e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441202
> Reviewed-by: Jonathan Ross <jonross@chromium.org>
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Commit-Queue: Behdad Bakhshinategh <behdadb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#822737}

TBR=sadrul@chromium.org,jonross@chromium.org,behdadb@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1138552
Bug: 1145636
Change-Id: I332b00355b258585f5447d0c2b4c0fea8b0a1e56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2519717
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824096}
This commit is contained in:
Nate Fischer
2020-11-04 19:23:39 +00:00
committed by Commit Bot
parent e4cd5bf4db
commit 4d2c797493
17 changed files with 50 additions and 454 deletions

@ -177,8 +177,6 @@ cc_component("cc") {
"metrics/frame_sequence_tracker.h",
"metrics/frame_sequence_tracker_collection.cc",
"metrics/frame_sequence_tracker_collection.h",
"metrics/frame_sorter.cc",
"metrics/frame_sorter.h",
"metrics/jank_metrics.cc",
"metrics/jank_metrics.h",
"metrics/latency_ukm_reporter.cc",
@ -691,7 +689,6 @@ cc_test("cc_unittests") {
"metrics/events_metrics_manager_unittest.cc",
"metrics/frame_sequence_metrics_unittest.cc",
"metrics/frame_sequence_tracker_unittest.cc",
"metrics/frame_sorter_unittest.cc",
"metrics/jank_metrics_unittest.cc",
"metrics/total_frame_counter_unittest.cc",
"metrics/video_playback_roughness_reporter_unittest.cc",

@ -292,17 +292,13 @@ CompositorFrameReporter::CompositorFrameReporter(
LatencyUkmReporter* latency_ukm_reporter,
bool should_report_metrics,
SmoothThread smooth_thread,
int layer_tree_host_id,
DroppedFrameCounter* dropped_frame_counter)
int layer_tree_host_id)
: should_report_metrics_(should_report_metrics),
args_(args),
active_trackers_(active_trackers),
latency_ukm_reporter_(latency_ukm_reporter),
dropped_frame_counter_(dropped_frame_counter),
smooth_thread_(smooth_thread),
layer_tree_host_id_(layer_tree_host_id) {
dropped_frame_counter_->OnBeginFrame(args);
}
layer_tree_host_id_(layer_tree_host_id) {}
std::unique_ptr<CompositorFrameReporter>
CompositorFrameReporter::CopyReporterAtBeginImplStage() {
@ -314,7 +310,7 @@ CompositorFrameReporter::CopyReporterAtBeginImplStage() {
}
auto new_reporter = std::make_unique<CompositorFrameReporter>(
active_trackers_, args_, latency_ukm_reporter_, should_report_metrics_,
smooth_thread_, layer_tree_host_id_, dropped_frame_counter_);
smooth_thread_, layer_tree_host_id_);
new_reporter->did_finish_impl_frame_ = did_finish_impl_frame_;
new_reporter->impl_frame_finish_time_ = impl_frame_finish_time_;
new_reporter->main_frame_abort_time_ = main_frame_abort_time_;
@ -322,6 +318,7 @@ CompositorFrameReporter::CopyReporterAtBeginImplStage() {
StageType::kBeginImplFrameToSendBeginMainFrame;
new_reporter->current_stage_.start_time = stage_history_.front().start_time;
new_reporter->set_tick_clock(tick_clock_);
new_reporter->SetDroppedFrameCounter(dropped_frame_counter_);
new_reporter->cloned_from_ = weak_factory_.GetWeakPtr();
// TODO(https://crbug.com/1127872) Check |cloned_to_| is null before replacing
@ -497,8 +494,8 @@ void CompositorFrameReporter::TerminateReporter() {
dropped_frame_counter_->AddGoodFrame();
}
dropped_frame_counter_->OnEndFrame(args_,
IsDroppedFrameAffectingSmoothness());
if (IsDroppedFrameAffectingSmoothness())
dropped_frame_counter_->AddDroppedFrameAffectingSmoothness();
}
}

@ -148,8 +148,7 @@ class CC_EXPORT CompositorFrameReporter {
LatencyUkmReporter* latency_ukm_reporter,
bool should_report_metrics,
SmoothThread smooth_thread,
int layer_tree_host_id,
DroppedFrameCounter* dropped_frame_counter);
int layer_tree_host_id);
~CompositorFrameReporter();
CompositorFrameReporter(const CompositorFrameReporter& reporter) = delete;
@ -200,6 +199,10 @@ class CC_EXPORT CompositorFrameReporter {
tick_clock_ = tick_clock;
}
void SetDroppedFrameCounter(DroppedFrameCounter* counter) {
dropped_frame_counter_ = counter;
}
bool has_partial_update() const { return has_partial_update_; }
void set_has_partial_update(bool has_partial_update) {
has_partial_update_ = has_partial_update;

@ -12,9 +12,7 @@
#include "base/test/metrics/histogram_tester.h"
#include "base/test/simple_test_tick_clock.h"
#include "cc/metrics/compositor_frame_reporting_controller.h"
#include "cc/metrics/dropped_frame_counter.h"
#include "cc/metrics/event_metrics.h"
#include "cc/metrics/total_frame_counter.h"
#include "components/viz/common/frame_timing_details.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
@ -36,11 +34,9 @@ class CompositorFrameReporterTest : public testing::Test {
nullptr,
/*should_report_metrics=*/true,
CompositorFrameReporter::SmoothThread::kSmoothBoth,
/*layer_tree_host_id=*/1,
&dropped_frame_counter_)) {
/*layer_tree_host_id=*/1)) {
pipeline_reporter_->set_tick_clock(&test_tick_clock_);
AdvanceNowByMs(1);
dropped_frame_counter_.set_total_counter(&total_frame_counter_);
}
protected:
@ -84,8 +80,6 @@ class CompositorFrameReporterTest : public testing::Test {
// and destroyed after that.
base::SimpleTestTickClock test_tick_clock_;
DroppedFrameCounter dropped_frame_counter_;
TotalFrameCounter total_frame_counter_;
std::unique_ptr<CompositorFrameReporter> pipeline_reporter_;
};

@ -84,11 +84,11 @@ void CompositorFrameReportingController::WillBeginImplFrame(
}
auto reporter = std::make_unique<CompositorFrameReporter>(
active_trackers_, args, latency_ukm_reporter_.get(),
should_report_metrics_, GetSmoothThread(), layer_tree_host_id_,
dropped_frame_counter_);
should_report_metrics_, GetSmoothThread(), layer_tree_host_id_);
reporter->set_tick_clock(tick_clock_);
reporter->StartStage(StageType::kBeginImplFrameToSendBeginMainFrame,
begin_time);
reporter->SetDroppedFrameCounter(dropped_frame_counter_);
reporters_[PipelineStage::kBeginImplFrame] = std::move(reporter);
}
@ -111,10 +111,10 @@ void CompositorFrameReportingController::WillBeginMainFrame(
// deadline yet). So will start a new reporter at BeginMainFrame.
auto reporter = std::make_unique<CompositorFrameReporter>(
active_trackers_, args, latency_ukm_reporter_.get(),
should_report_metrics_, GetSmoothThread(), layer_tree_host_id_,
dropped_frame_counter_);
should_report_metrics_, GetSmoothThread(), layer_tree_host_id_);
reporter->set_tick_clock(tick_clock_);
reporter->StartStage(StageType::kSendBeginMainFrameToCommit, Now());
reporter->SetDroppedFrameCounter(dropped_frame_counter_);
reporters_[PipelineStage::kBeginMainFrame] = std::move(reporter);
}
}
@ -495,11 +495,11 @@ void CompositorFrameReportingController::CreateReportersForDroppedFrames(
viz::BeginFrameArgs::NORMAL);
auto reporter = std::make_unique<CompositorFrameReporter>(
active_trackers_, args, latency_ukm_reporter_.get(),
should_report_metrics_, GetSmoothThread(), layer_tree_host_id_,
dropped_frame_counter_);
should_report_metrics_, GetSmoothThread(), layer_tree_host_id_);
reporter->set_tick_clock(tick_clock_);
reporter->StartStage(StageType::kBeginImplFrameToSendBeginMainFrame,
timestamp);
reporter->SetDroppedFrameCounter(dropped_frame_counter_);
reporter->TerminateFrame(FrameTerminationStatus::kDidNotPresentFrame,
args.deadline);
}

@ -13,7 +13,6 @@
#include "base/test/simple_test_tick_clock.h"
#include "cc/metrics/dropped_frame_counter.h"
#include "cc/metrics/event_metrics.h"
#include "cc/metrics/total_frame_counter.h"
#include "components/viz/common/frame_timing_details.h"
#include "components/viz/common/quads/compositor_frame_metadata.h"
#include "testing/gmock/include/gmock/gmock.h"
@ -62,8 +61,6 @@ class CompositorFrameReportingControllerTest : public testing::Test {
test_tick_clock_.SetNowTicks(base::TimeTicks::Now());
reporting_controller_.set_tick_clock(&test_tick_clock_);
args_ = SimulateBeginFrameArgs(current_id_);
reporting_controller_.SetDroppedFrameCounter(&dropped_counter);
dropped_counter.set_total_counter(&total_frame_counter_);
}
// The following functions simulate the actions that would
@ -203,8 +200,6 @@ class CompositorFrameReportingControllerTest : public testing::Test {
base::TimeTicks end_activation_time_;
base::TimeTicks submit_time_;
viz::FrameTokenGenerator next_token_;
DroppedFrameCounter dropped_counter;
TotalFrameCounter total_frame_counter_;
};
TEST_F(CompositorFrameReportingControllerTest, ActiveReporterCounts) {
@ -1313,6 +1308,9 @@ TEST_F(CompositorFrameReportingControllerTest,
TEST_F(CompositorFrameReportingControllerTest,
NewMainUpdateIsNotPartialUpdate) {
DroppedFrameCounter dropped_counter;
reporting_controller_.SetDroppedFrameCounter(&dropped_counter);
SimulateBeginMainFrame();
reporting_controller_.OnFinishImplFrame(current_id_);
reporting_controller_.DidSubmitCompositorFrame(1u, current_id_, {}, {});
@ -1343,6 +1341,9 @@ TEST_F(CompositorFrameReportingControllerTest,
TEST_F(CompositorFrameReportingControllerTest,
SkippedFramesFromDisplayCompositorAreDropped) {
DroppedFrameCounter dropped_counter;
reporting_controller_.SetDroppedFrameCounter(&dropped_counter);
// Submit and present two compositor frames.
SimulatePresentCompositorFrame();
EXPECT_EQ(1u, dropped_counter.total_frames());

@ -48,7 +48,6 @@ class CompositorTimingHistoryTest : public testing::Test {
reporting_controller_.get()) {
AdvanceNowBy(base::TimeDelta::FromMilliseconds(1));
timing_history_.SetRecordingEnabled(true);
reporting_controller_->SetDroppedFrameCounter(&dropped_counter);
}
void AdvanceNowBy(base::TimeDelta delta) { now_ += delta; }
@ -61,7 +60,6 @@ class CompositorTimingHistoryTest : public testing::Test {
TestCompositorTimingHistory timing_history_;
base::TimeTicks now_;
uint64_t sequence_number = 0;
DroppedFrameCounter dropped_counter;
viz::BeginFrameArgs GetFakeBeginFrameArg(bool on_critical_path = true) {
viz::BeginFrameArgs args = viz::BeginFrameArgs();

@ -4,18 +4,18 @@
#include "cc/metrics/dropped_frame_counter.h"
#include "base/bind.h"
#include <stddef.h>
#include <limits>
#include "base/memory/ptr_util.h"
#include "base/trace_event/trace_event.h"
#include "cc/metrics/frame_sorter.h"
#include "cc/metrics/total_frame_counter.h"
#include "cc/metrics/ukm_smoothness_data.h"
namespace cc {
DroppedFrameCounter::DroppedFrameCounter()
: frame_sorter_(base::BindRepeating(&DroppedFrameCounter::NotifyFrameResult,
base::Unretained(this))) {}
DroppedFrameCounter::~DroppedFrameCounter() = default;
DroppedFrameCounter::DroppedFrameCounter() = default;
uint32_t DroppedFrameCounter::GetAverageThroughput() const {
size_t good_frames = 0;
@ -44,22 +44,10 @@ void DroppedFrameCounter::AddDroppedFrame() {
++total_dropped_;
}
void DroppedFrameCounter::ResetFrameSorter() {
frame_sorter_.Reset();
}
void DroppedFrameCounter::OnBeginFrame(const viz::BeginFrameArgs& args) {
frame_sorter_.AddNewFrame(args);
}
void DroppedFrameCounter::OnEndFrame(const viz::BeginFrameArgs& args,
bool is_dropped) {
if (is_dropped) {
if (fcp_received_)
++total_smoothness_dropped_;
ReportFrames();
}
frame_sorter_.AddFrameResult(args, is_dropped);
void DroppedFrameCounter::AddDroppedFrameAffectingSmoothness() {
if (fcp_received_)
++total_smoothness_dropped_;
ReportFrames();
}
void DroppedFrameCounter::ReportFrames() {
@ -93,12 +81,6 @@ void DroppedFrameCounter::Reset() {
total_smoothness_dropped_ = 0;
fcp_received_ = false;
ring_buffer_.Clear();
frame_sorter_.Reset();
}
void DroppedFrameCounter::NotifyFrameResult(const viz::BeginFrameArgs& args,
bool is_dropped) {
// TODO(crbug.com/1115141) The implementation of smoothness metrics.
}
void DroppedFrameCounter::OnFcpReceived() {

@ -7,11 +7,13 @@
#include <stddef.h>
#include <memory>
#include "base/containers/ring_buffer.h"
#include "cc/cc_export.h"
#include "cc/metrics/frame_sorter.h"
namespace cc {
class TotalFrameCounter;
struct UkmSmoothnessDataShared;
@ -26,7 +28,6 @@ class CC_EXPORT DroppedFrameCounter {
};
DroppedFrameCounter();
~DroppedFrameCounter();
DroppedFrameCounter(const DroppedFrameCounter&) = delete;
DroppedFrameCounter& operator=(const DroppedFrameCounter&) = delete;
@ -46,27 +47,20 @@ class CC_EXPORT DroppedFrameCounter {
void AddGoodFrame();
void AddPartialFrame();
void AddDroppedFrame();
void AddDroppedFrameAffectingSmoothness();
void ReportFrames();
void OnBeginFrame(const viz::BeginFrameArgs& args);
void OnEndFrame(const viz::BeginFrameArgs& args, bool is_dropped);
void SetUkmSmoothnessDestination(UkmSmoothnessDataShared* smoothness_data);
void OnFcpReceived();
// Reset is used on navigation, which resets frame statistics as well as
// frame sorter.
void Reset();
// ResetFrameSorter is used when we need to keep track of frame statistics
// but not to track the frames prior to reset in frame sorter.
void ResetFrameSorter();
void OnFcpReceived();
void set_total_counter(TotalFrameCounter* total_counter) {
total_counter_ = total_counter;
}
private:
void NotifyFrameResult(const viz::BeginFrameArgs& args, bool is_dropped);
RingBufferType ring_buffer_;
size_t total_frames_ = 0;
size_t total_partial_ = 0;
@ -75,7 +69,7 @@ class CC_EXPORT DroppedFrameCounter {
bool fcp_received_ = false;
UkmSmoothnessDataShared* ukm_smoothness_data_ = nullptr;
FrameSorter frame_sorter_;
TotalFrameCounter* total_counter_ = nullptr;
};

@ -1,122 +0,0 @@
// Copyright 2020 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.
#include "cc/metrics/frame_sorter.h"
#include <utility>
namespace cc {
FrameSorter::FrameSorter(InOrderBeginFramesCallback callback)
: flush_callback_(std::move(callback)) {
DCHECK(!flush_callback_.is_null());
}
FrameSorter::~FrameSorter() = default;
void FrameSorter::AddNewFrame(const viz::BeginFrameArgs& args) {
if (current_source_id_.has_value() &&
current_source_id_ > args.frame_id.source_id) {
return;
}
if (current_source_id_.has_value() &&
current_source_id_ < args.frame_id.source_id) {
// The change in source_id can be as a result of crash on gpu process,
// which invalidates existing pending frames (no ack is expected).
Reset();
}
if (!pending_frames_.empty()) {
const auto& last = pending_frames_.back();
DCHECK_LE(last.frame_id.sequence_number, args.frame_id.sequence_number);
}
current_source_id_ = args.frame_id.source_id;
if (frames_expecting_acks_.count(args.frame_id)) {
DCHECK(!frames_expecting_two_acks_.count(args.frame_id));
frames_expecting_two_acks_.insert(args.frame_id);
return;
}
pending_frames_.push_back(args);
frames_expecting_acks_.insert(args.frame_id);
}
void FrameSorter::AddFrameResult(const viz::BeginFrameArgs& args,
bool is_dropped) {
if (pending_frames_.empty() || current_source_id_ > args.frame_id.source_id) {
// The change in source_id can be as a result of crash on gpu process,
// and as a result the corresponding frame to result does not exist.
return;
}
DCHECK(!pending_frames_.empty());
// If Frame expects two acks, record the result but do not push in acked set
if (RemoveFrameExpectingTwoAcks(args.frame_id)) {
received_partial_dropped_acks_.insert({args.frame_id, is_dropped});
return;
}
if (frames_to_ignore_acks_.count(args.frame_id)) {
frames_to_ignore_acks_.erase(args.frame_id);
return;
}
const auto& existing_result =
received_partial_dropped_acks_.find(args.frame_id);
if (existing_result != received_partial_dropped_acks_.end())
is_dropped |= existing_result->second;
received_acks_.insert({args.frame_id, is_dropped});
if (pending_frames_.front().frame_id == args.frame_id) {
FlushAcks();
} else {
// Checks if the corresponding frame to the result, exists in the
// pending_frames list.
DCHECK(std::binary_search(
pending_frames_.begin(), pending_frames_.end(), args,
[](const viz::BeginFrameArgs& one, const viz::BeginFrameArgs& two) {
return one.frame_id < two.frame_id;
}))
<< args.frame_id.ToString()
<< pending_frames_.front().frame_id.ToString();
}
}
bool FrameSorter::RemoveFrameExpectingTwoAcks(
const viz::BeginFrameId& frame_id) {
if (frames_expecting_two_acks_.count(frame_id)) {
frames_expecting_two_acks_.erase(frame_id);
return true;
}
return false;
}
void FrameSorter::Reset() {
for (auto pending_frame : pending_frames_) {
const auto& result = received_acks_.find(pending_frame.frame_id);
if (result == received_acks_.end()) {
// Future acks will be ignored for this frame.
frames_to_ignore_acks_.insert(pending_frame.frame_id);
} else {
flush_callback_.Run(pending_frame, result->second);
}
}
pending_frames_.clear();
frames_expecting_acks_.clear();
received_acks_.clear();
received_partial_dropped_acks_.clear();
}
void FrameSorter::FlushAcks() {
DCHECK(!pending_frames_.empty());
size_t flushed_count = 0;
while (!pending_frames_.empty()) {
const auto first = pending_frames_.front();
const auto& result = received_acks_.find(first.frame_id);
if (result == received_acks_.end())
break;
frames_expecting_acks_.erase(first.frame_id);
pending_frames_.erase(pending_frames_.begin());
++flushed_count;
flush_callback_.Run(first, result->second);
received_acks_.erase(result);
}
DCHECK_GT(flushed_count, 0u);
}
} // namespace cc

@ -1,75 +0,0 @@
// Copyright 2020 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.
#ifndef CC_METRICS_FRAME_SORTER_H_
#define CC_METRICS_FRAME_SORTER_H_
#include <stddef.h>
#include <vector>
#include "base/callback.h"
#include "base/containers/flat_map.h"
#include "base/containers/flat_set.h"
#include "base/optional.h"
#include "cc/cc_export.h"
#include "components/viz/common/frame_sinks/begin_frame_args.h"
namespace cc {
// This class is used to process the frames in order of initiation.
// So regardless of which order frames are terminated, the callback function
// will frames sorter will br called on the frames in the order of initiation
// (e.g. frame_time of that frame).
class CC_EXPORT FrameSorter {
public:
using InOrderBeginFramesCallback =
base::RepeatingCallback<void(const viz::BeginFrameArgs&,
bool /*is_dropped*/)>;
explicit FrameSorter(InOrderBeginFramesCallback callback);
~FrameSorter();
FrameSorter(const FrameSorter&) = delete;
FrameSorter& operator=(const FrameSorter&) = delete;
// The frames must be added in the correct order.
void AddNewFrame(const viz::BeginFrameArgs& args);
// The results can be added in any order. However, the frame must have been
// added by an earlier call to |AddNewFrame()|.
void AddFrameResult(const viz::BeginFrameArgs& args, bool is_dropped);
void Reset();
private:
void FlushAcks();
bool RemoveFrameExpectingTwoAcks(const viz::BeginFrameId& frame_id);
// The callback to run for each flushed frame.
const InOrderBeginFramesCallback flush_callback_;
// Frames which are started.
std::vector<viz::BeginFrameArgs> pending_frames_;
// Frames that expect acks (all frames in pending_frames_).
base::flat_set<viz::BeginFrameId> frames_expecting_acks_;
// Acked frames and their status. (True: dropped, False: not_dropped)
base::flat_map<viz::BeginFrameId, bool> received_acks_;
// Frames that expect two acks.
base::flat_set<viz::BeginFrameId> frames_expecting_two_acks_;
// Frames that need to be ignored. (They are started prior to a reset)
base::flat_set<viz::BeginFrameId> frames_to_ignore_acks_;
// Initial status of a frames that expect two acks.
base::flat_map<viz::BeginFrameId, bool> received_partial_dropped_acks_;
base::Optional<uint64_t> current_source_id_;
};
} // namespace cc
#endif // CC_METRICS_FRAME_SORTER_H_

@ -1,165 +0,0 @@
// Copyright 2020 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.
#include "cc/metrics/frame_sorter.h"
#include <string>
#include <utility>
#include "base/bind.h"
#include "base/strings/string_number_conversions.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace cc {
class FrameSorterTest : public testing::Test {
public:
FrameSorterTest()
: frame_sorter_(base::BindRepeating(&FrameSorterTest::flush_frame,
base::Unretained(this))) {
IncreaseSourceId();
}
~FrameSorterTest() override = default;
const viz::BeginFrameArgs GetNextFrameArgs() {
uint64_t sequence_number = next_frame_sequence_number_++;
return viz::BeginFrameArgs::Create(
BEGINFRAME_FROM_HERE, next_frame_source_id_, sequence_number,
last_begin_frame_time_,
last_begin_frame_time_ + viz::BeginFrameArgs::DefaultInterval(),
viz::BeginFrameArgs::DefaultInterval(), viz::BeginFrameArgs::NORMAL);
}
void IncreaseSourceId() {
next_frame_source_id_++;
current_frame_id = -1;
args_.clear();
next_frame_sequence_number_ = viz::BeginFrameArgs::kStartingFrameNumber;
}
// Simulates a sequence of queries
// Possible queries:
// "S{frame_number}": Start frame.
// "D{frame_number}": End frame as dropped.
// "P{frame_number}": End frame as none-dropped (presented).
// "E{frame_number}": Frame expects twp acks.
// "I": Increase source_id (simulating a crash).
// "R": Reset the frame sorter.
// Method expects the start of frames to be in order starting with 1.
void SimulateQueries(std::vector<std::string> queries) {
for (auto& query : queries) {
int id;
base::StringToInt(query.substr(1), &id);
id--; // Vector is 0 based, frames start from 1.
if (id > current_frame_id) {
current_frame_id++;
args_.push_back(GetNextFrameArgs());
}
switch (query[0]) {
case 'S':
frame_sorter_.AddNewFrame(args_[id]);
break;
case 'D':
frame_sorter_.AddFrameResult(args_[id], true);
break;
case 'P':
frame_sorter_.AddFrameResult(args_[id], false);
break;
case 'I':
IncreaseSourceId();
break;
case 'R':
frame_sorter_.Reset();
break;
}
}
}
void compare_results(
std::vector<std::pair<uint64_t, bool>> expected_results) {
int result_index = 0;
for (auto result : expected_results) {
EXPECT_EQ(sorted_frames_[result_index].first.frame_id.sequence_number,
result.first);
EXPECT_EQ(sorted_frames_[result_index].second, result.second);
result_index++;
}
}
FrameSorter frame_sorter_;
std::vector<std::pair<const viz::BeginFrameArgs, bool>> sorted_frames_;
private:
void flush_frame(const viz::BeginFrameArgs& args, bool is_dropped) {
sorted_frames_.emplace_back(std::make_pair(args, is_dropped));
}
base::TimeTicks last_begin_frame_time_ = base::TimeTicks::Now();
uint64_t next_frame_source_id_ = 0;
uint64_t next_frame_sequence_number_ =
viz::BeginFrameArgs::kStartingFrameNumber;
std::vector<const viz::BeginFrameArgs> args_ = {};
int current_frame_id = -1;
};
TEST_F(FrameSorterTest, TestSortingFrames) {
std::vector<std::string> queries = {"S1", "S2", "P1", "S3",
"P3", "S4", "P2", "D4"};
std::vector<std::pair<uint64_t, bool>> expected_results = {
{1, false}, {2, false}, {3, false}, {4, true}};
SimulateQueries(queries);
EXPECT_EQ(sorted_frames_.size(), 4u);
compare_results(expected_results);
}
TEST_F(FrameSorterTest, ResetInMiddleOfAcks) {
std::vector<std::string> queries = {"S1", "S2", "P1", "S3", "P3", "S4",
"R", "S5", "P2", "D5", "D4"};
std::vector<std::pair<uint64_t, bool>> expected_results = {
{1, false}, {3, false}, {5, true}};
SimulateQueries(queries);
EXPECT_EQ(sorted_frames_.size(), 3u);
compare_results(expected_results);
}
TEST_F(FrameSorterTest, ExpectingMultipleAcks) {
std::vector<std::string> queries = {"S1", "S2", "P1", "S2", "S3", "P3",
"S4", "S5", "P2", "D5", "D4", "D2"};
std::vector<std::pair<uint64_t, bool>> expected_results = {
{1, false}, {2, true}, {3, false}, {4, true}, {5, true}};
SimulateQueries(queries);
EXPECT_EQ(sorted_frames_.size(), 5u);
compare_results(expected_results);
}
TEST_F(FrameSorterTest, ExpectingMultipleAcksWithReset) {
std::vector<std::string> queries = {"S1", "S2", "P1", "S2", "S3", "P3", "S4",
"R", "S5", "P2", "D2", "D5", "D4"};
std::vector<std::pair<uint64_t, bool>> expected_results = {
{1, false}, {3, false}, {5, true}};
SimulateQueries(queries);
EXPECT_EQ(sorted_frames_.size(), 3u);
compare_results(expected_results);
}
TEST_F(FrameSorterTest, ExpectingMultipleAcksWithSourceIdIncrease) {
std::vector<std::string> queries = {"S1", "S1", "P1", "S2", "S2", "I", "R",
"S1", "S1", "P1", "S2", "S2", "D1"};
std::vector<std::pair<uint64_t, bool>> expected_results = {{1, true}};
SimulateQueries(queries);
EXPECT_EQ(sorted_frames_.size(), 1u);
compare_results(expected_results);
}
} // namespace cc

@ -7,7 +7,6 @@
#include <stddef.h>
#include <string>
#include <utility>
#include "base/memory/ptr_util.h"
#include "base/time/tick_clock.h"
@ -40,9 +39,7 @@ FakeCompositorTimingHistory::FakeCompositorTimingHistory(
reporting_controller.get()),
rendering_stats_instrumentation_owned_(
std::move(rendering_stats_instrumentation)),
reporting_controller_owned_(std::move(reporting_controller)) {
reporting_controller_owned_->SetDroppedFrameCounter(&dropped_counter);
}
reporting_controller_owned_(std::move(reporting_controller)) {}
FakeCompositorTimingHistory::~FakeCompositorTimingHistory() = default;

@ -12,7 +12,6 @@
#include "base/time/time.h"
#include "cc/metrics/compositor_timing_history.h"
#include "cc/metrics/dropped_frame_counter.h"
#include "cc/scheduler/scheduler.h"
#include "testing/gtest/include/gtest/gtest.h"
@ -80,7 +79,6 @@ class FakeCompositorTimingHistory : public CompositorTimingHistory {
base::TimeDelta prepare_tiles_duration_;
base::TimeDelta activate_duration_;
base::TimeDelta draw_duration_;
DroppedFrameCounter dropped_counter;
};
class TestScheduler : public Scheduler {

@ -2975,8 +2975,6 @@ void LayerTreeHostImpl::DidLoseLayerTreeFrameSink() {
has_valid_layer_tree_frame_sink_ = false;
client_->DidLoseLayerTreeFrameSinkOnImplThread();
lag_tracking_manager_.Clear();
dropped_frame_counter_.ResetFrameSorter();
}
bool LayerTreeHostImpl::OnlyExpandTopControlsAtPageTop() const {

@ -14054,21 +14054,20 @@ TEST_F(LayerTreeHostImplTest, FrameCounterReset) {
dropped_frame_counter->AddGoodFrame();
EXPECT_EQ(dropped_frame_counter->total_frames(), 1u);
dropped_frame_counter->AddDroppedFrameAffectingSmoothness();
// FCP not received, so the total_smoothness_dropped_ won't increase.
EXPECT_EQ(dropped_frame_counter->total_smoothness_dropped(), 0u);
auto interval = base::TimeDelta::FromMilliseconds(16);
base::TimeTicks now = base::TimeTicks::Now();
auto deadline = now + interval;
viz::BeginFrameArgs args = viz::BeginFrameArgs::Create(
BEGINFRAME_FROM_HERE, 1u /*source_id*/, 2u /*sequence_number*/, now,
BEGINFRAME_FROM_HERE, 1u /*source_id*/, 1u /*sequence_number*/, now,
deadline, interval, viz::BeginFrameArgs::NORMAL);
dropped_frame_counter->OnEndFrame(args, true);
// FCP not received, so the total_smoothness_dropped_ won't increase.
EXPECT_EQ(dropped_frame_counter->total_smoothness_dropped(), 0u);
BeginMainFrameMetrics begin_frame_metrics;
begin_frame_metrics.should_measure_smoothness = true;
host_impl_->ReadyToCommit(args, &begin_frame_metrics);
dropped_frame_counter->OnEndFrame(args, true);
dropped_frame_counter->AddDroppedFrameAffectingSmoothness();
EXPECT_EQ(dropped_frame_counter->total_smoothness_dropped(), 1u);
total_frame_counter->set_total_frames_for_testing(1u);

@ -9355,7 +9355,7 @@ class LayerTreeHostUkmSmoothnessMetric : public LayerTreeTest {
}
// Mark every frame as a dropped frame affecting smoothness.
host_impl->dropped_frame_counter()->OnEndFrame(viz::BeginFrameArgs(), true);
host_impl->dropped_frame_counter()->AddDroppedFrameAffectingSmoothness();
host_impl->SetNeedsRedraw();
--frames_counter_;
}
@ -9401,7 +9401,7 @@ class LayerTreeHostUkmSmoothnessMemoryOwnership : public LayerTreeTest {
}
// Mark every frame as a dropped frame affecting smoothness.
host_impl->dropped_frame_counter()->OnEndFrame(viz::BeginFrameArgs(), true);
host_impl->dropped_frame_counter()->AddDroppedFrameAffectingSmoothness();
host_impl->SetNeedsRedraw();
--frames_counter_;
}