Skip to content

Commit

Permalink
[Code health] Remove an un-necessary member in FrameSequenceTracker
Browse files Browse the repository at this point in the history
This CL doesn't cause behavior change.

This CL removes a type_ member in FrameSequenceTracker, because there
is a type_ in the FrameSequenceMetrics class, and the FrameSequenceTracker
class owns a FrameSequenceMetrics object.

Bug: None
Change-Id: I38bdc6f92533c7fb42e268f555a996a60fe53e77
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2155620
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: weiliangc <weiliangc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760782}
  • Loading branch information
xidachen authored and Commit Bot committed Apr 21, 2020
1 parent 4ff6783 commit 3114cbb
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 18 deletions.
18 changes: 8 additions & 10 deletions cc/metrics/frame_sequence_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#if DCHECK_IS_ON()
#define TRACKER_TRACE_STREAM frame_sequence_trace_
#define TRACKER_DCHECK_MSG \
" in " << GetFrameSequenceTrackerTypeName(this->type_) \
" in " << GetFrameSequenceTrackerTypeName(this->type()) \
<< " tracker: " << frame_sequence_trace_.str() << " (" \
<< frame_sequence_trace_.str().size() << ")";
#else
Expand Down Expand Up @@ -58,23 +58,21 @@ const char* FrameSequenceTracker::GetFrameSequenceTrackerTypeName(
FrameSequenceTracker::FrameSequenceTracker(
FrameSequenceTrackerType type,
ThroughputUkmReporter* throughput_ukm_reporter)
: type_(type),
custom_sequence_id_(-1),
: custom_sequence_id_(-1),
metrics_(std::make_unique<FrameSequenceMetrics>(type,
throughput_ukm_reporter)),
trace_data_(metrics_.get()) {
DCHECK_LT(type_, FrameSequenceTrackerType::kMaxType);
DCHECK(type_ != FrameSequenceTrackerType::kCustom);
DCHECK_LT(type, FrameSequenceTrackerType::kMaxType);
DCHECK(type != FrameSequenceTrackerType::kCustom);
}

FrameSequenceTracker::FrameSequenceTracker(
int custom_sequence_id,
FrameSequenceMetrics::CustomReporter custom_reporter)
: type_(FrameSequenceTrackerType::kCustom),
custom_sequence_id_(custom_sequence_id),
metrics_(
std::make_unique<FrameSequenceMetrics>(type_,
/*ukm_reporter=*/nullptr)),
: custom_sequence_id_(custom_sequence_id),
metrics_(std::make_unique<FrameSequenceMetrics>(
FrameSequenceTrackerType::kCustom,
/*ukm_reporter=*/nullptr)),
trace_data_(metrics_.get()) {
DCHECK_GT(custom_sequence_id_, 0);
metrics_->SetCustomReporter(std::move(custom_reporter));
Expand Down
3 changes: 1 addition & 2 deletions cc/metrics/frame_sequence_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class CC_EXPORT FrameSequenceTracker {
bool ShouldReportMetricsNow(const viz::BeginFrameArgs& args) const;

FrameSequenceMetrics* metrics() { return metrics_.get(); }
FrameSequenceTrackerType type() const { return type_; }
FrameSequenceTrackerType type() const { return metrics_->type(); }
int custom_sequence_id() const { return custom_sequence_id_; }

std::unique_ptr<FrameSequenceMetrics> TakeMetrics();
Expand Down Expand Up @@ -158,7 +158,6 @@ class CC_EXPORT FrameSequenceTracker {

bool ShouldIgnoreSequence(uint64_t sequence_number) const;

const FrameSequenceTrackerType type_;
const int custom_sequence_id_;

TerminationStatus termination_status_ = TerminationStatus::kActive;
Expand Down
11 changes: 6 additions & 5 deletions cc/metrics/frame_sequence_tracker_collection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ void FrameSequenceTrackerCollection::StopSequence(
std::move(frame_trackers_[type]);

if (compositor_frame_reporting_controller_)
compositor_frame_reporting_controller_->RemoveActiveTracker(tracker->type_);
compositor_frame_reporting_controller_->RemoveActiveTracker(
tracker->type());

frame_trackers_.erase(type);
tracker->ScheduleTerminate();
Expand Down Expand Up @@ -205,17 +206,17 @@ void FrameSequenceTrackerCollection::NotifyFramePresented(
// on its destruction, which add its data to |custom_tracker_results_|
// to be picked up by caller.
auto metrics = tracker->TakeMetrics();
if (tracker->type() == FrameSequenceTrackerType::kCustom)
if (metrics->type() == FrameSequenceTrackerType::kCustom)
continue;

auto key = std::make_pair(tracker->type(), metrics->GetEffectiveThread());
auto key = std::make_pair(metrics->type(), metrics->GetEffectiveThread());
if (accumulated_metrics_.contains(key)) {
metrics->Merge(std::move(accumulated_metrics_[key]));
accumulated_metrics_.erase(key);
}

if (metrics->HasEnoughDataForReporting()) {
if (tracker->type() == FrameSequenceTrackerType::kUniversal) {
if (metrics->type() == FrameSequenceTrackerType::kUniversal) {
uint32_t frames_expected = metrics->impl_throughput().frames_expected;
uint32_t frames_produced =
metrics->aggregated_throughput().frames_produced;
Expand Down Expand Up @@ -287,7 +288,7 @@ FrameSequenceTracker*
FrameSequenceTrackerCollection::GetRemovalTrackerForTesting(
FrameSequenceTrackerType type) {
for (const auto& tracker : removal_trackers_)
if (tracker->type_ == type)
if (tracker->type() == type)
return tracker.get();
return nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion cc/metrics/frame_sequence_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class FrameSequenceTrackerTest : public testing::Test {
bool RemovalTrackerExists(unsigned index,
FrameSequenceTrackerType type) const {
DCHECK_GT(collection_.removal_trackers_.size(), index);
return collection_.removal_trackers_[index]->type_ == type;
return collection_.removal_trackers_[index]->type() == type;
}

void GenerateSequence(const char* str) {
Expand Down

0 comments on commit 3114cbb

Please sign in to comment.