CloseAll: Add support for closing all desk windows

This CL gives the DesksController the ability to close all of a desk's
windows when the desk is being removed. It also adds test coverage for
removing a desk with all of its windows.

Bug: 1308426
Test: ash_unittests *DesksCloseAllTest*
Change-Id: I467ab6c116328e4b10dd19297e2df6f0f42cf3ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3546598
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Ben Becker <benbecker@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989998}

ash/accelerators/accelerator_controller_impl.cc

@@ -365,7 +365,8 @@ void HandleRemoveCurrentDesk() {
   // TODO(afakhry): Finalize the desk removal animation outside of overview with
   // UX. https://crbug.com/977434.
   desks_controller->RemoveDesk(desks_controller->active_desk(),
-                               DesksCreationRemovalSource::kKeyboard);
+                               DesksCreationRemovalSource::kKeyboard,
+                               /*close_windows=*/false);
   base::RecordAction(base::UserMetricsAction("Accel_Desks_RemoveDesk"));
 }
 

ash/wm/desks/autotest_desks_api.cc

@@ -179,7 +179,8 @@ bool AutotestDesksApi::RemoveActiveDesk(base::OnceClosure on_complete) {
 
   new DeskAnimationObserver(std::move(on_complete));
   controller->RemoveDesk(controller->active_desk(),
-                         DesksCreationRemovalSource::kButton);
+                         DesksCreationRemovalSource::kButton,
+                         /*close_windows=*/false);
   return true;
 }
 

ash/wm/desks/desk.cc

@@ -623,6 +623,42 @@ void Desk::RecordAndResetConsecutiveDailyVisits(bool being_removed) {
   first_day_visited_ = -1;
 }
 
