desks: For animation classes, store index instead of actual desk.

Requested in crrev.com/c/2354798. This will simplfy things down the line
as we use more index lookup and remove const from some parameters in
RootWindowDeskSwitchAnimator.

Test: manual, existing tests
Bug: 1111445
Change-Id: I7250d969b95f08da5819c73ca325bce2db0ea5bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2369518Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800828}

ash/wm/desks/desk_animation_base.cc

@@ -33,13 +33,14 @@ ui::Compositor* GetSelectedCompositorForAnimationSmoothness() {
 // DeskAnimationBase:
 
 DeskAnimationBase::DeskAnimationBase(DesksController* controller,
-                                     const Desk* ending_desk)
+                                     int ending_desk_index)
     : controller_(controller),
-      ending_desk_(ending_desk),
+      ending_desk_index_(ending_desk_index),
       throughput_tracker_(GetSelectedCompositorForAnimationSmoothness()
                               ->RequestNewThroughputTracker()) {
   DCHECK(controller_);
-  DCHECK(ending_desk_);
+  DCHECK_LE(ending_desk_index_, int{controller_->desks().size()});
+  DCHECK_GE(ending_desk_index_, 0);
 }
 
 DeskAnimationBase::~DeskAnimationBase() = default;
@@ -58,14 +59,14 @@ void DeskAnimationBase::Launch() {
   // This window must be able to accept events (See
   // `aura::Window::CanAcceptEvent()`) even though its desk is still being
   // activated. https://crbug.com/1008574.
-  const_cast<Desk*>(ending_desk_)->PrepareForActivationAnimation();
+  controller_->desks()[ending_desk_index_]->PrepareForActivationAnimation();
 
   DCHECK(!desk_switch_animators_.empty());
   for (auto& animator : desk_switch_animators_)
     animator->TakeStartingDeskScreenshot();
 }
 
