ash: Use ThroughputTracker for tablet transition smoothness

Bug: 1021774
Change-Id: Ie782cf0c74e51f04fab379983ecffe9e21ae4750
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2268499Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#783020}

ash/wm/tablet_mode/tablet_mode_controller.cc

@@ -9,7 +9,7 @@
 #include <utility>
 
 #include "ash/public/cpp/ash_switches.h"
-#include "ash/public/cpp/fps_counter.h"
+#include "ash/public/cpp/metrics_util.h"
 #include "ash/public/cpp/shelf_config.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/public/cpp/tablet_mode.h"
@@ -292,34 +292,14 @@ const char* ToString(TabletMode tablet_mode) {
   return "";
 }
 
-}  // namespace
-
-// Class which records animation smoothness when entering or exiting tablet
-// mode. No stats should be recorded if no windows are animated.
-class TabletModeController::TabletModeTransitionFpsCounter : public FpsCounter {
- public:
-  TabletModeTransitionFpsCounter(ui::Compositor* compositor,
-                                 bool enter_tablet_mode)
-      : FpsCounter(compositor), enter_tablet_mode_(enter_tablet_mode) {}
-  ~TabletModeTransitionFpsCounter() override = default;
-
-  void LogUma() {
-    int smoothness = ComputeSmoothness();
-    if (smoothness < 0)
-      return;
-
-    if (enter_tablet_mode_)
-      UMA_HISTOGRAM_PERCENTAGE(kTabletModeEnterHistogram, smoothness);
-    else
-      UMA_HISTOGRAM_PERCENTAGE(kTabletModeExitHistogram, smoothness);
-  }
-
-  bool enter_tablet_mode() const { return enter_tablet_mode_; }
+void ReportTrasitionSmoothness(bool enter_tablet_mode, int smoothness) {
+  if (enter_tablet_mode)
+    UMA_HISTOGRAM_PERCENTAGE(kTabletModeEnterHistogram, smoothness);
+  else
+    UMA_HISTOGRAM_PERCENTAGE(kTabletModeExitHistogram, smoothness);
+}
 
- private:
-  bool enter_tablet_mode_;
-  DISALLOW_COPY_AND_ASSIGN(TabletModeTransitionFpsCounter);
-};
+}  // namespace
 
 // An observer that observes the destruction of the |window_| and executes the
 // callback. Used to run cleanup when the window is destroyed in the middle of
@@ -523,9 +503,9 @@ void TabletModeController::StopObservingAnimation(bool record_stats,
     }
   }
 
-  if (record_stats && fps_counter_)
-    fps_counter_->LogUma();
-  fps_counter_.reset();
+  if (record_stats && transition_tracker_)
+    transition_tracker_->Stop();
+  transition_tracker_.reset();
 
   // Stop other animations (STEP_END), then update the tablet mode ui.
   if (tablet_mode_window_manager_ && delete_screenshot)
