views: Make bounds animator compatible with metrics reporter switches.

This is required as the shelf wants to use the same bounds animator to
record multiple histograms. See crrev.com/c/2105675 for a usage.

Test: added a tests
Bug: 1051490
Change-Id: Ie749ddd254ac8ae5e66229ec4b64cf86b5baa542
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2128373Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755130}

ash/shelf/shelf_navigation_widget.cc

@@ -620,7 +620,7 @@ void ShelfNavigationWidget::UpdateLayout(bool animate) {
 
   if (animate) {
     bounds_animator_->SetAnimationDuration(animation_duration);
-    bounds_animator_->set_animation_metrics_reporter(
+    bounds_animator_->SetAnimationMetricsReporter(
         home_button_metrics_reporter_.get());
     bounds_animator_->AnimateViewTo(home_button, home_button_bounds);
   } else {

ui/views/animation/animation_delegate_views.cc

@@ -53,6 +53,7 @@ void AnimationDelegateViews::OnViewIsDeleting(View* observed_view) {
 void AnimationDelegateViews::AnimationContainerShuttingDown(
     gfx::AnimationContainer* container) {
   container_ = nullptr;
+  compositor_animation_runner_ = nullptr;
 }
 
 base::TimeDelta AnimationDelegateViews::GetAnimationDurationForReporting()
@@ -60,6 +61,20 @@ base::TimeDelta AnimationDelegateViews::GetAnimationDurationForReporting()
   return base::TimeDelta();
 }
 
+void AnimationDelegateViews::SetAnimationMetricsReporter(
+    ui::AnimationMetricsReporter* animation_metrics_reporter) {
+  if (animation_metrics_reporter_ == animation_metrics_reporter)
+    return;
+
+  animation_metrics_reporter_ = animation_metrics_reporter;
+
+  if (!compositor_animation_runner_)
+    return;
+
+  compositor_animation_runner_->SetAnimationMetricsReporter(
+      animation_metrics_reporter_, GetAnimationDurationForReporting());
+}
+
 void AnimationDelegateViews::UpdateAnimationRunner() {
   if (!container_)
     return;
@@ -68,15 +83,19 @@ void AnimationDelegateViews::UpdateAnimationRunner() {
     // TODO(https://crbug.com/960621): make sure the container has a correct
     // compositor-assisted runner.
     container_->SetAnimationRunner(nullptr);
+    compositor_animation_runner_ = nullptr;
     return;
   }
 
   if (container_->has_custom_animation_runner())
     return;
 
-  container_->SetAnimationRunner(std::make_unique<CompositorAnimationRunner>(
-      view_->GetWidget(), animation_metrics_reporter_,
-      GetAnimationDurationForReporting()));
+  auto compositor_animation_runner =
+      std::make_unique<CompositorAnimationRunner>(view_->GetWidget());
+  compositor_animation_runner_ = compositor_animation_runner.get();
+  compositor_animation_runner_->SetAnimationMetricsReporter(
+      animation_metrics_reporter_, GetAnimationDurationForReporting());
+  container_->SetAnimationRunner(std::move(compositor_animation_runner));
 }
 
 }  // namespace views

ui/views/animation/animation_delegate_views.h

@@ -19,6 +19,7 @@ class AnimationMetricsReporter;
 }
 
 namespace views {
+class CompositorAnimationRunner;
 
 // Provides default implementation to adapt CompositorAnimationRunner for
 // Animation. Falls back to the default animation runner when |view| is nullptr.
@@ -50,12 +51,10 @@ class VIEWS_EXPORT AnimationDelegateViews
   // |set_animation_metrics_reporter()|.
   virtual base::TimeDelta GetAnimationDurationForReporting() const;
 
-  gfx::AnimationContainer* container() { return container_; }
+  void SetAnimationMetricsReporter(
+      ui::AnimationMetricsReporter* animation_metrics_reporter);
 
-  void set_animation_metrics_reporter(
-      ui::AnimationMetricsReporter* animation_metrics_reporter) {
-    animation_metrics_reporter_ = animation_metrics_reporter;
-  }
+  gfx::AnimationContainer* container() { return container_; }
 
  private:
   // Sets CompositorAnimationRunner to |container_| if possible. Otherwise,
@@ -67,6 +66,9 @@ class VIEWS_EXPORT AnimationDelegateViews
 
   ui::AnimationMetricsReporter* animation_metrics_reporter_ = nullptr;
 
+  // The animation runner that |container_| uses.
+  CompositorAnimationRunner* compositor_animation_runner_ = nullptr;
+
   ScopedObserver<View, ViewObserver> scoped_observer_{this};
 };
 

ui/views/animation/compositor_animation_runner.cc

@@ -13,18 +13,9 @@ namespace views {
 // CompositorAnimationRunner
 //
 
-CompositorAnimationRunner::CompositorAnimationRunner(
-    Widget* widget,
-    ui::AnimationMetricsReporter* animation_metrics_reporter,
-    base::TimeDelta expected_duration)
-    : widget_(widget), expected_duration_(expected_duration) {
+CompositorAnimationRunner::CompositorAnimationRunner(Widget* widget)
+    : widget_(widget) {
   widget_->AddObserver(this);
-  if (animation_metrics_reporter) {
-    DCHECK(!expected_duration_.is_zero());
-    animation_metrics_recorder_ =
-        std::make_unique<ui::AnimationMetricsRecorder>(
-            animation_metrics_reporter);
-  }
 }
 
 CompositorAnimationRunner::~CompositorAnimationRunner() {
@@ -34,6 +25,20 @@ CompositorAnimationRunner::~CompositorAnimationRunner() {
   DCHECK(!compositor_ || !compositor_->HasAnimationObserver(this));
 }
 
+void CompositorAnimationRunner::SetAnimationMetricsReporter(
+    ui::AnimationMetricsReporter* animation_metrics_reporter,
+    base::TimeDelta expected_duration) {
+  if (animation_metrics_reporter) {
+    DCHECK(!expected_duration.is_zero());
+    animation_metrics_recorder_ =
+        std::make_unique<ui::AnimationMetricsRecorder>(
+            animation_metrics_reporter);
+    expected_duration_ = expected_duration;
+  } else {
+    animation_metrics_recorder_.reset();
+  }
+}
+
 void CompositorAnimationRunner::Stop() {
   // Record metrics if necessary.
   if (animation_metrics_recorder_ && compositor_) {

ui/views/animation/compositor_animation_runner.h

@@ -28,17 +28,18 @@ class CompositorAnimationRunner : public gfx::AnimationRunner,
                                   public ui::CompositorAnimationObserver,
                                   public WidgetObserver {
  public:
-  // Callers can also pass a AnimationMetricsReporter which will record metrics
-  // after an animation is complete. A non zero |expected_duration| is required
-  // for computations.
-  explicit CompositorAnimationRunner(
-      Widget* widget,
-      ui::AnimationMetricsReporter* animation_metrics_reporter = nullptr,
-      base::TimeDelta expected_duration = base::TimeDelta());
+  explicit CompositorAnimationRunner(Widget* widget);
   CompositorAnimationRunner(CompositorAnimationRunner&) = delete;
   CompositorAnimationRunner& operator=(CompositorAnimationRunner&) = delete;
   ~CompositorAnimationRunner() override;
 
+  // Set a AnimationMetricsReporter which will record metrics
+  // after an animation is complete. A non zero |expected_duration| is required
+  // for computations.
+  void SetAnimationMetricsReporter(
+      ui::AnimationMetricsReporter* animation_metrics_reporter,
+      base::TimeDelta expected_duration);
+
   // gfx::AnimationRunner:
   void Stop() override;
 
@@ -72,7 +73,7 @@ class CompositorAnimationRunner : public gfx::AnimationRunner,
 
   // Expected duration of an animation. Used for and required to be non zero for
   // animation metrics recording.
-  const base::TimeDelta expected_duration_;
+  base::TimeDelta expected_duration_;
   std::unique_ptr<ui::AnimationMetricsRecorder> animation_metrics_recorder_;
 };
 

ui/views/animation/compositor_animation_runner_unittest.cc

@@ -93,8 +93,9 @@ TEST_F(CompositorAnimationRunnerTest, AnimationMetricsReporter) {
   ui::DrawWaiterForTest::WaitForCompositingStarted(widget->GetCompositor());
 
   TestAnimationMetricsReporter metrics_reporter;
+  TestAnimationMetricsReporter metrics_reporter2;
   TestAnimationDelegateViews delegate(widget->GetContentsView());
-  delegate.set_animation_metrics_reporter(&metrics_reporter);
+  delegate.SetAnimationMetricsReporter(&metrics_reporter);
   gfx::LinearAnimation animation(
       kDuration, gfx::LinearAnimation::kDefaultFrameRate, &delegate);
 
@@ -114,6 +115,25 @@ TEST_F(CompositorAnimationRunnerTest, AnimationMetricsReporter) {
                        }));
   run_loop.Run();
   EXPECT_EQ(1, metrics_reporter.report_count());
+  EXPECT_EQ(0, metrics_reporter2.report_count());
+
+  // Tests that switching metrics reporters for the next animation works as
+  // expected.
+  base::RunLoop run_loop2;
+  delegate.SetAnimationMetricsReporter(&metrics_reporter2);
+  animation.Start();
+  EXPECT_TRUE(animation.is_animating());
+
+  interval_timer.Start(FROM_HERE, kDuration, base::BindLambdaForTesting([&]() {
+                         if (animation.is_animating())
+                           return;
+
+                         interval_timer.Stop();
+                         run_loop2.Quit();
+                       }));
+  run_loop2.Run();
+  EXPECT_EQ(1, metrics_reporter.report_count());
+  EXPECT_EQ(1, metrics_reporter2.report_count());
 }
 
 // No DesktopAura on ChromeOS.