[animations] Adds metrics for jank on selected layer animations

ui/compositor/layer_animation_element.cc

Index: ui/compositor/layer_animation_element.cc
diff --git a/ui/compositor/layer_animation_element.cc b/ui/compositor/layer_animation_element.cc
index b446b7ad9d8248256f2f88e8b3ea5c7156fd6dbe..61e063e051f73ec0e4c00aedf58552b0d3729659 100644
--- a/ui/compositor/layer_animation_element.cc
+++ b/ui/compositor/layer_animation_element.cc
@@ -447,8 +447,8 @@ LayerAnimationElement::TargetValue::TargetValue(
 
 // LayerAnimationElement -------------------------------------------------------
 
-LayerAnimationElement::LayerAnimationElement(
-    AnimatableProperties properties, base::TimeDelta duration)
+LayerAnimationElement::LayerAnimationElement(AnimatableProperties properties,
+                                             base::TimeDelta duration)
     : first_frame_(true),
       properties_(properties),
       duration_(GetEffectiveDuration(duration)),
@@ -456,11 +456,12 @@ LayerAnimationElement::LayerAnimationElement(
       animation_id_(cc::AnimationIdProvider::NextAnimationId()),
       animation_group_id_(0),
       last_progressed_fraction_(0.0),
-      weak_ptr_factory_(this) {
-}
+      animation_metrics_reporter_(nullptr),
+      start_frame_number_(0),
+      weak_ptr_factory_(this) {}
 
 LayerAnimationElement::LayerAnimationElement(
-    const LayerAnimationElement &element)
+    const LayerAnimationElement& element)
     : first_frame_(element.first_frame_),
       properties_(element.properties_),
       duration_(element.duration_),
@@ -468,8 +469,9 @@ LayerAnimationElement::LayerAnimationElement(
       animation_id_(cc::AnimationIdProvider::NextAnimationId()),
       animation_group_id_(element.animation_group_id_),
       last_progressed_fraction_(element.last_progressed_fraction_),
-      weak_ptr_factory_(this) {
-}
+      animation_metrics_reporter_(nullptr),
+      start_frame_number_(0),
+      weak_ptr_factory_(this) {}
 
 LayerAnimationElement::~LayerAnimationElement() {
 }
@@ -481,6 +483,7 @@ void LayerAnimationElement::Start(LayerAnimationDelegate* delegate,
   animation_group_id_ = animation_group_id;
   last_progressed_fraction_ = 0.0;
   OnStart(delegate);
+  start_frame_number_ = delegate->GetFrameNumber();
   RequestEffectiveStart(delegate);
   first_frame_ = false;
 }
@@ -534,10 +537,24 @@ bool LayerAnimationElement::IsFinished(base::TimeTicks time,
 }
 
 bool LayerAnimationElement::ProgressToEnd(LayerAnimationDelegate* delegate) {
-  if (first_frame_)
+  if (first_frame_) {
     OnStart(delegate);
+    start_frame_number_ = delegate->GetFrameNumber();
+  }
   base::WeakPtr<LayerAnimationElement> alive(weak_ptr_factory_.GetWeakPtr());
   bool need_draw = OnProgress(1.0, delegate);
+
+  int end_frame_number = delegate->GetFrameNumber();
+  if (animation_metrics_reporter_ && end_frame_number > start_frame_number_) {
+    int smoothness = 100;
+    if (!duration_.is_zero()) {
+      smoothness = std::min(
+          100, static_cast<int>(
+                   100 * ((1000.f * (end_frame_number - start_frame_number_)) /
+                          (60.f * duration_.InMillisecondsF()))));

[ajuma, 2017/01/18 23:02:51]

Looking at how this can get called, it seems possible this gets called when
we're ending the animation early because of it being pre-empted. If that's
possible then the duration wouldn't be the right one to use.

[varkha, 2017/01/20 00:16:47]

Is something like that too naive? Looking at the ways ProgressToEnd gets called
it is not easy to distinguish and as long as we only use this timing for metrics
it should not change statistics significantly (and should not affect anything
else).

[ajuma, 2017/01/20 14:31:58]

My only worry is that the statistics might be misleading (that is, we'll think
frame rate is poor when it's not; or something that increases/decreases how
often animations end early will look like a regression/progression in
smoothness). 

One thing we could do to filter out animations that ended early is to check
whether effective_start_time_ + duration_ >= now (or, alternatively, we could
use now - effective_start_time_ instead of using duration_).

Either way, would it be possible to unit test this?

[varkha, 2017/01/20 17:07:58]

Isn't that exactly what I am doing here?

[ajuma, 2017/01/20 18:03:05]

Currently, this just uses duration_, which might not be the actual duration if
the animation ends early. e.g. Say the duration_ is 2 seconds, but the animation
is pre-empted after 1 second, I think duration_ will still have value 2 when
this code gets reached.

+    }
+    animation_metrics_reporter_->Report(smoothness);
+  }
   if (!alive)
     return need_draw;
   last_progressed_fraction_ = 1.0;