-void DeskAnimationBase::OnStartingDeskScreenshotTaken(const Desk* ending_desk) {
+void DeskAnimationBase::OnStartingDeskScreenshotTaken(int ending_desk_index) {
   DCHECK(!desk_switch_animators_.empty());
 
   // Once all starting desk screenshots on all roots are taken and placed on
@@ -81,7 +82,7 @@ void DeskAnimationBase::OnStartingDeskScreenshotTaken(const Desk* ending_desk) {
   for (auto* root : roots)
     root->GetHost()->compositor()->SetAllowLocksToExtendTimeout(true);
 
-  OnStartingDeskScreenshotTakenInternal(ending_desk);
+  OnStartingDeskScreenshotTakenInternal(ending_desk_index);
 
   for (auto* root : roots)
     root->GetHost()->compositor()->SetAllowLocksToExtendTimeout(false);

ash/wm/desks/desk_animation_base.h

@@ -11,7 +11,6 @@
 
 namespace ash {
 
-class Desk;
 class DesksController;
 
 // An abstract class that handles the shared operations need to be performed
@@ -21,12 +20,12 @@ class DesksController;
 // each animation type.
 class DeskAnimationBase : public RootWindowDeskSwitchAnimator::Delegate {
  public:
-  DeskAnimationBase(DesksController* controller, const Desk* ending_desk);
+  DeskAnimationBase(DesksController* controller, int ending_desk_index);
   DeskAnimationBase(const DeskAnimationBase&) = delete;
   DeskAnimationBase& operator=(const DeskAnimationBase&) = delete;
   ~DeskAnimationBase() override;
 
-  const Desk* ending_desk() const { return ending_desk_; }
+  int ending_desk_index() const { return ending_desk_index_; }
 
   // Launches the animation. This should be done once all animators
   // are created and added to `desk_switch_animators_`. This is to avoid any
@@ -35,7 +34,7 @@ class DeskAnimationBase : public RootWindowDeskSwitchAnimator::Delegate {
   void Launch();
 
   // RootWindowDeskSwitchAnimator::Delegate:
-  void OnStartingDeskScreenshotTaken(const Desk* ending_desk) override;
+  void OnStartingDeskScreenshotTaken(int ending_desk_index) override;
   void OnEndingDeskScreenshotTaken() override;
   void OnDeskSwitchAnimationFinished() override;
 
@@ -44,8 +43,7 @@ class DeskAnimationBase : public RootWindowDeskSwitchAnimator::Delegate {
   // things when phase (1), and phase (3) completes. Note that
   // `OnDeskSwitchAnimationFinishedInternal()` will be called before the desks
   // screenshot layers, stored in `desk_switch_animators_`, are destroyed.
-  virtual void OnStartingDeskScreenshotTakenInternal(
-      const Desk* ending_desk) = 0;
+  virtual void OnStartingDeskScreenshotTakenInternal(int ending_desk_index) = 0;
   virtual void OnDeskSwitchAnimationFinishedInternal() = 0;
 
   // Since performance here matters, we have to use the UMA histograms macros to
@@ -62,8 +60,8 @@ class DeskAnimationBase : public RootWindowDeskSwitchAnimator::Delegate {
   std::vector<std::unique_ptr<RootWindowDeskSwitchAnimator>>
       desk_switch_animators_;
 
-  // The desk that will be active after this animation ends.
-  const Desk* const ending_desk_;
+  // The index of the desk that will be active after this animation ends.
+  const int ending_desk_index_;
 
  private:
   // ThroughputTracker used for measuring this animation smoothness.

ash/wm/desks/desk_animation_impl.cc

@@ -27,22 +27,22 @@ constexpr char kDeskRemovalSmoothnessHistogramName[] =
 // DeskActivationAnimation:
 
 DeskActivationAnimation::DeskActivationAnimation(DesksController* controller,
-                                                 const Desk* ending_desk,
-                                                 bool move_left)
-    : DeskAnimationBase(controller, ending_desk) {
+                                                 int starting_desk_index,
+                                                 int ending_desk_index)
+    : DeskAnimationBase(controller, ending_desk_index) {
   for (auto* root : Shell::GetAllRootWindows()) {
     desk_switch_animators_.emplace_back(
-        std::make_unique<RootWindowDeskSwitchAnimator>(root, ending_desk, this,
-                                                       move_left,
-                                                       /*for_remove=*/false));
+        std::make_unique<RootWindowDeskSwitchAnimator>(
+            root, starting_desk_index, ending_desk_index, this,
+            /*for_remove=*/false));
   }
 }
 
 DeskActivationAnimation::~DeskActivationAnimation() = default;
 
 void DeskActivationAnimation::OnStartingDeskScreenshotTakenInternal(
-    const Desk* ending_desk) {
-  DCHECK_EQ(ending_desk_, ending_desk);
+    int ending_desk_index) {
+  DCHECK_EQ(ending_desk_index_, ending_desk_index);
   // The order here matters. Overview must end before ending tablet split view
   // before switching desks. (If clamshell split view is active on one or more
   // displays, then it simply will end when we end overview.) That's because
@@ -65,8 +65,9 @@ void DeskActivationAnimation::OnStartingDeskScreenshotTakenInternal(
   split_view_controller->EndSplitView(
       SplitViewController::EndReason::kDesksChange);
 
-  controller_->ActivateDeskInternal(ending_desk,
-                                    /*update_window_activation=*/true);
+  controller_->ActivateDeskInternal(
+      controller_->desks()[ending_desk_index_].get(),
+      /*update_window_activation=*/true);
 
   MaybeRestoreSplitView(/*refresh_snapped_windows=*/true);
 }
@@ -83,30 +84,32 @@ metrics_util::ReportCallback DeskActivationAnimation::GetReportCallback()
 // DeskRemovalAnimation:
 
 DeskRemovalAnimation::DeskRemovalAnimation(DesksController* controller,
-                                           const Desk* desk_to_remove,
-                                           const Desk* desk_to_activate,
-                                           bool move_left,
+                                           int desk_to_remove_index,
+                                           int desk_to_activate_index,
                                            DesksCreationRemovalSource source)
-    : DeskAnimationBase(controller, desk_to_activate),
-      desk_to_remove_(desk_to_remove),
+    : DeskAnimationBase(controller, desk_to_activate_index),
+      desk_to_remove_index_(desk_to_remove_index),
       request_source_(source) {
   DCHECK(!Shell::Get()->overview_controller()->InOverviewSession());
-  DCHECK_EQ(controller_->active_desk(), desk_to_remove_);
+  DCHECK_EQ(controller_->active_desk(),
+            controller_->desks()[desk_to_remove_index_].get());
 
   for (auto* root : Shell::GetAllRootWindows()) {
     desk_switch_animators_.emplace_back(
-        std::make_unique<RootWindowDeskSwitchAnimator>(root, desk_to_activate,
-                                                       this, move_left,
-                                                       /*for_remove=*/true));
+        std::make_unique<RootWindowDeskSwitchAnimator>(
+            root, desk_to_remove_index_, desk_to_activate_index, this,
+            /*for_remove=*/true));
   }
 }
 
 DeskRemovalAnimation::~DeskRemovalAnimation() = default;
 
 void DeskRemovalAnimation::OnStartingDeskScreenshotTakenInternal(
-    const Desk* ending_desk) {
-  DCHECK_EQ(ending_desk_, ending_desk);
-  DCHECK_EQ(controller_->active_desk(), desk_to_remove_);
+    int ending_desk_index) {
+  DCHECK_EQ(ending_desk_index_, ending_desk_index);
+  DCHECK_EQ(controller_->active_desk(),
+            controller_->desks()[desk_to_remove_index_].get());
+
   // We are removing the active desk, which may have tablet split view active.
   // We will restore the split view state of the newly activated desk at the
   // end of the animation. Clamshell split view is impossible because
@@ -117,17 +120,19 @@ void DeskRemovalAnimation::OnStartingDeskScreenshotTakenInternal(
       SplitViewController::EndReason::kDesksChange);
 
   // At the end of phase (1), we activate the target desk (i.e. the desk that
-  // will be activated after the active desk `desk_to_remove_` is removed).
-  // This means that phase (2) will take a screenshot of that desk before we
-  // move the windows of `desk_to_remove_` to that target desk.
-  controller_->ActivateDeskInternal(ending_desk,
-                                    /*update_window_activation=*/false);
+  // will be activated after the active desk `desk_to_remove_index_` is
+  // removed). This means that phase (2) will take a screenshot of that desk
+  // before we move the windows of `desk_to_remove_index_` to that target desk.
+  controller_->ActivateDeskInternal(
+      controller_->desks()[ending_desk_index_].get(),
+      /*update_window_activation=*/false);
 }
 
 void DeskRemovalAnimation::OnDeskSwitchAnimationFinishedInternal() {
   // Do the actual desk removal behind the scenes before the screenshot layers
   // are destroyed.
-  controller_->RemoveDeskInternal(desk_to_remove_, request_source_);
+  controller_->RemoveDeskInternal(
+      controller_->desks()[desk_to_remove_index_].get(), request_source_);
 
   MaybeRestoreSplitView(/*refresh_snapped_windows=*/true);
 }

ash/wm/desks/desk_animation_impl.h

@@ -11,20 +11,19 @@
 
 namespace ash {
 
-class Desk;
 class DesksController;
 
 class DeskActivationAnimation : public DeskAnimationBase {
  public:
   DeskActivationAnimation(DesksController* controller,
-                          const Desk* ending_desk,
-                          bool move_left);
+                          int starting_desk_index,
+                          int ending_desk_index);
   DeskActivationAnimation(const DeskActivationAnimation&) = delete;
   DeskActivationAnimation& operator=(const DeskActivationAnimation&) = delete;
   ~DeskActivationAnimation() override;
 
   // DeskAnimationBase:
-  void OnStartingDeskScreenshotTakenInternal(const Desk* ending_desk) override;
+  void OnStartingDeskScreenshotTakenInternal(int ending_desk_index) override;
   void OnDeskSwitchAnimationFinishedInternal() override {}
   metrics_util::ReportCallback GetReportCallback() const override;
 };
@@ -32,21 +31,20 @@ class DeskActivationAnimation : public DeskAnimationBase {
 class DeskRemovalAnimation : public DeskAnimationBase {
  public:
   DeskRemovalAnimation(DesksController* controller,
-                       const Desk* desk_to_remove,
-                       const Desk* desk_to_activate,
-                       bool move_left,
+                       int desk_to_remove_index,
+                       int desk_to_activate_index,
                        DesksCreationRemovalSource source);
   DeskRemovalAnimation(const DeskRemovalAnimation&) = delete;
   DeskRemovalAnimation& operator=(const DeskRemovalAnimation&) = delete;
   ~DeskRemovalAnimation() override;
 
   // DeskAnimationBase:
-  void OnStartingDeskScreenshotTakenInternal(const Desk* ending_desk) override;
+  void OnStartingDeskScreenshotTakenInternal(int ending_desk_index) override;
   void OnDeskSwitchAnimationFinishedInternal() override;
   metrics_util::ReportCallback GetReportCallback() const override;
 
  private:
-  const Desk* const desk_to_remove_;
+  const int desk_to_remove_index_;
   const DesksCreationRemovalSource request_source_;
 };
 

ash/wm/desks/desks_controller.cc

@@ -163,7 +163,7 @@ DesksController* DesksController::Get() {
 
 const Desk* DesksController::GetTargetActiveDesk() const {
   if (!animations_.empty())
-    return animations_.back()->ending_desk();
+    return desks_[animations_.back()->ending_desk_index()].get();
   return active_desk();
 }
 
@@ -255,9 +255,8 @@ void DesksController::RemoveDesk(const Desk* desk,
         current_desk_index + ((current_desk_index > 0) ? -1 : 1);
     DCHECK_GE(target_desk_index, 0);
     DCHECK_LT(target_desk_index, static_cast<int>(desks_.size()));
-    const bool move_left = current_desk_index < target_desk_index;
     animations_.emplace_back(std::make_unique<DeskRemovalAnimation>(
-        this, desk, desks_[target_desk_index].get(), move_left, source));
+        this, current_desk_index, target_desk_index, source));
     animations_.back()->Launch();
     return;
   }
@@ -299,12 +298,9 @@ void DesksController::ActivateDesk(const Desk* desk, DesksSwitchSource source) {
     return;
   }
 
-  // New desks are always added at the end of the list to the right of existing
-  // desks. Therefore, desks at lower indices are located on the left of desks
-  // with higher indices.
-  const bool move_left = GetDeskIndex(active_desk_) < target_desk_index;
-  animations_.emplace_back(
-      std::make_unique<DeskActivationAnimation>(this, desk, move_left));
+  const int starting_desk_index = GetDeskIndex(active_desk());
+  animations_.emplace_back(std::make_unique<DeskActivationAnimation>(
+      this, starting_desk_index, target_desk_index));
   animations_.back()->Launch();
 }
 

ash/wm/desks/root_window_desk_switch_animator.cc

@@ -102,21 +102,19 @@ std::unique_ptr<ui::Layer> CreateLayerFromScreenshotResult(
 
 RootWindowDeskSwitchAnimator::RootWindowDeskSwitchAnimator(
     aura::Window* root,
-    const Desk* ending_desk,
+    int starting_desk_index,
+    int ending_desk_index,
     Delegate* delegate,
-    bool move_left,
     bool for_remove)
     : root_window_(root),
-      starting_desk_(DesksController::Get()->active_desk()),
-      ending_desk_(ending_desk),
+      starting_desk_index_(starting_desk_index),
+      ending_desk_index_(ending_desk_index),
       delegate_(delegate),
       animation_layer_owner_(CreateAnimationLayerOwner(root)),
       x_translation_offset_(root->layer()->size().width() + kDesksSpacing),
-      move_left_(move_left),
       for_remove_(for_remove) {
   DCHECK(root_window_);
-  DCHECK(starting_desk_);
-  DCHECK(ending_desk_);
+  DCHECK_NE(starting_desk_index_, ending_desk_index_);
   DCHECK(delegate_);
 }
 
@@ -133,8 +131,9 @@ void RootWindowDeskSwitchAnimator::TakeStartingDeskScreenshot() {
   if (for_remove_) {
     // The active desk is about to be removed. Recreate and detach its old
     // layers to animate them in a jump-like animation.
-    auto* desk_container =
-        starting_desk_->GetDeskContainerForRoot(root_window_);
+    auto* desk_container = DesksController::Get()
+                               ->desks()[starting_desk_index_]
+                               ->GetDeskContainerForRoot(root_window_);
     old_windows_layer_tree_owner_ = wm::RecreateLayers(desk_container);
     root_window_->layer()->Add(old_windows_layer_tree_owner_->root());
     root_window_->layer()->StackAtTop(old_windows_layer_tree_owner_->root());
@@ -170,7 +169,7 @@ void RootWindowDeskSwitchAnimator::StartAnimation() {
 
   gfx::Transform animation_layer_ending_transfrom;
 
-  if (move_left_) {
+  if (starting_desk_index_ < ending_desk_index_) {
     // Starting desk is one the left, so the ending transform of the parent
     // "animation layer" is then a translation to the left such that at the end,
     // the ending screenshot layer becomes the one visible on the screen.
@@ -237,7 +236,7 @@ void RootWindowDeskSwitchAnimator::CompleteAnimationPhase1WithLayer(
   gfx::Rect screenshot_bounds(root_window_->layer()->size());
   gfx::Transform animation_layer_starting_transfrom;
 
-  if (!move_left_) {
+  if (starting_desk_index_ > ending_desk_index_) {
     // Starting desk is one the right, so we need to offset the screenshot layer
     // horizontally to the right by an amount equal to its width plus
     // kDesksSpacing (|x_translation_offset_|).
@@ -298,7 +297,7 @@ void RootWindowDeskSwitchAnimator::CompleteAnimationPhase1WithLayer(
   }
 
   starting_desk_screenshot_taken_ = true;
-  delegate_->OnStartingDeskScreenshotTaken(ending_desk_);
+  delegate_->OnStartingDeskScreenshotTaken(ending_desk_index_);
 }
 
 void RootWindowDeskSwitchAnimator::OnStartingDeskScreenshotTaken(
@@ -313,7 +312,7 @@ void RootWindowDeskSwitchAnimator::OnStartingDeskScreenshotTaken(
       LOG(ERROR) << "Received multiple empty screenshots of the starting desk.";
       NOTREACHED();
       starting_desk_screenshot_taken_ = true;
-      delegate_->OnStartingDeskScreenshotTaken(ending_desk_);
+      delegate_->OnStartingDeskScreenshotTaken(ending_desk_index_);
     }
 
     return;
@@ -346,7 +345,7 @@ void RootWindowDeskSwitchAnimator::OnEndingDeskScreenshotTaken(
 
   gfx::Rect screenshot_bounds(root_window_->layer()->size());
 
-  if (move_left_) {
+  if (starting_desk_index_ < ending_desk_index_) {
     // Starting desk is one the left, so we need to offset the ending desk
     // screenshot layer horizontally to the right by an amount equal to its
     // width plus kDesksSpacing (|x_translation_offset_|).

ash/wm/desks/root_window_desk_switch_animator.h

@@ -26,8 +26,6 @@ class CopyOutputResult;
 
 namespace ash {
 
-class Desk;
-
 // Performs the desk switch animation on a root window (i.e. display). Since a
 // desk spans all displays, one instance of this object will be created for each
 // display when a new desk is activated.
@@ -71,8 +69,8 @@ class Desk;
 //
 // - Phase (3) begins when StartAnimation() is called.
 //   * The parent layer of both screenshot layers is animated, either:
-//     - To the left (move_left_ == true); when the starting desk is on the
-//       left.
+//     - To the left (starting_desk_index_ < ending_desk_index_); when the
+//       starting desk is on the left.
 //
 //              <<<<<-------------------------- move left.
 //                       +-----------+
@@ -94,8 +92,8 @@ class Desk;
 //       * End transform: Negative translation to the left to slide out starting
 //         desk, and slide in ending desk screenshots into the screen.
 //
-//     - Or to the right (move_left_ == false), when the starting desk is on the
-//       right.
+//     - Or to the right (starting_desk_index_ > ending_desk_index_), when the
+//       starting desk is on the right.
 //
 //          move right. -------------------------->>>>>
 //                       +-----------+
@@ -159,9 +157,10 @@ class RootWindowDeskSwitchAnimator : public ui::ImplicitAnimationObserver {
   class Delegate {
    public:
     // Called when phase (1) completes. The starting desk screenshot has been
-    // taken and put on the screen. |ending_desk| is the desk that will be
-    // activated after all starting desk screenshots on all roots are taken.
-    virtual void OnStartingDeskScreenshotTaken(const Desk* ending_desk) = 0;
+    // taken and put on the screen. |ending_desk_index| is the index of the desk
+    // that will be activated after all starting desk screenshots on all roots
+    // are taken.
+    virtual void OnStartingDeskScreenshotTaken(int ending_desk_index) = 0;
 
     // Called when phase (2) completes. The ending desk screenshot has been
     // taken and put on the screen.
@@ -176,9 +175,9 @@ class RootWindowDeskSwitchAnimator : public ui::ImplicitAnimationObserver {
   };
 
   RootWindowDeskSwitchAnimator(aura::Window* root,
-                               const Desk* ending_desk,
+                               int starting_desk_index,
+                               int ending_desk_index,
                                Delegate* delegate,
-                               bool move_left,
                                bool for_remove);
 
   ~RootWindowDeskSwitchAnimator() override;
@@ -215,10 +214,11 @@ class RootWindowDeskSwitchAnimator : public ui::ImplicitAnimationObserver {
   // Completes the first phase of the animation using the given |layer| as the
   // screenshot layer of the starting desk. This layer will be parented to the
   // animation layer, which will be setup with its initial transform according
-  // to |move_left|. If |for_remove_| is true, the detached old layer tree of
-  // the soon-to-be-removed-desk's windows will be translated up vertically to
-  // simulate a jump from the removed desk to the target desk.
-  // |Delegate::OnStartingDeskScreenshotTaken()| will be called at the end.
+  // to |starting_desk_index_| and |ending_desk_index_|. If |for_remove_| is
+  // true, the detached old layer tree of the soon-to-be-removed-desk's windows
+  // will be translated up vertically to simulate a jump from the removed desk
+  // to the target desk. |Delegate::OnStartingDeskScreenshotTaken()| will be
+  // called at the end.
   void CompleteAnimationPhase1WithLayer(std::unique_ptr<ui::Layer> layer);
 
   void OnStartingDeskScreenshotTaken(
@@ -229,11 +229,11 @@ class RootWindowDeskSwitchAnimator : public ui::ImplicitAnimationObserver {
   // The root window that this animator is associated with.
   aura::Window* const root_window_;
 
-  // The active desk at the start of the animation.
-  const Desk* const starting_desk_;
+  // The index of the active desk at the start of the animation.
+  const int starting_desk_index_;
 
-  // The desk to activate and animate to with this animator.
-  const Desk* const ending_desk_;
+  // The index of the desk to activate and animate to with this animator.
+  const int ending_desk_index_;
 
   Delegate* const delegate_;
 
@@ -249,7 +249,7 @@ class RootWindowDeskSwitchAnimator : public ui::ImplicitAnimationObserver {
 
   // The amount by which the animation layer will be translated horizontally
   // either startingly or at the end of the animation, depending on the value of
-  // |move_left_|.
+  // of the desk indices.
   const int x_translation_offset_;
 
   // Number of retires for taking the starting and ending screenshots, if we
@@ -257,10 +257,6 @@ class RootWindowDeskSwitchAnimator : public ui::ImplicitAnimationObserver {
   int starting_desk_screenshot_retries_ = 0;
   int ending_desk_screenshot_retries_ = 0;
 
-  // True when the animation layer should be translated towards the left, which
-  // means the starting desk is on the left of the ending desk.
-  const bool move_left_;
-
   // True if this animator is handling the remove-active-desk animation.
   const bool for_remove_;