Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(361)

Unified Diff: ash/rotator/screen_rotation_animator.cc

Issue 2771713004: Adds UMA for screen rotation animation smoothness. (Closed)
Patch Set: Adds histograms.xml. Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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();

Powered by Google App Engine
This is Rietveld 408576698