desks: Add metric to track number of desk traversals in a time period.

Starts tracking when a desk switch animation starts. Sets up a timer
which then records the number of traversals during a sequence of
desk animations. There may be multiple animations during the timer
duration, they will notify the metrics collector class on animation
end with how many traversals have happened.

For touchpad swipes, add some logic to track how much visible desk
switches the user has seen. This is different from the number of desk
activations. For all other desk switch sources, the number of
traversals during an animation is 1 + #Replacements.

Test: manual, added tests
Bug: 1146479
Change-Id: Ie3c46ebe80e35a768a89eb6e7105d807db087529
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2523061
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826426}

ash/wm/desks/autotest_desks_api.cc

@@ -110,9 +110,9 @@ class ChainedDeskAnimationObserver : public ui::LayerAnimationObserver,
     if (animation_layer_)
       return;
 
-    animation_layer_ = DesksController::Get()
-                           ->GetAnimationForTesting()
-                           ->GetFirstDeskSwitchAnimatorForTesting()
+    auto* animation = DesksController::Get()->animation();
+    DCHECK(animation);
+    animation_layer_ = animation->GetFirstDeskSwitchAnimatorForTesting()
                            ->GetAnimationLayerForTesting();
     animation_layer_->GetAnimator()->AddObserver(this);
   }

ash/wm/desks/desk_animation_base.cc

@@ -132,6 +132,10 @@ void DeskAnimationBase::OnDeskSwitchAnimationFinished() {
   // `this` is now deleted.
 }
 
+void DeskAnimationBase::OnVisibleDeskChanged() {
+  ++visible_desk_changes_;
+}
+
 RootWindowDeskSwitchAnimator*
 DeskAnimationBase::GetFirstDeskSwitchAnimatorForTesting() const {
   DCHECK(!desk_switch_animators_.empty());

ash/wm/desks/desk_animation_base.h

@@ -29,6 +29,7 @@ class DeskAnimationBase : public RootWindowDeskSwitchAnimator::Delegate {
   ~DeskAnimationBase() override;
 
   int ending_desk_index() const { return ending_desk_index_; }
+  int visible_desk_changes() const { return visible_desk_changes_; }
 
   // Launches the animation. This should be done once all animators
   // are created and added to `desk_switch_animators_`. This is to avoid any
@@ -53,6 +54,7 @@ class DeskAnimationBase : public RootWindowDeskSwitchAnimator::Delegate {
   void OnStartingDeskScreenshotTaken(int ending_desk_index) override;
   void OnEndingDeskScreenshotTaken() override;
   void OnDeskSwitchAnimationFinished() override;
+  void OnVisibleDeskChanged() override;
 
   RootWindowDeskSwitchAnimator* GetFirstDeskSwitchAnimatorForTesting() const;
 
@@ -86,6 +88,13 @@ class DeskAnimationBase : public RootWindowDeskSwitchAnimator::Delegate {
   // OnEndingDeskScreenshotTaken is called.
   const bool is_continuous_gesture_animation_;
 
+  // Used for metrics collection to track how many desks changes a user has seen
+  // during the animation. This is different from the number of desk activations
+  // as the user may switch desks but not activate it if the desk already has a
+  // screenshot taken. This will only change for an activation animation, not a
+  // remove animation.
+  int visible_desk_changes_ = 0;
+
   // ThroughputTracker used for measuring this animation smoothness.
   ui::ThroughputTracker throughput_tracker_;
 };

ash/wm/desks/desk_animation_impl.cc

@@ -68,6 +68,11 @@ DeskActivationAnimation::DeskActivationAnimation(DesksController* controller,
             root, starting_desk_index, ending_desk_index, this,
             /*for_remove=*/false));
   }
+
+  // On starting, the user may stay on the current desk for a touchpad swipe.
+  // All other switch sources are guaranteed to move at least once.
+  if (switch_source_ != DesksSwitchSource::kDeskSwitchTouchpad)
+    visible_desk_changes_ = 1;
 }
 
 DeskActivationAnimation::~DeskActivationAnimation() = default;
