Reland app_list: Use AnimationThroughputReporter for animation metrics

Reland + fix test flake

> This wires AnimationThroughputReporter for state transition animation
> metrics.
>
> Bug: 1021774
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2240411
> Reviewed-by: Jun Mukai <mukai@chromium.org>

Bug: 1021774, 1094441
Change-Id: I2e75502e8a8024fbf6e95a84afbf391d22a4df4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2246955Reviewed-by: Andrew Xu <andrewxu@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#778558}

ash/app_list/app_list_presenter_impl.cc

@@ -15,6 +15,7 @@
 #include "ash/public/cpp/app_list/app_list_features.h"
 #include "ash/public/cpp/app_list/app_list_switches.h"
 #include "ash/public/cpp/app_list/app_list_types.h"
+#include "ash/public/cpp/metrics_util.h"
 #include "ash/public/cpp/pagination/pagination_model.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "base/bind.h"
@@ -25,6 +26,7 @@
 #include "base/optional.h"
 #include "ui/aura/client/focus_client.h"
 #include "ui/aura/window.h"
+#include "ui/compositor/animation_throughput_reporter.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/display/screen.h"
@@ -312,10 +314,12 @@ void AppListPresenterImpl::UpdateYPositionAndOpacityForHomeLauncher(
   // reported for transform animation only.
   layer->SetOpacity(opacity);
 
+  base::Optional<ui::AnimationThroughputReporter> reporter;
   if (settings.has_value() && transition.has_value()) {
     view_->OnTabletModeAnimationTransitionNotified(transition.value());
-    settings->SetAnimationMetricsReporter(
-        view_->GetStateTransitionMetricsReporter());
+    reporter.emplace(settings->GetAnimator(),
+                     metrics_util::ForSmoothness(
+                         view_->GetStateTransitionMetricsReportCallback()));
   }
 
   layer->SetTransform(translation);
@@ -369,10 +373,12 @@ void AppListPresenterImpl::UpdateScaleAndOpacityForHomeLauncher(
   // reported for transform animation only.
   layer->SetOpacity(opacity);
 
+  base::Optional<ui::AnimationThroughputReporter> reporter;
   if (settings.has_value() && transition.has_value()) {
     view_->OnTabletModeAnimationTransitionNotified(*transition);
-    settings->SetAnimationMetricsReporter(
-        view_->GetStateTransitionMetricsReporter());
+    reporter.emplace(settings->GetAnimator(),
+                     metrics_util::ForSmoothness(
+                         view_->GetStateTransitionMetricsReportCallback()));
   }
 
   gfx::Transform transform =

ash/app_list/views/app_list_view.cc

@@ -23,6 +23,7 @@
 #include "ash/public/cpp/app_list/app_list_features.h"
 #include "ash/public/cpp/app_list/app_list_types.h"
 #include "ash/public/cpp/ash_features.h"
+#include "ash/public/cpp/metrics_util.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/public/cpp/wallpaper_types.h"
 #include "base/macros.h"
@@ -37,7 +38,7 @@
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/ui_base_switches.h"
-#include "ui/compositor/animation_metrics_reporter.h"
+#include "ui/compositor/animation_throughput_reporter.h"
 #include "ui/compositor/layer.h"
 #include "ui/compositor/layer_animation_element.h"
 #include "ui/compositor/layer_animation_observer.h"
@@ -251,65 +252,63 @@ float ComputeSubpixelOffset(const display::Display& display, float value) {
 ////////////////////////////////////////////////////////////////////////////////
 // AppListView::StateAnimationMetricsReporter
 
-class AppListView::StateAnimationMetricsReporter
-    : public ui::AnimationMetricsReporter {
+class AppListView::StateAnimationMetricsReporter {
  public:
   explicit StateAnimationMetricsReporter(AppListView* view) : view_(view) {}
+  StateAnimationMetricsReporter(const StateAnimationMetricsReporter&) = delete;
+  StateAnimationMetricsReporter& operator=(
+      const StateAnimationMetricsReporter&) = delete;
+  ~StateAnimationMetricsReporter() = default;
 
-  ~StateAnimationMetricsReporter() override = default;
-
+  // Sets target state of the transition for metrics.
   void SetTargetState(AppListViewState target_state) {
     target_state_ = target_state;
   }
 
-  void Start(bool is_in_tablet_mode) {
-    is_in_tablet_mode_ = is_in_tablet_mode;
-#if defined(DCHECK)
-    DCHECK(!started_);
-    started_ = ui::ScopedAnimationDurationScaleMode::duration_scale_mode() !=
-               ui::ScopedAnimationDurationScaleMode::ZERO_DURATION;
-#endif
-  }
-
-  void Reset();
-
+  // Sets tablet animation transition type for metrics.
   void SetTabletModeAnimationTransition(
       TabletModeAnimationTransition transition) {
     tablet_transition_ = transition;
   }
 
-  // ui::AnimationMetricsReporter:
-  void Report(int value) override;
+  // Resets the target state and animation type for metrics.
+  void Reset();
+
+  // Gets a callback to report smoothness.
+  metrics_util::SmoothnessCallback GetReportCallback(bool tablet_mode) {
+    return base::BindRepeating(&StateAnimationMetricsReporter::Report,
+                               weak_ptr_factory_.GetWeakPtr(), tablet_mode);
+  }
 
  private:
+  // Reports smoothness.
+  void Report(bool tablet_mode, int smoothess);
+
   void RecordMetricsInTablet(int value);
   void RecordMetricsInClamshell(int value);
 
-#if defined(DCHECK)
-  bool started_ = false;
-#endif
   base::Optional<AppListViewState> target_state_;
   base::Optional<TabletModeAnimationTransition> tablet_transition_;
-  bool is_in_tablet_mode_ = false;
   AppListView* view_;
 
-  DISALLOW_COPY_AND_ASSIGN(StateAnimationMetricsReporter);
+  base::WeakPtrFactory<StateAnimationMetricsReporter> weak_ptr_factory_{this};
 };
 
 void AppListView::StateAnimationMetricsReporter::Reset() {
-#if defined(DCHECK)
-  started_ = false;
-#endif
   tablet_transition_.reset();
   target_state_.reset();
 }
 
-void AppListView::StateAnimationMetricsReporter::Report(int value) {
-  UMA_HISTOGRAM_PERCENTAGE("Apps.StateTransition.AnimationSmoothness", value);
-  if (is_in_tablet_mode_)
-    RecordMetricsInTablet(value);
+void AppListView::StateAnimationMetricsReporter::Report(bool tablet_mode,
+                                                        int smoothess) {
+  UMA_HISTOGRAM_PERCENTAGE("Apps.StateTransition.AnimationSmoothness",
+                           smoothess);
+
+  if (tablet_mode)
+    RecordMetricsInTablet(smoothess);
   else
-    RecordMetricsInClamshell(value);
+    RecordMetricsInClamshell(smoothess);
+
   view_->OnStateTransitionAnimationCompleted();
   Reset();
 }
@@ -1729,7 +1728,9 @@ void AppListView::ApplyBoundsAnimation(AppListViewState target_state,
   ui::ScopedLayerAnimationSettings animation(layer->GetAnimator());
   animation.SetPreemptionStrategy(
       ui::LayerAnimator::IMMEDIATELY_SET_NEW_TARGET);
-  animation.SetAnimationMetricsReporter(GetStateTransitionMetricsReporter());
+  ui::AnimationThroughputReporter reporter(
+      animation.GetAnimator(),
+      metrics_util::ForSmoothness(GetStateTransitionMetricsReportCallback()));
   TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("ui", "AppList::StateTransitionAnimations",
                                     bounds_animation_observer_.get());
   bounds_animation_observer_->set_target_state(target_state);
@@ -2030,9 +2031,10 @@ AppListViewState AppListView::CalculateStateAfterShelfDrag(
   return app_list_state;
 }
 
-ui::AnimationMetricsReporter* AppListView::GetStateTransitionMetricsReporter() {
-  state_animation_metrics_reporter_->Start(delegate_->IsInTabletMode());
-  return state_animation_metrics_reporter_.get();
+metrics_util::SmoothnessCallback
+AppListView::GetStateTransitionMetricsReportCallback() {
+  return state_animation_metrics_reporter_->GetReportCallback(
+      delegate_->IsInTabletMode());
 }
 
 void AppListView::OnHomeLauncherDragStart() {

ash/app_list/views/app_list_view.h

@@ -13,6 +13,7 @@
 #include "ash/app_list/app_list_metrics.h"
 #include "ash/app_list/app_list_view_delegate.h"
 #include "ash/public/cpp/app_list/app_list_types.h"
+#include "ash/public/cpp/metrics_util.h"
 #include "ash/public/cpp/presentation_time_recorder.h"
 #include "base/callback.h"
 #include "base/macros.h"
@@ -32,7 +33,6 @@ class Display;
 }
 
 namespace ui {
-class AnimationMetricsReporter;
 class ImplicitAnimationObserver;
 }  // namespace ui
 
@@ -298,8 +298,8 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
       const ui::LocatedEvent& event_in_screen,
       float launcher_above_shelf_bottom_amount) const;
 
-  // Returns a animation metrics reportre for state transition.
-  ui::AnimationMetricsReporter* GetStateTransitionMetricsReporter();
+  // Returns a animation metrics reporting callback  for state transition.
+  metrics_util::SmoothnessCallback GetStateTransitionMetricsReportCallback();
 
   // Called when drag in tablet mode starts/proceeds/ends.
   void OnHomeLauncherDragStart();

ash/home_screen/swipe_home_to_overview_controller_unittest.cc

@@ -10,6 +10,7 @@
 #include "ash/home_screen/home_screen_controller.h"
 #include "ash/home_screen/home_screen_delegate.h"
 #include "ash/public/cpp/ash_features.h"
+#include "ash/root_window_controller.h"
 #include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_metrics.h"
 #include "ash/shelf/test/overview_animation_waiter.h"
@@ -17,12 +18,14 @@
 #include "ash/test/ash_test_base.h"
 #include "ash/wm/overview/overview_controller.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller_test_api.h"
+#include "base/macros.h"
 #include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
+#include "ui/compositor/test/test_utils.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/gfx/geometry/rect_f.h"
 
@@ -98,6 +101,8 @@ class SwipeHomeToOverviewControllerTest : public AshTestBase {
   }
 
   void WaitForHomeLauncherAnimationToFinish() {
+    auto* compositor =
+        Shell::GetPrimaryRootWindowController()->GetHost()->compositor();
     // Wait until home launcher animation finishes.
     while (GetAppListTestHelper()
                ->GetAppListView()
@@ -105,12 +110,13 @@ class SwipeHomeToOverviewControllerTest : public AshTestBase {
                ->GetLayer()
                ->GetAnimator()
                ->is_animating()) {
-      base::RunLoop run_loop;
-      base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-          FROM_HERE, run_loop.QuitClosure(),
-          base::TimeDelta::FromMilliseconds(200));
-      run_loop.Run();
+      EXPECT_TRUE(ui::WaitForNextFrameToBePresented(compositor));
     }
+
+    // Ensure there is one more frame presented after animation finishes
+    // to allow animation throughput data is passed from cc to ui.
+    ignore_result(ui::WaitForNextFrameToBePresented(
+        compositor, base::TimeDelta::FromMilliseconds(200)));
   }
 
   void TapOnHomeLauncherSearchBox() {
@@ -142,8 +148,7 @@ class SwipeHomeToOverviewControllerTest : public AshTestBase {
 
 // Verify that the metrics of home launcher animation are recorded correctly
 // when entering/exiting overview mode.
-// Disabled due to flake, see crbug.com/1094441 .
-TEST_F(SwipeHomeToOverviewControllerTest, DISABLED_VerifyHomeLauncherMetrics) {
+TEST_F(SwipeHomeToOverviewControllerTest, VerifyHomeLauncherMetrics) {
   // Set non-zero animation duration to report animation metrics.
   ui::ScopedAnimationDurationScaleMode non_zero_duration_mode(
       ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);