[animations] Adds metrics for jank on selected layer animations

ui/views/animation/flood_fill_ink_drop_ripple.cc

Index: ui/views/animation/flood_fill_ink_drop_ripple.cc
diff --git a/ui/views/animation/flood_fill_ink_drop_ripple.cc b/ui/views/animation/flood_fill_ink_drop_ripple.cc
index e542679e98ed3ccb0b681adeef99315ab9a2e254..091093928158539ec8bf2967676779b7c41a528b 100644
--- a/ui/views/animation/flood_fill_ink_drop_ripple.cc
+++ b/ui/views/animation/flood_fill_ink_drop_ripple.cc
@@ -6,7 +6,9 @@
 
 #include <algorithm>
 
+#include "base/lazy_instance.h"
 #include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_animation_sequence.h"
@@ -113,6 +115,15 @@ float CalculateCircleLayerRadius(const gfx::Rect& clip_bounds) {
   return std::max(clip_bounds.width(), clip_bounds.height()) / 2.f;
 }
 
+class FloodRippleMetricsReporter : public ui::AnimationMetricsReporter {
+  void Report(int value) override {
+    UMA_HISTOGRAM_PERCENTAGE("Views.AnimationSmoothness.FloodFillRipple",

[bruthig, 2017/01/24 15:56:30]

I'm not sure how helpful it will be to get an aggregate of all flood fill
animations across all surfaces.  It would be much much better if they could be
named with the surface that they are on, e.g. AppListFloodFill.  IIUC it should
be possible, you just have to expand the histogram macros manually.

[varkha, 2017/01/24 20:23:25]

I thought of those more as examples to use the new metrics. I agree that
aggregating across all ripples is not the use case I had in mind; it would be
better to have more specific (and more problematic) surfaces: notification tray
ripple, app-list ripple and maybe one example in top chrome (chrome kebob menu
button or bookmarks). I will leave the ripple metrics instantiations out of this
CL.

+                             value);
+  }
+};
+base::LazyInstance<FloodRippleMetricsReporter>::Leaky g_reporter =

[bruthig, 2017/01/24 15:56:30]

Do these really need to be global state?  Would any of the following work?

. Make FloodFillInkDropRipple inherit ui::AnimationMetricsReporter
. Use scoped_refptr's instead
. Use normal instances and pass them by copy

FloodRippleMetricsReporter may be a small class but if this is the de facto
standard than I imagine others will be adding a lot of global state to track
animations.  I don't think eating up that RAM is worthy for something that is
only useful for a small portion of the time.

[varkha, 2017/01/24 20:23:25]

Chatted offline about it. I think so far with a very few actual histograms added
simplicity would win (and avoid some non-trivial instantiation and
reconstitution costs for the histogram objects). I will leave the ripple metrics
out of this CL for now.

+    LAZY_INSTANCE_INITIALIZER;
+
 }  // namespace
 
 namespace views {
@@ -327,6 +338,7 @@ void FloodFillInkDropRipple::AnimateToTransform(
   if (animation_observer)
     sequence->AddObserver(animation_observer);
 
+  sequence->SetAnimationMetricsReporter(g_reporter.Pointer());
   animator->StartAnimation(sequence);
 }
 
@@ -348,6 +360,7 @@ void FloodFillInkDropRipple::PauseTransformAnimation(
   if (observer)
     sequence->AddObserver(observer);
 
+  sequence->SetAnimationMetricsReporter(g_reporter.Pointer());
   animator->StartAnimation(sequence);
 }
 
@@ -373,6 +386,7 @@ void FloodFillInkDropRipple::AnimateToOpacity(
   if (animation_observer)
     animation_sequence->AddObserver(animation_observer);
 
+  animation_sequence->SetAnimationMetricsReporter(g_reporter.Pointer());
   animator->StartAnimation(animation_sequence);
 }
 
@@ -394,6 +408,7 @@ void FloodFillInkDropRipple::PauseOpacityAnimation(
   if (observer)
     sequence->AddObserver(observer);
 
+  sequence->SetAnimationMetricsReporter(g_reporter.Pointer());
   animator->StartAnimation(sequence);
 }