+void Desk::CloseAllAppWindows() {
+  DCHECK(features::IsDesksCloseAllEnabled());
+
+  {
+    // We need to disable the desk notifying content has been changed here
+    // because the desk is going to be removed soon, so updating content here is
+    // unnecessary.
+    auto desk_throttled = GetScopedNotifyContentChangedDisabler();
+
+    // We need to copy the app windows from `windows_` into `app_windows` so
+    // that we do not modify `windows_` in place. This also gives us a filtered
+    // list with all of the app windows that we need to remove.
+    std::vector<aura::Window*> app_windows;
+    base::ranges::copy_if(
+        windows_, std::back_inserter(app_windows), [](aura::Window* window) {
+          return window->GetProperty(aura::client::kAppType) !=
+                 static_cast<int>(AppType::NON_APP);
+        });
+
+    // We initialize `window_tracker` from the filtered `app_windows` list, and
+    // pop and close windows from the `window_tracker` list. This avoids us
+    // revisiting windows that may have already been indirectly closed due to
+    // the closure of other windows, as the `window_tracker` automatically
+    // removes windows when they are closed.
+    aura::WindowTracker window_tracker(app_windows);
+    while (!window_tracker.windows().empty()) {
+      aura::Window* window = window_tracker.Pop();
+      views::Widget* widget = views::Widget::GetWidgetForNativeView(window);
+      DCHECK(widget);
+      widget->CloseNow();
+    }
+  }
+
+  NotifyContentChanged();
+}
+
 void Desk::MoveWindowToDeskInternal(aura::Window* window,
                                     Desk* target_desk,
                                     aura::Window* target_root) {

ash/wm/desks/desk.h

@@ -130,9 +130,9 @@ class ASH_EXPORT Desk {
   void Deactivate(bool update_window_activation);
 
   // In preparation for removing this desk, moves all the windows on this desk
-  // to |target_desk| such that they become last in MRU order across all desks,
+  // to `target_desk` such that they become last in MRU order across all desks,
   // and they will be stacked at the bottom among the children of
-  // |target_desk|'s container.
+  // `target_desk`'s container.
   // Note that from a UX stand point, removing a desk is viewed as the user is
   // now done with this desk, and therefore its windows are demoted and
   // deprioritized.
@@ -176,6 +176,12 @@ class ASH_EXPORT Desk {
   // accounts for cases where the user removes the active desk.
   void RecordAndResetConsecutiveDailyVisits(bool being_removed);
 
+  // Closes all app windows associated with this desk. If we are removing the
+  // active desk in overview, we do not want to close the non-app windows, but
+  // rather move them to the new active desk. So, we can close all of the app
+  // windows before moving the leftover windows.
+  void CloseAllAppWindows();
+
  private:
   friend class DesksTestApi;
 

ash/wm/desks/desk_action_context_menu.cc

@@ -12,20 +12,6 @@
 
 namespace ash {
 
-namespace {
-
-// An enum with identifiers to link context menu items to their associated
-// functions.
-enum CommandId {
-  // Closes target desk and moves its windows to another desk.
-  kCombineDesks,
-  // Saves target desk in DesksController and gives user option to undo the
-  // desk before the desk is fully removed and its windows are closed.
-  kCloseAll,
-};
-
-}  // namespace
-
 DeskActionContextMenu::DeskActionContextMenu(
     const std::u16string& initial_combine_desks_target_name,
     base::RepeatingClosure combine_desks_callback,

ash/wm/desks/desk_action_context_menu.h

@@ -20,6 +20,16 @@ namespace ash {
 class DeskActionContextMenu : public views::ContextMenuController,
                               public ui::SimpleMenuModel::Delegate {
  public:
+  // An enum with identifiers to link context menu items to their associated
+  // functions.
+  enum CommandId {
+    // Closes target desk and moves its windows to another desk.
+    kCombineDesks,
+    // Saves target desk in DesksController and gives user option to undo the
+    // desk before the desk is fully removed and its windows are closed.
+    kCloseAll,
+  };
+
   DeskActionContextMenu(const std::u16string& initial_combine_desks_target_name,
                         base::RepeatingClosure combine_desks_callback,
                         base::RepeatingClosure close_all_callback,

ash/wm/desks/desk_action_view.cc

@@ -10,6 +10,7 @@
 #include "ash/style/close_button.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
+#include "ui/compositor/layer.h"
 #include "ui/views/background.h"
 
 namespace ash {
@@ -46,6 +47,7 @@ DeskActionView::DeskActionView(
   UpdateCombineDesksTooltip(initial_combine_desks_target_name);
 
   SetPaintToLayer();
+  layer()->SetFillsBoundsOpaquely(false);
 }
 
 void DeskActionView::UpdateCombineDesksTooltip(

ash/wm/desks/desk_animation_impl.cc

@@ -371,7 +371,8 @@ void DeskRemovalAnimation::OnDeskSwitchAnimationFinishedInternal() {
   // Do the actual desk removal behind the scenes before the screenshot layers
   // are destroyed.
   controller_->RemoveDeskInternal(
-      controller_->desks()[desk_to_remove_index_].get(), request_source_);
+      controller_->desks()[desk_to_remove_index_].get(), request_source_,
+      /*close_windows=*/false);
 
   MaybeRestoreSplitView(/*refresh_snapped_windows=*/true);
 }

ash/wm/desks/desk_mini_view.cc

@@ -101,25 +101,29 @@ DeskMiniView::DeskMiniView(DesksBarView* owner_bar,
 
     desk_action_view_ = AddChildView(std::make_unique<DeskActionView>(
         initial_combine_desks_target_name,
+        /*combine_desks_callback=*/
         base::BindRepeating(&DeskMiniView::OnRemovingDesk,
-                            base::Unretained(this)),
+                            base::Unretained(this), /*close_windows=*/false),
+        /*close_all_callback=*/
         base::BindRepeating(&DeskMiniView::OnRemovingDesk,
-                            base::Unretained(this))));
+                            base::Unretained(this), /*close_windows=*/true)));
 
     context_menu_ = std::make_unique<DeskActionContextMenu>(
         initial_combine_desks_target_name,
+        /*combine_desks_callback=*/
         base::BindRepeating(&DeskMiniView::OnRemovingDesk,
-                            base::Unretained(this)),
+                            base::Unretained(this), /*close_windows=*/false),
+        /*close_all_callback=*/
         base::BindRepeating(&DeskMiniView::OnRemovingDesk,
-                            base::Unretained(this)),
+                            base::Unretained(this), /*close_windows=*/true),
         base::BindRepeating(&DeskMiniView::OnContextMenuClosed,
                             base::Unretained(this)));
 
     // TODO(crbug.com/1308429): Can initialize highlight overlay here.
   } else {
     close_desk_button_ = AddChildView(std::make_unique<CloseButton>(
         base::BindRepeating(&DeskMiniView::OnRemovingDesk,
-                            base::Unretained(this)),
+                            base::Unretained(this), /*close_windows=*/false),
         CloseButton::Type::kSmall));
   }
   UpdateDeskButtonVisibility();
@@ -355,7 +359,7 @@ void DeskMiniView::MaybeCloseHighlightedView() {
   // TODO(crbug.com/1307011): This function is called when we press ctrl+W
   // while highlighted over a desk mini view to combine desks. Should be
   // reworked when we add an accelerator for close-all.
-  OnRemovingDesk();
+  OnRemovingDesk(/*close_windows=*/false);
 }
 
 void DeskMiniView::MaybeSwapHighlightedView(bool right) {
@@ -537,7 +541,7 @@ void DeskMiniView::OnViewBlurred(views::View* observed_view) {
   desks_restore_util::UpdatePrimaryUserDeskNamesPrefs();
 }
 
-void DeskMiniView::OnRemovingDesk() {
+void DeskMiniView::OnRemovingDesk(bool close_windows) {
   if (!desk_)
     return;
 
@@ -557,10 +561,8 @@ void DeskMiniView::OnRemovingDesk() {
 
   desk_preview_->OnRemovingDesk();
 
-  // TODO(crbug.com/1308426): When adding the ability to close all windows with
-  // the desk, we will pass a boolean parameter that is given to this function
-  // during binding.
-  controller->RemoveDesk(desk_, DesksCreationRemovalSource::kButton);
+  controller->RemoveDesk(desk_, DesksCreationRemovalSource::kButton,
+                         close_windows);
 }
 
 void DeskMiniView::OnContextMenuClosed() {

ash/wm/desks/desk_mini_view.h

@@ -137,12 +137,13 @@ class ASH_EXPORT DeskMiniView : public views::View,
   void OnViewBlurred(views::View* observed_view) override;
 
  private:
+  friend class DesksTestApi;
+
   // Sets either the `desk_action_view_` or the `close_desk_button_` visibility
   // to false depending on whether the `kDesksCloseAll` feature is active, and
-  // then removes the desk.
-  // TODO(crbug.com/1308426): This function will take a boolean parameter when
-  // we add ability to close desk with all its windows.
-  void OnRemovingDesk();
+  // then removes the desk. If `close_windows` is true, the function tells the
+  // `DesksController` to remove `desk_`'s windows as well.
+  void OnRemovingDesk(bool close_windows);
 
   // Callback for when `context_menu_` is closed. Makes `desk_action_view_`
   // visible.

ash/wm/desks/desks_controller.cc

@@ -427,7 +427,8 @@ void DesksController::NewDesk(DesksCreationRemovalSource source) {
 }
 
 void DesksController::RemoveDesk(const Desk* desk,
-                                 DesksCreationRemovalSource source) {
+                                 DesksCreationRemovalSource source,
+                                 bool close_windows) {
   DCHECK(CanRemoveDesks());
   DCHECK(HasDesk(desk));
 
@@ -450,7 +451,7 @@ void DesksController::RemoveDesk(const Desk* desk,
     return;
   }
 
-  RemoveDeskInternal(desk, source);
+  RemoveDeskInternal(desk, source, close_windows);
 }
 
 void DesksController::ReorderDesk(int old_index, int new_index) {
@@ -1274,7 +1275,8 @@ void DesksController::ActivateDeskInternal(const Desk* desk,
 }
 
 void DesksController::RemoveDeskInternal(const Desk* desk,
-                                         DesksCreationRemovalSource source) {
+                                         DesksCreationRemovalSource source,
+                                         bool close_windows) {
   DCHECK(CanRemoveDesks());
 
   base::AutoReset<bool> in_progress(&are_desks_being_modified_, true);
@@ -1285,6 +1287,7 @@ void DesksController::RemoveDeskInternal(const Desk* desk,
   DCHECK(iter != desks_.end());
 
   const int removed_desk_index = std::distance(desks_.begin(), iter);
+
   // Update workspaces of windows in desks that have higher indices than the
   // removed desk since indices of those desks shift by one.
   for (int i = removed_desk_index + 1; i < static_cast<int>(desks_.size());
@@ -1296,7 +1299,7 @@ void DesksController::RemoveDeskInternal(const Desk* desk,
     }
   }
 
-  // Record |desk|'s lifetime before it's removed from |desks_|.
+  // Record `desk`'s lifetime before it's removed from `desks_`.
   auto* non_const_desk = const_cast<Desk*>(desk);
   non_const_desk->RecordLifetimeHistogram();
   non_const_desk->RecordAndResetConsecutiveDailyVisits(/*being_removed=*/true);
@@ -1319,28 +1322,14 @@ void DesksController::RemoveDeskInternal(const Desk* desk,
   auto removed_desk_mini_views_pauser =
       removed_desk->GetScopedNotifyContentChangedDisabler();
 
-  // - Move windows in removed desk (if any) to the currently active desk.
   // - If the active desk is the one being removed, activate the desk to its
   //   left, if no desk to the left, activate one on the right.
+  // - If we are not closing the windows, move the windows in removed desk (if
+  //   any) to the currently active desk.
+  // - Finally, if we are closing the windows and the active desk is not being
+  // removed, we just need to clear all of the windows in `removed_desk`.
   const bool will_switch_desks = (removed_desk.get() == active_desk_);
-  if (!will_switch_desks) {
-    // We will refresh the mini_views of the active desk only once at the end.
-    auto active_desk_mini_view_pauser =
-        active_desk_->GetScopedNotifyContentChangedDisabler();
-
-    removed_desk->MoveWindowsToDesk(active_desk_);
-
-    MaybeUpdateShelfItems({}, removed_desk_windows);
-
-    // If overview mode is active, we add the windows of the removed desk to the
-    // overview grid in the order of the new MRU (which changes after removing a
-    // desk by making the windows of the removed desk as the least recently used
-    // across all desks). Note that this can only be done after the windows have
-    // moved to the active desk in `MoveWindowsToDesk()` above, so that building
-    // the window MRU list should contain those windows.
-    if (in_overview)
-      AppendWindowsToOverview(removed_desk_windows);
-  } else {
+  if (will_switch_desks) {
     Desk* target_desk = nullptr;
     if (iter_after == desks_.begin()) {
       // Nothing before this desk.
@@ -1372,6 +1361,12 @@ void DesksController::RemoveDeskInternal(const Desk* desk,
     if (in_overview)
       RemoveAllWindowsFromOverview();
 
+    // TODO(crbug.com/1311314): Closing app windows before moving non-app
+    // windows may work for now, but when we add in the ability to save desks we
+    // will need to find a way to still maintain the app windows.
+    if (close_windows)
+      removed_desk->CloseAllAppWindows();
+
     // If overview mode is active, change desk activation without changing
     // window activation. Activation should remain on the dummy
     // "OverviewModeFocusedWidget" while overview mode is active.
@@ -1389,6 +1384,28 @@ void DesksController::RemoveDeskInternal(const Desk* desk,
     // all to the grid in the order of the new MRU.
     if (in_overview)
       AppendWindowsToOverview(target_desk->windows());
+  } else if (!close_windows) {
+    // We will refresh the mini_views of the active desk only once at the end.
+    auto active_desk_mini_view_pauser =
+        active_desk_->GetScopedNotifyContentChangedDisabler();
+
+    removed_desk->MoveWindowsToDesk(active_desk_);
+
+    MaybeUpdateShelfItems({}, removed_desk_windows);
+
+    // If overview mode is active, we add the windows of the removed desk to the
+    // overview grid in the order of the new MRU (which changes after removing a
+    // desk by making the windows of the removed desk as the least recently used
+    // across all desks). Note that this can only be done after the windows have
+    // moved to the active desk in `MoveWindowsToDesk()` above, so that building
+    // the window MRU list should contain those windows.
+    if (in_overview)
+      AppendWindowsToOverview(removed_desk_windows);
+  } else {
+    // In this case, we know that `close_windows` is true but that the desk we
+    // are closing is not currently active. So all that we need to do at this
+    // point is call `CloseAllAppWindows` on `removed_desk`.
+    removed_desk->CloseAllAppWindows();
   }
 
   // It's OK now to refresh the mini_views of *only* the active desk, and only

ash/wm/desks/desks_controller.h

@@ -144,11 +144,15 @@ class ASH_EXPORT DesksController : public chromeos::DesksHelper,
   // Creates a new desk. CanCreateDesks() must be checked before calling this.
   void NewDesk(DesksCreationRemovalSource source);
 
-  // Removes and deletes the given |desk|. |desk| must already exist, and
+  // Removes and deletes the given `desk`. `desk` must already exist, and
   // CanRemoveDesks() must be checked before this.
   // This will trigger the `DeskRemovalAnimation` if the active desk is being
   // removed outside of overview.
-  void RemoveDesk(const Desk* desk, DesksCreationRemovalSource source);
+  // If `close_windows` is true, the function will close all of the `desk`'s
+  // windows as well. Otherwise, it will move `desk`'s windows to another desk.
+  void RemoveDesk(const Desk* desk,
+                  DesksCreationRemovalSource source,
+                  bool close_windows);
 
   // Reorder the desk at |old_index| to |new_index|.
   void ReorderDesk(int old_index, int new_index);
@@ -324,7 +328,11 @@ class ASH_EXPORT DesksController : public chromeos::DesksHelper,
   void ActivateDeskInternal(const Desk* desk, bool update_window_activation);
 
   // Removes `desk` without animation.
-  void RemoveDeskInternal(const Desk* desk, DesksCreationRemovalSource source);
+  // If `close_windows` is true, the removed `desk`'s windows are closed along
+  // with the desk. Otherwise, they are moved to another desk.
+  void RemoveDeskInternal(const Desk* desk,
+                          DesksCreationRemovalSource source,
+                          bool close_windows);
 
   // Moves all the windows that are visible on all desks that currently
   // reside on |active_desk_| to |new_desk|.