Commit cc0e9d5e authored by Fady Samuel's avatar Fady Samuel Committed by Commit Bot

Revert "Surface synchronization: Don't report blocked duration of throttled children"

This reverts commit d15ba9ba.

Reason for revert: This doesn't seem to solve the deadline reporting bug. This was a speculative fix that didn't fix the issue.

Original change's description:
> Surface synchronization: Don't report blocked duration of throttled children
> 
> A child can be "throttled" for an arbitrary length of time if it is never
> embedded. This skews blocked duration metrics. Simply don't report those
> metrics if a surface is blocked on a parent.
> 
> Bug: 890767
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
> Change-Id: I3c8a8494a5b1e5a973b2e5c2b61cffe5c2b76f6f
> Reviewed-on: https://chromium-review.googlesource.com/c/1274591
> Commit-Queue: Fady Samuel <fsamuel@chromium.org>
> Reviewed-by: Saman Sami <samans@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598886}

TBR=fsamuel@chromium.org,samans@chromium.org

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

Bug: 890767
Change-Id: I3b1188d49a1301681391d526abaa25dcabf69f79
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/c/1281083Reviewed-by: default avatarFady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599716}
parent c3e113ab
...@@ -198,11 +198,6 @@ void Surface::OnSurfaceDependencyAdded() { ...@@ -198,11 +198,6 @@ void Surface::OnSurfaceDependencyAdded() {
DCHECK(frame_sink_id_dependencies_.empty()); DCHECK(frame_sink_id_dependencies_.empty());
// Cancel the deadline and don't report UMA. The UMA metrics are only
// meaningful for activation dependencies and not throttling.
deadline_->CancelWithoutReport();
DCHECK(!deadline_->has_deadline());
// All blockers have been cleared. The surface can be activated now. // All blockers have been cleared. The surface can be activated now.
ActivatePendingFrame(base::nullopt); ActivatePendingFrame(base::nullopt);
} }
......
...@@ -41,13 +41,6 @@ base::Optional<base::TimeDelta> SurfaceDependencyDeadline::Cancel() { ...@@ -41,13 +41,6 @@ base::Optional<base::TimeDelta> SurfaceDependencyDeadline::Cancel() {
return CancelInternal(false); return CancelInternal(false);
} }
void SurfaceDependencyDeadline::CancelWithoutReport() {
if (!deadline_)
return;
begin_frame_source_->RemoveObserver(this);
deadline_.reset();
}
void SurfaceDependencyDeadline::InheritFrom( void SurfaceDependencyDeadline::InheritFrom(
const SurfaceDependencyDeadline& other) { const SurfaceDependencyDeadline& other) {
if (*this == other) if (*this == other)
...@@ -103,7 +96,8 @@ base::Optional<base::TimeDelta> SurfaceDependencyDeadline::CancelInternal( ...@@ -103,7 +96,8 @@ base::Optional<base::TimeDelta> SurfaceDependencyDeadline::CancelInternal(
if (!deadline_) if (!deadline_)
return base::nullopt; return base::nullopt;
CancelWithoutReport(); begin_frame_source_->RemoveObserver(this);
deadline_.reset();
base::TimeDelta duration = tick_clock_->NowTicks() - start_time_; base::TimeDelta duration = tick_clock_->NowTicks() - start_time_;
......
...@@ -36,12 +36,6 @@ class VIZ_SERVICE_EXPORT SurfaceDependencyDeadline : public BeginFrameObserver { ...@@ -36,12 +36,6 @@ class VIZ_SERVICE_EXPORT SurfaceDependencyDeadline : public BeginFrameObserver {
// deadline set, then return base::nullopt. // deadline set, then return base::nullopt.
base::Optional<base::TimeDelta> Cancel(); base::Optional<base::TimeDelta> Cancel();
// If a deadline had been set, then cancel the deadline and don't
// generate a UMA report. When a surface activation has been throttled
// then the time to activation could be arbitrarily delayed and may
// skew metrics.
void CancelWithoutReport();
bool has_deadline() const { return deadline_.has_value(); } bool has_deadline() const { return deadline_.has_value(); }
base::Optional<base::TimeTicks> deadline_for_testing() const { base::Optional<base::TimeTicks> deadline_for_testing() const {
......
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