Chromium Code Reviews| Index: ash/rotator/screen_rotation_animator.cc |
| diff --git a/ash/rotator/screen_rotation_animator.cc b/ash/rotator/screen_rotation_animator.cc |
| index c0dbe2058d6f7a4c33314ed1f07650cde2446195..cda85cb88eac97a8e7fabcf1354158b186f7f0e1 100644 |
| --- a/ash/rotator/screen_rotation_animator.cc |
| +++ b/ash/rotator/screen_rotation_animator.cc |
| @@ -14,6 +14,7 @@ |
| #include "ash/shell.h" |
| #include "base/command_line.h" |
| #include "base/memory/ptr_util.h" |
| +#include "base/metrics/histogram_macros.h" |
| #include "base/time/time.h" |
| #include "ui/aura/window.h" |
| #include "ui/compositor/layer.h" |
| @@ -47,6 +48,10 @@ const int kRotationDurationInMs = 250; |
| const int kCounterClockWiseRotationFactor = 1; |
| const int kClockWiseRotationFactor = -1; |
| +// The percentage histogram boundary. |
| +const int kPercentage_MIN = 1; |
| +const int kPercentage_MAX = 101; |
|
varkha
2017/03/24 20:38:21
Don't need those if you use UMA_HISTOGRAM_PERCENTA
wutao
2017/03/24 21:48:24
Will remove.
|
| + |
| // Aborts the active animations of the layer, and recurses upon its child |
| // layers. |
| void AbortAnimations(ui::Layer* layer) { |
| @@ -143,7 +148,7 @@ LayerCleanupObserver::~LayerCleanupObserver() { |
| void LayerCleanupObserver::OnLayerAnimationEnded( |
| ui::LayerAnimationSequence* sequence) { |
| if (animator_) |
| - animator_->OnLayerAnimationEnded(); |
| + animator_->ProcessAnimationQueue(); |
| delete this; |
| } |
| @@ -151,7 +156,7 @@ void LayerCleanupObserver::OnLayerAnimationEnded( |
| void LayerCleanupObserver::OnLayerAnimationAborted( |
| ui::LayerAnimationSequence* sequence) { |
| if (animator_) |
| - animator_->OnLayerAnimationAborted(); |
| + animator_->ProcessAnimationQueue(); |
| delete this; |
| } |
| @@ -177,13 +182,35 @@ struct ScreenRotationAnimator::ScreenRotationRequest { |
| display::Display::RotationSource source; |
| }; |
| +class ScreenRotationAnimator::ScreenRotationAnimationMetricsReporter |
| + : public ui::AnimationMetricsReporter { |
| + void Report(int value) override { |
| + base::LinearHistogram::FactoryGet( |
|
Mark P
2017/03/24 20:32:41
Maybe I haven't thought about this enough, but why
varkha
2017/03/24 20:38:21
UMA_HISTOGRAM_PERCENTAGE("Ash.Rotation.AnimationSm
wutao
2017/03/24 21:48:24
Thanks.
wutao
2017/03/24 21:48:24
I was confused by the use case of UMA_HISTOGRAM_PE
|
| + "Ash.Rotation.AnimationSmoothness", kPercentage_MIN, kPercentage_MAX, |
| + kPercentage_MAX + 1, base::HistogramBase::kUmaTargetedHistogramFlag) |
| + ->Add(value); |
| + } |
| +}; |
| + |
| ScreenRotationAnimator::ScreenRotationAnimator(int64_t display_id) |
| : display_id_(display_id), |
| is_rotating_(false), |
| + metrics_reporter_( |
| + base::MakeUnique<ScreenRotationAnimationMetricsReporter>()), |
| disable_animation_timers_for_test_(false), |
| weak_factory_(this) {} |
| -ScreenRotationAnimator::~ScreenRotationAnimator() {} |
| +ScreenRotationAnimator::~ScreenRotationAnimator() { |
| + // To prevent a call to |LayerCleanupObserver::OnLayerAnimationAborted()| from |
| + // calling a method on the |animator_|. |
| + weak_factory_.InvalidateWeakPtrs(); |
| + |
| + // Explicitly reset the |old_layer_tree_owner_| and |metrics_reporter_| in |
| + // order to make sure |metrics_reporter_| outlives the attached animation |
| + // sequence. |
| + old_layer_tree_owner_.reset(); |
| + metrics_reporter_.reset(); |
| +} |
| void ScreenRotationAnimator::AnimateRotation( |
| std::unique_ptr<ScreenRotationRequest> rotation_request) { |
| @@ -278,6 +305,7 @@ void ScreenRotationAnimator::AnimateRotation( |
| // control the animation. |
| if (disable_animation_timers_for_test_) |
| animator->set_disable_timer_for_test(true); |
| + animation_sequence->SetAnimationMetricsReporter(metrics_reporter_.get()); |
| animator->StartAnimation(animation_sequence.release()); |
| rotation_request.reset(); |
| @@ -313,15 +341,6 @@ void ScreenRotationAnimator::RemoveScreenRotationAnimatorObserver( |
| screen_rotation_animator_observers_.RemoveObserver(observer); |
| } |
| -void ScreenRotationAnimator::OnLayerAnimationEnded() { |
| - ProcessAnimationQueue(); |
| -} |
| - |
| -void ScreenRotationAnimator::OnLayerAnimationAborted() { |
| - AbortAnimations(old_layer_tree_owner_->root()); |
| - ProcessAnimationQueue(); |
| -} |
| - |
| void ScreenRotationAnimator::ProcessAnimationQueue() { |
| is_rotating_ = false; |
| old_layer_tree_owner_.reset(); |