Commit c5352cfb authored by Mohsen Izadi's avatar Mohsen Izadi Committed by Commit Bot

Use mockable time in CompositorFrameReporter

This CL adds the possibility to inject a base::TickClock* into
CompositorFrameReporter, so its timing can be mocked by tests.
CompositorFrameReportingController and CompositorFrameReporterTest are
also updated to make use of this.

Bug: 1054009,1057193
Change-Id: I38ab3ebb92b06f5c31e1dbdc514cab29b9791cae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2160408
Commit-Queue: Mohsen Izadi <mohsen@chromium.org>
Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762122}
parent edf7e440
...@@ -229,6 +229,7 @@ CompositorFrameReporter::CopyReporterAtBeginImplStage() const { ...@@ -229,6 +229,7 @@ CompositorFrameReporter::CopyReporterAtBeginImplStage() const {
new_reporter->current_stage_.stage_type = new_reporter->current_stage_.stage_type =
StageType::kBeginImplFrameToSendBeginMainFrame; StageType::kBeginImplFrameToSendBeginMainFrame;
new_reporter->current_stage_.start_time = stage_history_.front().start_time; new_reporter->current_stage_.start_time = stage_history_.front().start_time;
new_reporter->set_tick_clock(tick_clock_);
return new_reporter; return new_reporter;
} }
...@@ -292,7 +293,7 @@ void CompositorFrameReporter::OnAbortBeginMainFrame(base::TimeTicks timestamp) { ...@@ -292,7 +293,7 @@ void CompositorFrameReporter::OnAbortBeginMainFrame(base::TimeTicks timestamp) {
void CompositorFrameReporter::OnDidNotProduceFrame( void CompositorFrameReporter::OnDidNotProduceFrame(
FrameSkippedReason skip_reason) { FrameSkippedReason skip_reason) {
did_not_produce_frame_time_ = base::TimeTicks::Now(); did_not_produce_frame_time_ = Now();
frame_skip_reason_ = skip_reason; frame_skip_reason_ = skip_reason;
} }
...@@ -323,7 +324,7 @@ void CompositorFrameReporter::SetEventsMetrics( ...@@ -323,7 +324,7 @@ void CompositorFrameReporter::SetEventsMetrics(
void CompositorFrameReporter::TerminateReporter() { void CompositorFrameReporter::TerminateReporter() {
if (frame_termination_status_ == FrameTerminationStatus::kUnknown) if (frame_termination_status_ == FrameTerminationStatus::kUnknown)
TerminateFrame(FrameTerminationStatus::kUnknown, base::TimeTicks::Now()); TerminateFrame(FrameTerminationStatus::kUnknown, Now());
DCHECK_EQ(current_stage_.start_time, base::TimeTicks()); DCHECK_EQ(current_stage_.start_time, base::TimeTicks());
const char* termination_status_str = nullptr; const char* termination_status_str = nullptr;
switch (frame_termination_status_) { switch (frame_termination_status_) {
...@@ -723,4 +724,8 @@ base::TimeDelta CompositorFrameReporter::SumOfStageHistory() const { ...@@ -723,4 +724,8 @@ base::TimeDelta CompositorFrameReporter::SumOfStageHistory() const {
return sum; return sum;
} }
base::TimeTicks CompositorFrameReporter::Now() const {
return tick_clock_->NowTicks();
}
} // namespace cc } // namespace cc
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <vector> #include <vector>
#include "base/optional.h" #include "base/optional.h"
#include "base/time/default_tick_clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "cc/base/base_export.h" #include "cc/base/base_export.h"
#include "cc/cc_export.h" #include "cc/cc_export.h"
...@@ -175,6 +176,11 @@ class CC_EXPORT CompositorFrameReporter { ...@@ -175,6 +176,11 @@ class CC_EXPORT CompositorFrameReporter {
FrameSkippedReason frame_skip_reason() const { return *frame_skip_reason_; } FrameSkippedReason frame_skip_reason() const { return *frame_skip_reason_; }
void set_tick_clock(const base::TickClock* tick_clock) {
DCHECK(tick_clock);
tick_clock_ = tick_clock;
}
private: private:
void TerminateReporter(); void TerminateReporter();
void EndCurrentStage(base::TimeTicks end_time); void EndCurrentStage(base::TimeTicks end_time);
...@@ -223,6 +229,8 @@ class CC_EXPORT CompositorFrameReporter { ...@@ -223,6 +229,8 @@ class CC_EXPORT CompositorFrameReporter {
// This method is only used for DCheck // This method is only used for DCheck
base::TimeDelta SumOfStageHistory() const; base::TimeDelta SumOfStageHistory() const;
base::TimeTicks Now() const;
const bool should_report_metrics_; const bool should_report_metrics_;
StageData current_stage_; StageData current_stage_;
...@@ -261,6 +269,8 @@ class CC_EXPORT CompositorFrameReporter { ...@@ -261,6 +269,8 @@ class CC_EXPORT CompositorFrameReporter {
base::Optional<base::TimeTicks> did_not_produce_frame_time_; base::Optional<base::TimeTicks> did_not_produce_frame_time_;
base::Optional<FrameSkippedReason> frame_skip_reason_; base::Optional<FrameSkippedReason> frame_skip_reason_;
base::Optional<base::TimeTicks> main_frame_abort_time_; base::Optional<base::TimeTicks> main_frame_abort_time_;
const base::TickClock* tick_clock_ = base::DefaultTickClock::GetInstance();
}; };
} // namespace cc } // namespace cc
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/strings/strcat.h" #include "base/strings/strcat.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/simple_test_tick_clock.h"
#include "cc/input/scroll_input_type.h" #include "cc/input/scroll_input_type.h"
#include "cc/metrics/compositor_frame_reporting_controller.h" #include "cc/metrics/compositor_frame_reporting_controller.h"
#include "cc/metrics/event_metrics.h" #include "cc/metrics/event_metrics.h"
...@@ -34,16 +35,17 @@ class CompositorFrameReporterTest : public testing::Test { ...@@ -34,16 +35,17 @@ class CompositorFrameReporterTest : public testing::Test {
base::TimeTicks() + base::TimeDelta::FromMilliseconds(16), base::TimeTicks() + base::TimeDelta::FromMilliseconds(16),
nullptr, nullptr,
/*should_report_metrics=*/true)) { /*should_report_metrics=*/true)) {
pipeline_reporter_->set_tick_clock(&test_tick_clock_);
AdvanceNowByMs(1); AdvanceNowByMs(1);
} }
protected: protected:
base::TimeTicks AdvanceNowByMs(int advance_ms) { base::TimeTicks AdvanceNowByMs(int advance_ms) {
now_ += base::TimeDelta::FromMicroseconds(advance_ms); test_tick_clock_.Advance(base::TimeDelta::FromMicroseconds(advance_ms));
return now_; return test_tick_clock_.NowTicks();
} }
base::TimeTicks Now() { return now_; } base::TimeTicks Now() { return test_tick_clock_.NowTicks(); }
viz::FrameTimingDetails BuildFrameTimingDetails() { viz::FrameTimingDetails BuildFrameTimingDetails() {
viz::FrameTimingDetails frame_timing_details; viz::FrameTimingDetails frame_timing_details;
...@@ -56,10 +58,11 @@ class CompositorFrameReporterTest : public testing::Test { ...@@ -56,10 +58,11 @@ class CompositorFrameReporterTest : public testing::Test {
return frame_timing_details; return frame_timing_details;
} }
std::unique_ptr<CompositorFrameReporter> pipeline_reporter_; // This should be defined before |pipeline_reporter_| so it is created before
// and destroyed after that.
base::SimpleTestTickClock test_tick_clock_;
private: std::unique_ptr<CompositorFrameReporter> pipeline_reporter_;
base::TimeTicks now_;
}; };
TEST_F(CompositorFrameReporterTest, MainFrameAbortedReportingTest) { TEST_F(CompositorFrameReporterTest, MainFrameAbortedReportingTest) {
......
...@@ -67,11 +67,10 @@ void CompositorFrameReportingController::WillBeginImplFrame( ...@@ -67,11 +67,10 @@ void CompositorFrameReportingController::WillBeginImplFrame(
reporter->TerminateFrame(FrameTerminationStatus::kDidNotProduceFrame, reporter->TerminateFrame(FrameTerminationStatus::kDidNotProduceFrame,
reporter->did_not_produce_frame_time()); reporter->did_not_produce_frame_time());
} }
std::unique_ptr<CompositorFrameReporter> reporter = auto reporter = std::make_unique<CompositorFrameReporter>(
std::make_unique<CompositorFrameReporter>( active_trackers_, args.frame_id, args.frame_time + (args.interval * 1.5),
active_trackers_, args.frame_id, latency_ukm_reporter_.get(), should_report_metrics_);
args.frame_time + (args.interval * 1.5), latency_ukm_reporter_.get(), reporter->set_tick_clock(tick_clock_);
should_report_metrics_);
reporter->StartStage(StageType::kBeginImplFrameToSendBeginMainFrame, reporter->StartStage(StageType::kBeginImplFrameToSendBeginMainFrame,
begin_time); begin_time);
reporters_[PipelineStage::kBeginImplFrame] = std::move(reporter); reporters_[PipelineStage::kBeginImplFrame] = std::move(reporter);
...@@ -94,11 +93,11 @@ void CompositorFrameReportingController::WillBeginMainFrame( ...@@ -94,11 +93,11 @@ void CompositorFrameReportingController::WillBeginMainFrame(
// In this case we have already submitted the ImplFrame, but we received // In this case we have already submitted the ImplFrame, but we received
// beginMain frame before next BeginImplFrame (Not reached the ImplFrame // beginMain frame before next BeginImplFrame (Not reached the ImplFrame
// deadline yet). So will start a new reporter at BeginMainFrame. // deadline yet). So will start a new reporter at BeginMainFrame.
std::unique_ptr<CompositorFrameReporter> reporter = auto reporter = std::make_unique<CompositorFrameReporter>(
std::make_unique<CompositorFrameReporter>(
active_trackers_, args.frame_id, active_trackers_, args.frame_id,
args.frame_time + (args.interval * 1.5), args.frame_time + (args.interval * 1.5), latency_ukm_reporter_.get(),
latency_ukm_reporter_.get(), should_report_metrics_); should_report_metrics_);
reporter->set_tick_clock(tick_clock_);
reporter->StartStage(StageType::kSendBeginMainFrameToCommit, Now()); reporter->StartStage(StageType::kSendBeginMainFrameToCommit, Now());
reporters_[PipelineStage::kBeginMainFrame] = std::move(reporter); reporters_[PipelineStage::kBeginMainFrame] = std::move(reporter);
} }
......
...@@ -79,6 +79,7 @@ class CC_EXPORT CompositorFrameReportingController { ...@@ -79,6 +79,7 @@ class CC_EXPORT CompositorFrameReportingController {
virtual void RemoveActiveTracker(FrameSequenceTrackerType type); virtual void RemoveActiveTracker(FrameSequenceTrackerType type);
void set_tick_clock(const base::TickClock* tick_clock) { void set_tick_clock(const base::TickClock* tick_clock) {
DCHECK(tick_clock);
tick_clock_ = tick_clock; tick_clock_ = tick_clock;
} }
......
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