ash: Switch to use CalculateSmoothnessV3

This CL changes to use CalculateSmoothnessV3 for ash smoothness metrics. Bug: 1513519 Change-Id: I305b21ede528d7e1ce99546c8d082859587062e5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5178793 Reviewed-by: Ahmed Fakhry <[email protected]> Reviewed-by: Alex Newcomer <[email protected]> Commit-Queue: Xiyuan Xia <[email protected]> Reviewed-by: Scott Violet <[email protected]> Code-Coverage: [email protected] <[email protected]> Cr-Commit-Position: refs/heads/main@{#1246715}
azx1573 · Jan 12, 2024 · 08a9390 · 08a9390
1 parent 244f42f
commit 08a9390
Show file tree

Hide file tree

Showing 41 changed files with 114 additions and 143 deletions.
diff --git a/ash/ambient/ui/ambient_animation_view.cc b/ash/ambient/ui/ambient_animation_view.cc
@@ -105,7 +105,7 @@ void OnCompositorThroughputReported(
           << " expected_fps=" << data.frames_expected / duration_sec
           << " actual_fps=" << data.frames_produced / duration_sec
           << " duration=" << duration;
-  metrics_util::ForSmoothness(
+  metrics_util::ForSmoothnessV3(
       base::BindRepeating(&LogCompositorThroughput, ui_settings))
       .Run(data);
 }

diff --git a/ash/ambient/ui/photo_view.cc b/ash/ambient/ui/photo_view.cc
@@ -139,7 +139,7 @@ void PhotoView::StartTransitionAnimation() {
 
     ui::AnimationThroughputReporter reporter(
         animation.GetAnimator(),
-        metrics_util::ForSmoothness(base::BindRepeating(ReportSmoothness)));
+        metrics_util::ForSmoothnessV3(base::BindRepeating(ReportSmoothness)));
 
     visible_layer->SetOpacity(0.0f);
   }
@@ -157,7 +157,7 @@ void PhotoView::StartTransitionAnimation() {
 
     ui::AnimationThroughputReporter reporter(
         animation.GetAnimator(),
-        metrics_util::ForSmoothness(base::BindRepeating(ReportSmoothness)));
+        metrics_util::ForSmoothnessV3(base::BindRepeating(ReportSmoothness)));
 
     invisible_layer->SetOpacity(1.0f);
   }