@@ -96,6 +101,11 @@ bool DeskActivationAnimation::Replace(bool moving_left,
 
   ending_desk_index_ = new_ending_desk_index;
 
+  // Similar to on starting, for touchpad, the user can replace the animation
+  // without switching visible desks.
+  if (switch_source_ != DesksSwitchSource::kDeskSwitchTouchpad)
+    ++visible_desk_changes_;
+
   // List of animators that need a screenshot. It should be either empty or
   // match the size of |desk_switch_animators_| as all the animations should be
   // in sync.

ash/wm/desks/desks_controller.cc

@@ -33,10 +33,12 @@
 #include "base/auto_reset.h"
 #include "base/check_op.h"
 #include "base/containers/unique_ptr_adapters.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/notreached.h"
 #include "base/numerics/ranges.h"
 #include "base/stl_util.h"
+#include "base/timer/timer.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/wm/public/activation_client.h"
 
@@ -59,6 +61,16 @@ constexpr char kNumberOfWindowsOnDesk_3_HistogramName[] =
 constexpr char kNumberOfWindowsOnDesk_4_HistogramName[] =
     "Ash.Desks.NumberOfWindowsOnDesk_4";
 
+constexpr char kNumberOfDeskTraversalsHistogramName[] =
+    "Ash.Desks.NumberOfDeskTraversals";
+constexpr int kDeskTraversalsMaxValue = 20;
+
+// After an desk activation animation starts,
+// |kNumberOfDeskTraversalsHistogramName| will be recorded after this time
+// interval.
+constexpr base::TimeDelta kDeskTraversalsTimeout =
+    base::TimeDelta::FromSeconds(5);
+
 // Appends the given |windows| to the end of the currently active overview mode
 // session such that the most-recently used window is added first. If
 // The windows will animate to their positions in the overview grid.
@@ -143,8 +155,69 @@ bool IsParentSwitchableContainer(const aura::Window* window) {
 
 }  // namespace
 
+// Helper class which wraps around a OneShotTimer and used for recording how
+// many times the user has traversed desks. Here traversal means the amount of
+// times the user has seen a visual desk change. This differs from desk
+// activation as a desk is only activated as needed for a screenshot during an
+// animation. The user may bounce back and forth on two desks that already
+// have screenshots, and each bounce is recorded as a traversal. For touchpad
+// swipes, the amount of traversals in one animation depends on the amount of
+// changes in the most visible desk have been seen. For other desk changes,
+// the amount of traversals in one animation is 1 + number of Replace() calls.
+// Multiple animations may be recorded before the timer stops.
+class DesksController::DeskTraversalsMetricsHelper {
+ public:
+  explicit DeskTraversalsMetricsHelper(DesksController* controller)
+      : controller_(controller) {}
+  DeskTraversalsMetricsHelper(const DeskTraversalsMetricsHelper&) = delete;
+  DeskTraversalsMetricsHelper& operator=(const DeskTraversalsMetricsHelper&) =
+      delete;
+  ~DeskTraversalsMetricsHelper() = default;
+
+  // Starts |timer_| unless it is already running.
+  void MaybeStart() {
+    if (timer_.IsRunning())
+      return;
+
+    count_ = 0;
+    timer_.Start(FROM_HERE, kDeskTraversalsTimeout,
+                 base::BindOnce(&DeskTraversalsMetricsHelper::OnTimerStop,
+                                base::Unretained(this)));
+  }
+
+  // Called when a desk animation is finished. Adds all observed
+  // |visible_desk_changes| to |count_|.
+  void OnAnimationFinished(int visible_desk_changes) {
+    if (timer_.IsRunning())
+      count_ += visible_desk_changes;
+  }
+
+ private:
+  void OnTimerStop() {
+    // If an animation is still running, add its current visible desk change
+    // count to |count_|.
+    DeskAnimationBase* current_animation = controller_->animation();
+    if (current_animation)
+      count_ += current_animation->visible_desk_changes();
+
+    base::UmaHistogramExactLinear(kNumberOfDeskTraversalsHistogramName, count_,
+                                  kDeskTraversalsMaxValue);
+  }
+
+  // Pointer to the DesksController that owns this. Guaranteed to be not
+  // nullptr for the lifetime of |this|.
+  DesksController* const controller_;
+
+  base::OneShotTimer timer_;
+
+  // Tracks the amount of traversals that have happened since |timer_| has
+  // started.
+  int count_ = 0;
+};
+
 DesksController::DesksController()
-    : is_enhanced_desk_animations_(features::IsEnhancedDeskAnimations()) {
+    : is_enhanced_desk_animations_(features::IsEnhancedDeskAnimations()),
+      metrics_helper_(std::make_unique<DeskTraversalsMetricsHelper>(this)) {
   Shell::Get()->activation_client()->AddObserver(this);
   Shell::Get()->session_controller()->AddObserver(this);
 
@@ -322,6 +395,8 @@ void DesksController::ActivateDesk(const Desk* desk, DesksSwitchSource source) {
       this, starting_desk_index, target_desk_index, source,
       update_window_activation);
   animation_->Launch();
+
+  metrics_helper_->MaybeStart();
 }
 
 bool DesksController::ActivateAdjacentDesk(bool going_left,
@@ -521,13 +596,9 @@ void DesksController::OnFirstSessionStarted() {
   desks_restore_util::RestorePrimaryUserDesks();
 }
 
-DeskAnimationBase* DesksController::GetAnimationForTesting() const {
-  DCHECK(animation_);
-  return animation_.get();
-}
-
 void DesksController::OnAnimationFinished(DeskAnimationBase* animation) {
   DCHECK_EQ(animation_.get(), animation);
+  metrics_helper_->OnAnimationFinished(animation->visible_desk_changes());
   animation_.reset();
 }
 

ash/wm/desks/desks_controller.h

@@ -73,6 +73,8 @@ class ASH_EXPORT DesksController : public DesksHelper,
 
   const Desk* active_desk() const { return active_desk_; }
 
+  DeskAnimationBase* animation() const { return animation_.get(); }
+
   // Returns the current |active_desk()| or the soon-to-be active desk if a desk
   // switch animation is in progress.
   const Desk* GetTargetActiveDesk() const;
@@ -178,9 +180,8 @@ class ASH_EXPORT DesksController : public DesksHelper,
   void OnActiveUserSessionChanged(const AccountId& account_id) override;
   void OnFirstSessionStarted() override;
 
-  DeskAnimationBase* GetAnimationForTesting() const;
-
  private:
+  class DeskTraversalsMetricsHelper;
   friend class DeskAnimationBase;
   friend class DeskActivationAnimation;
   friend class DeskRemovalAnimation;
@@ -244,6 +245,10 @@ class ASH_EXPORT DesksController : public DesksHelper,
   // True when the enhanced desk animations feature is enabled.
   const bool is_enhanced_desk_animations_;
 
+  // Responsible for tracking and writing number of desk traversals one has
+  // done within a span of X seconds.
+  std::unique_ptr<DeskTraversalsMetricsHelper> metrics_helper_;
+
   base::ObserverList<Observer>::Unchecked observers_;
 
   DISALLOW_COPY_AND_ASSIGN(DesksController);

ash/wm/desks/desks_unittests.cc

@@ -56,6 +56,7 @@
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/prefs/pref_service.h"
@@ -3686,6 +3687,45 @@ TEST_P(PerDeskShelfTest, RemoveActiveDesk) {
   VerifyViewVisibility(app1, true);
   VerifyViewVisibility(app2, true);
 }
+
+// A test class that uses a mock time task environment.
+class DesksMockTimeTest : public AshTestBase {
+ public:
+  DesksMockTimeTest()
+      : AshTestBase(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
+  DesksMockTimeTest(const DesksMockTimeTest&) = delete;
+  DesksMockTimeTest& operator=(const DesksMockTimeTest&) = delete;
+  ~DesksMockTimeTest() override = default;
+};
+
+TEST_F(DesksMockTimeTest, DeskTraversalNonTouchpadMetrics) {
+  NewDesk();
+  NewDesk();
+  NewDesk();
+  ASSERT_EQ(4u, DesksController::Get()->desks().size());
+
+  constexpr char kDeskTraversalsHistogramName[] =
+      "Ash.Desks.NumberOfDeskTraversals";
+
+  base::HistogramTester histogram_tester;
+  auto* controller = DesksController::Get();
+  const auto& desks = controller->desks();
+  ASSERT_EQ(controller->active_desk(), desks[0].get());
+
+  // Move 5 desks. There is nothing recorded at the end since the timer is still
+  // running.
+  ActivateDesk(desks[1].get());
+  ActivateDesk(desks[0].get());
+  ActivateDesk(desks[1].get());
+  ActivateDesk(desks[2].get());
+  ActivateDesk(desks[3].get());
+  histogram_tester.ExpectBucketCount(kDeskTraversalsHistogramName, 5, 0);
+
+  // Advance the time to end the timer. There should be 5 desks recorded.
+  task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(5));
+  histogram_tester.ExpectBucketCount(kDeskTraversalsHistogramName, 5, 1);
+}
+
 // TODO(afakhry): Add more tests:
 // - Always on top windows are not tracked by any desk.
 // - Reusing containers when desks are removed and created.

ash/wm/desks/root_window_desk_switch_animator.cc

@@ -130,6 +130,7 @@ RootWindowDeskSwitchAnimator::RootWindowDeskSwitchAnimator(
     : root_window_(root),
       starting_desk_index_(starting_desk_index),
       ending_desk_index_(ending_desk_index),
+      visible_desk_index_(starting_desk_index),
       delegate_(delegate),
       animation_layer_owner_(CreateAnimationLayerOwner(root)),
       root_window_size_(
@@ -301,13 +302,17 @@ bool RootWindowDeskSwitchAnimator::UpdateSwipeAnimation(float scroll_delta_x) {
           : transformed_animation_layer_bounds.x() >
                 -kMinDistanceBeforeScreenshotDp;
 
+  const int old_visible_desk_index = visible_desk_index_;
+  visible_desk_index_ = GetIndexOfMostVisibleDeskScreenshot();
+  if (old_visible_desk_index != visible_desk_index_)
+    delegate_->OnVisibleDeskChanged();
+
   if (!going_out_of_bounds)
     return false;
 
-  // Get the current visible desk index. The upcoming desk we need to show will
-  // be an adjacent desk based on |moving_left|.
-  const int current_visible_desk_index = GetIndexOfMostVisibleDeskScreenshot();
-  int new_desk_index = current_visible_desk_index + (moving_left ? 1 : -1);
+  // The upcoming desk we need to show will be an adjacent desk to the desk at
+  // |visible_desk_index_| based on |moving_left|.
+  const int new_desk_index = visible_desk_index_ + (moving_left ? 1 : -1);
 
   if (new_desk_index < 0 ||
       new_desk_index >= int{DesksController::Get()->desks().size()}) {
@@ -331,14 +336,13 @@ void RootWindowDeskSwitchAnimator::EndSwipeAnimation() {
     return;
   }
 
-  // If the ending desk screenshot has not finished,
-  // GetIndexOfMostVisibleDeskScreenshot will still return a valid desk index
-  // that we can animate to, but we need to make sure the ending desk screenshot
-  // callback does not get called.
+  // If the ending desk screenshot has not finished, |visible_desk_index_| will
+  // still return a valid desk index that we can animate to, but we need to make
+  // sure the ending desk screenshot callback does not get called.
   if (!ending_desk_screenshot_taken_)
     weak_ptr_factory_.InvalidateWeakPtrs();
 
-  ending_desk_index_ = GetIndexOfMostVisibleDeskScreenshot();
+  ending_desk_index_ = visible_desk_index_;
   StartAnimation();
 }
 

ash/wm/desks/root_window_desk_switch_animator.h

@@ -196,6 +196,10 @@ class ASH_EXPORT RootWindowDeskSwitchAnimator
     // desk screenshot is now showing on the screen.
     virtual void OnDeskSwitchAnimationFinished() = 0;
 
+    // Called while doing a continuous gesture to notify when the desk that is
+    // visible to the user has changed. Used for metrics collection.
+    virtual void OnVisibleDeskChanged() = 0;
+
    protected:
     virtual ~Delegate() = default;
   };
@@ -311,6 +315,10 @@ class ASH_EXPORT RootWindowDeskSwitchAnimator
   // The index of the desk to activate and animate to with this animator.
   int ending_desk_index_;
 
+  // The index of the desk that is most visible to the user based on the
+  // transform of the animation layer.
+  int visible_desk_index_;
+
   Delegate* const delegate_;
 
   // The owner of the layer tree of the old detached layers of the removed

ash/wm/desks/root_window_desk_switch_animator_unittest.cc

@@ -90,6 +90,7 @@ class RootWindowDeskSwitchAnimatorTest
   int ending_desk_screenshot_taken_count() const {
     return ending_desk_screenshot_taken_count_;
   }
+  int visible_desk_changed_count() const { return visible_desk_changed_count_; }
 
   // Creates an animator from the given indices on the primary root window.
   // Creates a test api for the animator as well.
@@ -140,6 +141,7 @@ class RootWindowDeskSwitchAnimatorTest
   }
 
   void OnDeskSwitchAnimationFinished() override {}
+  void OnVisibleDeskChanged() override { ++visible_desk_changed_count_; }
 
  private:
   base::test::ScopedFeatureList scoped_feature_list_;
@@ -155,6 +157,7 @@ class RootWindowDeskSwitchAnimatorTest
 
   int starting_desk_screenshot_taken_count_ = 0;
   int ending_desk_screenshot_taken_count_ = 0;
+  int visible_desk_changed_count_ = 0;
 };
 
 // Tests a simple animation from one desk to another.
@@ -464,4 +467,47 @@ TEST_F(RootWindowDeskSwitchAnimatorTest,
   animator()->EndSwipeAnimation();
 }
 
+// Tests that visible desk change count updates as expected. It is used higher
+// up for metrics collection, but the logic is in this class.
+TEST_F(RootWindowDeskSwitchAnimatorTest, VisibleDeskChangeCount) {
+  // Add three desks for a total of four.
+  DesksController::Get()->NewDesk(DesksCreationRemovalSource::kButton);
+  DesksController::Get()->NewDesk(DesksCreationRemovalSource::kButton);
+  DesksController::Get()->NewDesk(DesksCreationRemovalSource::kButton);
+
+  InitAnimator(0, 1);
+  TakeStartingDeskScreenshotAndWait();
+  TakeEndingDeskScreenshotAndWait();
+  EXPECT_EQ(0, visible_desk_changed_count());
+
+  const int touchpad_swipe_length_for_desk_change =
+      RootWindowDeskSwitchAnimator::kTouchpadSwipeLengthForDeskChange;
+
+  // Swipe enough so that our third and fourth desk screenshots are taken, and
+  // then swipe so that the fourth desk is fully shown. There should be 3
+  // visible desk changes in total.
+  ASSERT_TRUE(
+      animator()->UpdateSwipeAnimation(-touchpad_swipe_length_for_desk_change));
+  TakeEndingDeskScreenshotAndWait();
+  ASSERT_TRUE(
+      animator()->UpdateSwipeAnimation(-touchpad_swipe_length_for_desk_change));
+  TakeEndingDeskScreenshotAndWait();
+  animator()->UpdateSwipeAnimation(-3 * touchpad_swipe_length_for_desk_change);
+  EXPECT_EQ(3, visible_desk_changed_count());
+
+  // Do some minor swipes to the right. We should still be focused on the last
+  // desk so the visible desk change count remains the same.
+  animator()->UpdateSwipeAnimation(touchpad_swipe_length_for_desk_change / 10);
+  animator()->UpdateSwipeAnimation(touchpad_swipe_length_for_desk_change / 10);
+  EXPECT_EQ(3, visible_desk_changed_count());
+
+  // Do two full swipes to the right, and then two full swipes to the left. Test
+  // that the desk change count has increased by four.
+  animator()->UpdateSwipeAnimation(touchpad_swipe_length_for_desk_change);
+  animator()->UpdateSwipeAnimation(touchpad_swipe_length_for_desk_change);
+  animator()->UpdateSwipeAnimation(-touchpad_swipe_length_for_desk_change);
+  animator()->UpdateSwipeAnimation(-touchpad_swipe_length_for_desk_change);
+  EXPECT_EQ(7, visible_desk_changed_count());
+}
+
 }  // namespace ash

tools/metrics/histograms/histograms_xml/ash/histograms.xml

@@ -477,6 +477,20 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="Ash.Desks.NumberOfDeskTraversals" units="units"
+    expires_after="2021-11-10">
+  <owner>afakhry@chromium.org</owner>
+  <owner>sammiequon@chromium.org</owner>
+  <owner>tclaiborne@chromium.org</owner>
+  <summary>
+    Tracks the number of times a desk (virtual workspace) has visibly changed in
+    a time span of five seconds. This counts the number of total traversals; for
+    example if the user has moved from desk 1 to 2 to 1 to 2 to 3, the number
+    recorded is 4. This only gets recorded if a traversal starts. A traversal
+    means that the desk has changed onscreen.
+  </summary>
+</histogram>
+
 <histogram name="Ash.Desks.NumberOfWindowsOnDesk_1" units="units"
     expires_after="2021-04-11">
   <owner>afakhry@chromium.org</owner>