@@ -757,7 +737,7 @@ void TabletModeController::OnLayerAnimationStarted(
 
 void TabletModeController::OnLayerAnimationAborted(
     ui::LayerAnimationSequence* sequence) {
-  if (!fps_counter_ || !ShouldObserveSequence(sequence))
+  if (!transition_tracker_ || !ShouldObserveSequence(sequence))
     return;
 
   StopObservingAnimation(/*record_stats=*/false, /*delete_screenshot=*/true);
@@ -768,9 +748,9 @@ void TabletModeController::OnLayerAnimationEnded(
   // This may be called before |OnLayerAnimationScheduled()| if tablet is
   // entered/exited while an animation is in progress, so we won't get
   // stats/screenshot in those cases.
-  // TODO(sammiequon): We may want to remove the |fps_counter_| check and
+  // TODO(sammiequon): We may want to remove the |transition_tracker_| check and
   // simplify things since those are edge cases.
-  if (!fps_counter_ || !ShouldObserveSequence(sequence))
+  if (!transition_tracker_ || !ShouldObserveSequence(sequence))
     return;
 
   StopObservingAnimation(/*record_stats=*/true, /*delete_screenshot=*/true);
@@ -781,10 +761,11 @@ void TabletModeController::OnLayerAnimationScheduled(
   if (!ShouldObserveSequence(sequence))
     return;
 
-  if (!fps_counter_) {
-    fps_counter_ = std::make_unique<TabletModeTransitionFpsCounter>(
-        animating_layer_->GetCompositor(),
-        state_ == State::kEnteringTabletMode);
+  if (!transition_tracker_) {
+    transition_tracker_ =
+        animating_layer_->GetCompositor()->RequestNewThroughputTracker();
+    transition_tracker_->Start(metrics_util::ForSmoothness(base::BindRepeating(
+        &ReportTrasitionSmoothness, state_ == State::kEnteringTabletMode)));
     return;
   }
 

ash/wm/tablet_mode/tablet_mode_controller.h

@@ -28,6 +28,7 @@
 #include "ui/compositor/layer_animation_observer.h"
 #include "ui/compositor/layer_observer.h"
 #include "ui/compositor/layer_tree_owner.h"
+#include "ui/compositor/throughput_tracker.h"
 #include "ui/events/devices/input_device_event_observer.h"
 #include "ui/gfx/geometry/vector3d_f.h"
 
@@ -205,7 +206,6 @@ class ASH_EXPORT TabletModeController
 
  private:
   class DestroyObserver;
-  class TabletModeTransitionFpsCounter;
   class ScopedShelfHider;
   friend class TabletModeControllerTestApi;
 
@@ -449,7 +449,8 @@ class ASH_EXPORT TabletModeController
   // does not show the old version of shelf in the background).
   std::unique_ptr<ScopedShelfHider> shelf_hider_;
 
-  std::unique_ptr<TabletModeTransitionFpsCounter> fps_counter_;
+  // Tracks and record transition smoothness.
+  base::Optional<ui::ThroughputTracker> transition_tracker_;
 
   base::CancelableOnceCallback<void(std::unique_ptr<viz::CopyOutputResult>)>
       screenshot_taken_callback_;

ash/wm/tablet_mode/tablet_mode_controller_unittest.cc

@@ -18,7 +18,6 @@
 #include "ash/public/cpp/app_types.h"
 #include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/ash_switches.h"
-#include "ash/public/cpp/fps_counter.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/public/cpp/tablet_mode.h"
 #include "ash/public/cpp/test/shell_test_api.h"
@@ -49,6 +48,7 @@
 #include "ui/base/hit_test.h"
 #include "ui/compositor/layer_animator.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
+#include "ui/compositor/test/test_utils.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/screen.h"
 #include "ui/display/test/display_manager_test_api.h"
@@ -111,7 +111,6 @@ class TabletModeControllerTest : public MultiDisplayOverviewAndSplitViewTest {
     MultiDisplayOverviewAndSplitViewTest::SetUp();
     AccelerometerReader::GetInstance()->RemoveObserver(
         tablet_mode_controller());
-    FpsCounter::SetForceReportZeroAnimationForTest(true);
 
     // Set the first display to be the internal display for the accelerometer
     // screen rotation tests.
@@ -129,7 +128,6 @@ class TabletModeControllerTest : public MultiDisplayOverviewAndSplitViewTest {
   }
 
   void TearDown() override {
-    FpsCounter::SetForceReportZeroAnimationForTest(false);
     AccelerometerReader::GetInstance()->AddObserver(tablet_mode_controller());
     MultiDisplayOverviewAndSplitViewTest::TearDown();
   }
@@ -214,6 +212,23 @@ class TabletModeControllerTest : public MultiDisplayOverviewAndSplitViewTest {
     return window;
   }
 
+  // Waits for |window|'s animation to finish.
+  void WaitForWindowAnimation(aura::Window* window) {
+    auto* compositor = window->layer()->GetCompositor();
+
+    while (window->layer()->GetAnimator()->is_animating())
+      EXPECT_TRUE(ui::WaitForNextFrameToBePresented(compositor));
+  }
+
+  // Wait one more frame presented for the metrics to get recorded.
+  // ignore_result() and timeout is because the frame could already be
+  // presented.
+  void WaitForSmoothnessMetrics() {
+    ignore_result(ui::WaitForNextFrameToBePresented(
+        Shell::GetPrimaryRootWindow()->layer()->GetCompositor(),
+        base::TimeDelta::FromMilliseconds(100)));
+  }
+
  private:
   std::unique_ptr<TabletModeControllerTestApi> test_api_;
 
@@ -1530,6 +1545,7 @@ TEST_P(TabletModeControllerTest, TabletModeTransitionHistogramsNotLogged) {
     histogram_tester.ExpectTotalCount(kExitHistogram, 0);
     tablet_mode_controller()->SetEnabledForTest(true);
     tablet_mode_controller()->SetEnabledForTest(false);
+    WaitForSmoothnessMetrics();
     histogram_tester.ExpectTotalCount(kEnterHistogram, 0);
     histogram_tester.ExpectTotalCount(kExitHistogram, 0);
   }
@@ -1549,10 +1565,12 @@ TEST_P(TabletModeControllerTest, TabletModeTransitionHistogramsNotLogged) {
     window->layer()->GetAnimator()->StopAnimating();
     tablet_mode_controller()->SetEnabledForTest(true);
     EXPECT_FALSE(window->layer()->GetAnimator()->is_animating());
+    WaitForSmoothnessMetrics();
     histogram_tester.ExpectTotalCount(kEnterHistogram, 0);
     histogram_tester.ExpectTotalCount(kExitHistogram, 0);
     tablet_mode_controller()->SetEnabledForTest(false);
     EXPECT_FALSE(window->layer()->GetAnimator()->is_animating());
+    WaitForSmoothnessMetrics();
     histogram_tester.ExpectTotalCount(kEnterHistogram, 0);
     histogram_tester.ExpectTotalCount(kExitHistogram, 0);
   }
@@ -1573,8 +1591,9 @@ TEST_P(TabletModeControllerTest, TabletModeTransitionHistogramsLogged) {
   tablet_mode_controller()->SetEnabledForTest(true);
   EXPECT_TRUE(window->layer()->GetAnimator()->is_animating());
   EXPECT_TRUE(window2->layer()->GetAnimator()->is_animating());
-  layer->GetAnimator()->StopAnimating();
-  layer2->GetAnimator()->StopAnimating();
+  WaitForWindowAnimation(window.get());
+  WaitForWindowAnimation(window2.get());
+  WaitForSmoothnessMetrics();
   histogram_tester.ExpectTotalCount(kEnterHistogram, 1);
   histogram_tester.ExpectTotalCount(kExitHistogram, 0);
 
@@ -1583,8 +1602,8 @@ TEST_P(TabletModeControllerTest, TabletModeTransitionHistogramsLogged) {
   tablet_mode_controller()->SetEnabledForTest(false);
   EXPECT_FALSE(layer->GetAnimator()->is_animating());
   EXPECT_TRUE(layer2->GetAnimator()->is_animating());
-  layer->GetAnimator()->StopAnimating();
-  layer2->GetAnimator()->StopAnimating();
+  WaitForWindowAnimation(window2.get());
+  WaitForSmoothnessMetrics();
   histogram_tester.ExpectTotalCount(kEnterHistogram, 1);
   histogram_tester.ExpectTotalCount(kExitHistogram, 1);
 }
@@ -1604,6 +1623,7 @@ TEST_P(TabletModeControllerTest, TabletModeTransitionHistogramsSnappedWindows) {
   tablet_mode_controller()->SetEnabledForTest(true);
   EXPECT_FALSE(window->layer()->GetAnimator()->is_animating());
   EXPECT_FALSE(window2->layer()->GetAnimator()->is_animating());
+  WaitForSmoothnessMetrics();
   histogram_tester.ExpectTotalCount(kEnterHistogram, 0);
   histogram_tester.ExpectTotalCount(kExitHistogram, 0);
 }