diff --git a/ash/app_list/app_list_controller_impl.cc b/ash/app_list/app_list_controller_impl.cc
@@ -1968,7 +1968,7 @@ void AppListControllerImpl::StartTrackingAnimationSmoothness(
   auto* compositor = root_window->layer()->GetCompositor();
   smoothness_tracker_ = compositor->RequestNewThroughputTracker();
   smoothness_tracker_->Start(
-      metrics_util::ForSmoothness(base::BindRepeating([](int smoothness) {
+      metrics_util::ForSmoothnessV3(base::BindRepeating([](int smoothness) {
         UMA_HISTOGRAM_PERCENTAGE(kHomescreenAnimationHistogram, smoothness);
       })));
 }

diff --git a/ash/app_list/app_list_presenter_impl.cc b/ash/app_list/app_list_presenter_impl.cc
@@ -475,7 +475,7 @@ void AppListPresenterImpl::UpdateScaleAndOpacityForHomeLauncher(
   if (settings.has_value() && transition.has_value()) {
     view_->OnTabletModeAnimationTransitionNotified(*transition);
     reporter.emplace(settings->GetAnimator(),
-                     metrics_util::ForSmoothness(
+                     metrics_util::ForSmoothnessV3(
                          view_->GetStateTransitionMetricsReportCallback()));
   }
 

diff --git a/ash/app_list/views/app_list_bubble_apps_page.cc b/ash/app_list/views/app_list_bubble_apps_page.cc
@@ -302,7 +302,7 @@ void AppListBubbleAppsPage::AnimateShowLauncher(bool is_side_shelf) {
   // handled in AppListBubbleView, so track overall smoothness here.
   ui::AnimationThroughputReporter reporter(
       scrollable_apps_grid_view_->layer()->GetAnimator(),
-      metrics_util::ForSmoothness(base::BindRepeating([](int value) {
+      metrics_util::ForSmoothnessV3(base::BindRepeating([](int value) {
         // This histogram name is used in Tast tests. Do not rename.
         base::UmaHistogramPercentage(
             "Apps.ClamshellLauncher.AnimationSmoothness.OpenAppsPage", value);
@@ -397,7 +397,7 @@ void AppListBubbleAppsPage::AnimateShowPage() {
 
   ui::AnimationThroughputReporter reporter(
       scroll_contents->layer()->GetAnimator(),
-      metrics_util::ForSmoothness(base::BindRepeating([](int value) {
+      metrics_util::ForSmoothnessV3(base::BindRepeating([](int value) {
         base::UmaHistogramPercentage(
             "Apps.ClamshellLauncher.AnimationSmoothness.ShowAppsPage", value);
       })));
@@ -464,7 +464,7 @@ void AppListBubbleAppsPage::AnimateHidePage() {
 
   ui::AnimationThroughputReporter reporter(
       scroll_contents->layer()->GetAnimator(),
-      metrics_util::ForSmoothness(base::BindRepeating([](int value) {
+      metrics_util::ForSmoothnessV3(base::BindRepeating([](int value) {
         base::UmaHistogramPercentage(
             "Apps.ClamshellLauncher.AnimationSmoothness.HideAppsPage", value);
       })));

diff --git a/ash/app_list/views/app_list_bubble_view.cc b/ash/app_list/views/app_list_bubble_view.cc
@@ -405,7 +405,7 @@ void AppListBubbleView::StartShowAnimation(bool is_side_shelf) {
 
   ui::AnimationThroughputReporter reporter(
       layer()->GetAnimator(),
-      metrics_util::ForSmoothness(base::BindRepeating([](int value) {
+      metrics_util::ForSmoothnessV3(base::BindRepeating([](int value) {
         base::UmaHistogramPercentage(
             "Apps.ClamshellLauncher.AnimationSmoothness.Open", value);
       })));
@@ -479,7 +479,7 @@ void AppListBubbleView::StartHideAnimation(
 
   ui::AnimationThroughputReporter reporter(
       layer()->GetAnimator(),
-      metrics_util::ForSmoothness(base::BindRepeating([](int value) {
+      metrics_util::ForSmoothnessV3(base::BindRepeating([](int value) {
         base::UmaHistogramPercentage(
             "Apps.ClamshellLauncher.AnimationSmoothness.Close", value);
       })));

diff --git a/ash/app_list/views/app_list_folder_view.cc b/ash/app_list/views/app_list_folder_view.cc
@@ -785,7 +785,7 @@ void AppListFolderView::ScheduleShowHideAnimation(bool show,
   show_hide_metrics_tracker_ =
       GetWidget()->GetCompositor()->RequestNewThroughputTracker();
   show_hide_metrics_tracker_->Start(
-      metrics_util::ForSmoothness(base::BindRepeating([](int smoothness) {
+      metrics_util::ForSmoothnessV3(base::BindRepeating([](int smoothness) {
         UMA_HISTOGRAM_PERCENTAGE(
             "Apps.AppListFolder.ShowHide.AnimationSmoothness", smoothness);
       })));

diff --git a/ash/app_list/views/apps_grid_view.cc b/ash/app_list/views/apps_grid_view.cc
@@ -1625,7 +1625,7 @@ void AppsGridView::AnimateToIdealBounds(bool is_animating_top_to_bottom) {
     item_reorder_animation_tracker_ =
         layer()->GetCompositor()->RequestNewThroughputTracker();
     item_reorder_animation_tracker_->Start(
-        metrics_util::ForSmoothness(base::BindRepeating(
+        metrics_util::ForSmoothnessV3(base::BindRepeating(
             &ReportItemDragReorderAnimationSmoothness, IsTabletMode())));
   }
 
@@ -2453,7 +2453,7 @@ views::AnimationBuilder AppsGridView::FadeOutVisibleItemsForReorder(
   grid_animation_status_ = AppListGridAnimationStatus::kReorderFadeOut;
   reorder_animation_tracker_.emplace(
       layer()->GetCompositor()->RequestNewThroughputTracker());
-  reorder_animation_tracker_->Start(metrics_util::ForSmoothness(
+  reorder_animation_tracker_->Start(metrics_util::ForSmoothnessV3(
       base::BindRepeating(&ReportReorderAnimationSmoothness, IsTabletMode())));
 
   views::AnimationBuilder animation_builder;

diff --git a/ash/app_list/views/assistant/assistant_page_view.cc b/ash/app_list/views/assistant/assistant_page_view.cc
@@ -278,7 +278,7 @@ void AssistantPageView::OnAnimationStarted(AppListState from_state,
 
     ui::AnimationThroughputReporter reporter(
         settings->GetAnimator(),
-        metrics_util::ForSmoothness(base::BindRepeating([](int value) {
+        metrics_util::ForSmoothnessV3(base::BindRepeating([](int value) {
           base::UmaHistogramPercentage(
               "Ash.Assistant.AnimationSmoothness.ResizeAssistantPageView",
               value);

diff --git a/ash/app_list/views/paged_apps_grid_view.cc b/ash/app_list/views/paged_apps_grid_view.cc
@@ -596,7 +596,7 @@ void PagedAppsGridView::TransitionStarted() {
 
   pagination_metrics_tracker_ =
       GetWidget()->GetCompositor()->RequestNewThroughputTracker();
-  pagination_metrics_tracker_->Start(metrics_util::ForSmoothness(
+  pagination_metrics_tracker_->Start(metrics_util::ForSmoothnessV3(
       base::BindRepeating(&ReportPaginationSmoothness)));
 }
 
@@ -835,7 +835,7 @@ void PagedAppsGridView::AnimateCardifiedState() {
   for (auto& background_card : background_cards_) {
     reporters.push_back(std::make_unique<ui::AnimationThroughputReporter>(
         background_card->layer()->GetAnimator(),
-        metrics_util::ForSmoothness(base::BindRepeating(
+        metrics_util::ForSmoothnessV3(base::BindRepeating(
             &ReportCardifiedSmoothness, cardified_state_))));
   }
 

diff --git a/ash/assistant/util/animation_util.cc b/ash/assistant/util/animation_util.cc
@@ -123,7 +123,7 @@ void StartLayerAnimationSequence(
 
   std::optional<ui::AnimationThroughputReporter> reporter;
   if (smoothness_callback) {
-    reporter.emplace(layer_animator, ash::metrics_util::ForSmoothness(
+    reporter.emplace(layer_animator, ash::metrics_util::ForSmoothnessV3(
                                          smoothness_callback.value()));
   }
   layer_animator->StartAnimation(layer_animation_sequence);

diff --git a/ash/metrics/login_unlock_throughput_recorder.cc b/ash/metrics/login_unlock_throughput_recorder.cc
@@ -109,8 +109,8 @@ void RecordDurationMetrics(
 
   int duration_ms = (base::TimeTicks::Now() - start).InMilliseconds();
   int smoothness, jank;
-  smoothness = metrics_util::CalculateSmoothness(data);
-  jank = metrics_util::CalculateJank(data);
+  smoothness = metrics_util::CalculateSmoothnessV3(data);
+  jank = metrics_util::CalculateJankV3(data);
 
   std::string suffix = GetDeviceModeSuffix();
   base::UmaHistogramPercentage(smoothness_name + suffix, smoothness);
@@ -163,7 +163,7 @@ void RecordSmoothnessMetrics(
     return;
   }
 
-  const int smoothness = metrics_util::CalculateSmoothness(data);
+  const int smoothness = metrics_util::CalculateSmoothnessV3(data);
 
   const std::string suffix = GetDeviceModeSuffix();
   base::UmaHistogramPercentage(smoothness_name + suffix, smoothness);

diff --git a/ash/public/cpp/metrics_util.cc b/ash/public/cpp/metrics_util.cc
@@ -45,7 +45,6 @@ void CollectDataAndForwardReport(
 // Calculates smoothness from |throughput| and sends to |callback|.
 void ForwardSmoothness(base::TimeTicks start_tick,
                        SmoothnessCallback callback,
-                       bool use_v3,
                        const cc::FrameSequenceMetrics::CustomReportData& data) {
   bool animation_in_test =
       ui::ScopedAnimationDurationScaleMode::duration_multiplier() !=
@@ -57,13 +56,9 @@ void ForwardSmoothness(base::TimeTicks start_tick,
   // *   frame_expected == 1 && frames_produced == 0
   //        when there is one BeginFrame but no frame generated between
   //        start and stop; should not included this case in tests.
-  if (use_v3 && (!data.frames_expected_v3 ||
-                 (!animation_in_test && data.frames_expected_v3 == 1 &&
-                  data.frames_expected_v3 == data.frames_dropped_v3))) {
-    return;
-  } else if (!data.frames_expected ||
-             (!animation_in_test && data.frames_expected == 1 &&
-              data.frames_produced == 0)) {
+  if (!data.frames_expected_v3 ||
+      (!animation_in_test && data.frames_expected_v3 == 1 &&
+       data.frames_expected_v3 == data.frames_dropped_v3)) {
     return;
   }
 
@@ -72,8 +67,7 @@ void ForwardSmoothness(base::TimeTicks start_tick,
   constexpr int kLowSmoothness = 20;
 
   const base::TimeDelta duration = base::TimeTicks::Now() - start_tick;
-  const int smoothness =
-      use_v3 ? CalculateSmoothnessV3(data) : CalculateSmoothness(data);
+  const int smoothness = CalculateSmoothnessV3(data);
 
   if (duration > kLongAnimation) {
     VLOG(1) << "Ash system animation takes longer than usual, duration= "
@@ -88,24 +82,11 @@ void ForwardSmoothness(base::TimeTicks start_tick,
 
 }  // namespace
 
-ReportCallback ForSmoothness(SmoothnessCallback callback,
-                             bool exclude_from_data_collection) {
-  const base::TimeTicks now = base::TimeTicks::Now();
-  auto forward_smoothness = base::BindRepeating(
-      &ForwardSmoothness, now, std::move(callback), /*use_v3=*/false);
-  if (!g_data_collection_enabled || exclude_from_data_collection) {
-    return forward_smoothness;
-  }
-
-  return base::BindRepeating(&CollectDataAndForwardReport, now,
-                             std::move(forward_smoothness));
-}
-
 ReportCallback ForSmoothnessV3(SmoothnessCallback callback,
                                bool exclude_from_data_collection) {
   const base::TimeTicks now = base::TimeTicks::Now();
-  auto forward_smoothness = base::BindRepeating(
-      &ForwardSmoothness, now, std::move(callback), /*use_v3=*/true);
+  auto forward_smoothness =
+      base::BindRepeating(&ForwardSmoothness, now, std::move(callback));
   if (!g_data_collection_enabled || exclude_from_data_collection)
     return forward_smoothness;
 
@@ -130,27 +111,16 @@ std::vector<AnimationData> GetCollectedData() {
   return data;
 }
 
-int CalculateSmoothness(
-    const cc::FrameSequenceMetrics::CustomReportData& data) {
-  DCHECK(data.frames_expected);
-  return std::floor(100.0f * data.frames_produced / data.frames_expected);
-}
-
 int CalculateSmoothnessV3(
     const cc::FrameSequenceMetrics::CustomReportData& data) {
-  DCHECK(data.frames_expected);
+  DCHECK(data.frames_expected_v3);
   return std::floor(100.0f *
                     (data.frames_expected_v3 - data.frames_dropped_v3) /
                     data.frames_expected_v3);
 }
 
-int CalculateJank(const cc::FrameSequenceMetrics::CustomReportData& data) {
-  DCHECK(data.frames_expected);
-  return std::floor(100.0f * data.jank_count / data.frames_expected);
-}
-
 int CalculateJankV3(const cc::FrameSequenceMetrics::CustomReportData& data) {
-  DCHECK(data.frames_expected);
+  DCHECK(data.frames_expected_v3);
   return std::floor(100.0f * data.jank_count_v3 / data.frames_expected_v3);
 }
 

diff --git a/ash/public/cpp/metrics_util.h b/ash/public/cpp/metrics_util.h
@@ -35,11 +35,6 @@ struct ASH_PUBLIC_EXPORT AnimationData {
 // cc::FrameSequenceMetrics::ThroughputData, calculates the smoothness
 // out of it and forward it to the smoothness report callback.
 ASH_PUBLIC_EXPORT ReportCallback
-ForSmoothness(SmoothnessCallback callback,
-              bool exclude_from_data_collection = false);
-
-// As ForSmoothness, but using the validated V3 graphics smoothness metrics.
-ASH_PUBLIC_EXPORT ReportCallback
 ForSmoothnessV3(SmoothnessCallback callback,
                 bool exclude_from_data_collection = false);
 
@@ -54,14 +49,10 @@ ASH_PUBLIC_EXPORT std::vector<AnimationData> StopDataCollection();
 ASH_PUBLIC_EXPORT std::vector<AnimationData> GetCollectedData();
 
 // Returns smoothness calculated from given data.
-ASH_PUBLIC_EXPORT int CalculateSmoothness(
-    const cc::FrameSequenceMetrics::CustomReportData& data);
 ASH_PUBLIC_EXPORT int CalculateSmoothnessV3(
     const cc::FrameSequenceMetrics::CustomReportData& data);
 
 // Returns jank percentage calculated from given data.
-ASH_PUBLIC_EXPORT int CalculateJank(
-    const cc::FrameSequenceMetrics::CustomReportData& data);
 ASH_PUBLIC_EXPORT int CalculateJankV3(
     const cc::FrameSequenceMetrics::CustomReportData& data);
 

diff --git a/ash/public/cpp/metrics_util_unittest.cc b/ash/public/cpp/metrics_util_unittest.cc
@@ -15,8 +15,8 @@ namespace metrics_util {
 
 TEST(MetricsUtilTest, ReportSmoothness) {
   cc::FrameSequenceMetrics::CustomReportData report_data;
-  report_data.frames_produced = 30;
-  report_data.frames_expected = 60;
+  report_data.frames_dropped_v3 = 30;
+  report_data.frames_expected_v3 = 60;
   constexpr int kExpectedSmoothes = 50;
 
   std::optional<int> reported_smoothness;
@@ -48,7 +48,7 @@ TEST(MetricsUtilTest, ReportSmoothness) {
 
     reported_smoothness.reset();
     ReportCallback report_callback =
-        ForSmoothness(smoothness_callback, test.exclude_from_collection);
+        ForSmoothnessV3(smoothness_callback, test.exclude_from_collection);
     report_callback.Run(report_data);
     ASSERT_TRUE(reported_smoothness.has_value());
     EXPECT_EQ(kExpectedSmoothes, reported_smoothness.value());

diff --git a/ash/rotator/screen_rotation_animator.cc b/ash/rotator/screen_rotation_animator.cc
@@ -440,7 +440,7 @@ void ScreenRotationAnimator::AnimateRotation(
   }
   ui::AnimationThroughputReporter reporter(
       old_layer_animator,
-      metrics_util::ForSmoothness(base::BindRepeating([](int smoothness) {
+      metrics_util::ForSmoothnessV3(base::BindRepeating([](int smoothness) {
         UMA_HISTOGRAM_PERCENTAGE(kRotationAnimationSmoothness, smoothness);
       })));
 

diff --git a/ash/shelf/hotseat_transition_animator.cc b/ash/shelf/hotseat_transition_animator.cc
@@ -144,7 +144,7 @@ void HotseatTransitionAnimator::DoAnimation(HotseatState old_state,
 
     ui::AnimationThroughputReporter reporter(
         shelf_bg_animation_setter.GetAnimator(),
-        metrics_util::ForSmoothness(
+        metrics_util::ForSmoothnessV3(
             base::BindRepeating(&ReportSmoothness, new_state)));
 
     shelf_widget_->GetAnimatingBackground()->SetTransform(transform);

diff --git a/ash/shelf/scrollable_shelf_view.cc b/ash/shelf/scrollable_shelf_view.cc
@@ -672,7 +672,7 @@ void ScrollableShelfView::StartShelfScrollAnimation(float scroll_distance) {
 
   ui::AnimationThroughputReporter reporter(
       animation_settings.GetAnimator(),
-      metrics_util::ForSmoothness(
+      metrics_util::ForSmoothnessV3(
           base::BindRepeating(&ReportSmoothness, Shell::Get()->IsInTabletMode(),
                               Shell::Get()->app_list_controller()->IsVisible(
                                   GetDisplayIdForView(this)))));

diff --git a/ash/shelf/shelf.cc b/ash/shelf/shelf.cc
@@ -136,7 +136,7 @@ class HotseatWidgetAnimationMetricsReporter {
 
   metrics_util::ReportCallback GetReportCallback(HotseatState target_state) {
     DCHECK_NE(target_state, HotseatState::kNone);
-    return metrics_util::ForSmoothness(base::BindRepeating(
+    return metrics_util::ForSmoothnessV3(base::BindRepeating(
         &HotseatWidgetAnimationMetricsReporter::ReportSmoothness,
         weak_ptr_factory_.GetWeakPtr(), target_state));
   }
@@ -191,7 +191,7 @@ class ASH_EXPORT NavigationWidgetAnimationMetricsReporter {
   metrics_util::ReportCallback GetReportCallback(
       HotseatState target_hotseat_state) {
     DCHECK_NE(target_hotseat_state, HotseatState::kNone);
-    return metrics_util::ForSmoothness(base::BindRepeating(
+    return metrics_util::ForSmoothnessV3(base::BindRepeating(
         &NavigationWidgetAnimationMetricsReporter::ReportSmoothness,
         weak_ptr_factory_.GetWeakPtr(), target_hotseat_state));
   }

diff --git a/ash/shelf/shelf_navigation_widget.cc b/ash/shelf/shelf_navigation_widget.cc
@@ -236,7 +236,7 @@ class ASH_EXPORT NavigationButtonAnimationMetricsReporter {
   metrics_util::ReportCallback GetReportCallback(
       HotseatState target_hotseat_state) {
     DCHECK_NE(target_hotseat_state, HotseatState::kNone);
-    return metrics_util::ForSmoothness(base::BindRepeating(
+    return metrics_util::ForSmoothnessV3(base::BindRepeating(
         &NavigationButtonAnimationMetricsReporter::ReportSmoothness,
         weak_ptr_factory_.GetWeakPtr(), target_hotseat_state));
   }

diff --git a/ash/shelf/shelf_view.cc b/ash/shelf/shelf_view.cc
@@ -1515,7 +1515,7 @@ void ShelfView::AnimateToIdealBounds() {
 
   move_animation_tracker_.emplace(
       GetWidget()->GetCompositor()->RequestNewThroughputTracker());
-  move_animation_tracker_->Start(metrics_util::ForSmoothness(
+  move_animation_tracker_->Start(metrics_util::ForSmoothnessV3(
       base::BindRepeating(&ReportMoveAnimationSmoothness)));
 
   for (size_t i = 0; i < view_model_->view_size(); ++i) {
@@ -1580,7 +1580,7 @@ void ShelfView::FadeIn(views::View* view) {
 
   ui::AnimationThroughputReporter reporter(
       fade_in_animation_settings.GetAnimator(),
-      metrics_util::ForSmoothness(
+      metrics_util::ForSmoothnessV3(
           base::BindRepeating(&ReportFadeInAnimationSmoothness)));
 
   view->layer()->SetOpacity(1.f);
@@ -2375,7 +2375,7 @@ void ShelfView::ShelfItemRemoved(int model_index, const ShelfItem& old_item) {
     if (!fade_out_animation_tracker_) {
       fade_out_animation_tracker_.emplace(
           GetWidget()->GetCompositor()->RequestNewThroughputTracker());
-      fade_out_animation_tracker_->Start(metrics_util::ForSmoothness(
+      fade_out_animation_tracker_->Start(metrics_util::ForSmoothnessV3(
           base::BindRepeating(&ReportFadeOutAnimationSmoothness)));
     }