Saved Desks: Add context menu button to desk

Add a button to the DeskActionView that opens up the context
menu for the given desk. This button replaces the combine
desks button if the Forest feature is enabled. This is part
of an effort to move the "Save as template" and "Save for
later" buttons to the context menu.

Bug: b/327639285
Test: ForestSavedDeskTest.*
Change-Id: I2cf75a693e0ddb49013edca3da26430789052b6d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5641375
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Elijah Hewer <hewer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1319868}

ash/ash_strings.grd

@@ -2714,6 +2714,9 @@ You can also use the keyboard shortcut. First, highlight text, then press <ph na
       <message name="IDS_ASH_DESKS_DESK_NAME_COMMIT" desc="Alert when the desk name is committed">
         Desk name was changed to <ph name="DESK_NAME">$1</ph>
       </message>
+      <message name="IDS_ASH_DESKS_CONTEXT_MENU_DESCRIPTION" translateable="false" desc="The tooltip that describes the action of opening the context menu for a desk.">
+        Open context menu
+      </message>
       <message name="IDS_ASH_DESKS_COMBINE_DESKS_DESCRIPTION" desc="The tooltip and context menu text that describes the action of closing a desk and moving its windows to another desk.">
         Combine with <ph name="DESK_NAME">$1</ph>
       </message>

ash/resources/vector_icons/BUILD.gn

@@ -559,6 +559,7 @@ aggregate_vector_icons("ash_vector_icons") {
     "system_tray_tracing.icon",
     "system_tray_update.icon",
     "system_tray_volume_mute.icon",
+    "three_dot_more.icon",
     "three_files.icon",
     "toolbar_position_bottom_center.icon",
     "toolbar_position_top_center.icon",

ash/resources/vector_icons/three_dot_more.icon

@@ -0,0 +1,17 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+CANVAS_DIMENSIONS, 20,
+MOVE_TO, 10, 13.2f,
+R_ARC_TO, 1.6f, 1.6f, 0, 1, 1, 0, 3.2f,
+R_ARC_TO, 1.6f, 1.6f, 0, 0, 1, 0, -3.2f,
+CLOSE,
+R_MOVE_TO, 0, -1.6f,
+R_ARC_TO, 1.6f, 1.6f, 0, 1, 1, 0, -3.2f,
+R_ARC_TO, 1.6f, 1.6f, 0, 0, 1, 0, 3.2f,
+CLOSE,
+R_MOVE_TO, 0, -4.8f,
+R_ARC_TO, 1.6f, 1.6f, 0, 1, 1, 0, -3.2f,
+R_ARC_TO, 1.6f, 1.6f, 0, 0, 1, 0, 3.2f,
+CLOSE

ash/wm/desks/desk_action_button.cc

@@ -22,6 +22,17 @@ namespace ash {
 
 namespace {
 
+const gfx::VectorIcon* GetVectorIconForType(DeskActionButton::Type type) {
+  switch (type) {
+    case DeskActionButton::Type::kContextMenu:
+      return &kThreeDotMoreIcon;
+    case DeskActionButton::Type::kCloseDesk:
+      return nullptr;
+    case DeskActionButton::Type::kCombineDesk:
+      return &kCombineDesksIcon;
+  }
+}
+
 constexpr int kDeskCloseButtonSize = 24;
 
 }  // namespace
@@ -32,7 +43,7 @@ DeskActionButton::DeskActionButton(const std::u16string& tooltip,
                                    DeskActionView* desk_action_view)
     : CloseButton(pressed_callback,
                   CloseButton::Type::kMediumFloating,
-                  type == Type::kCombineDesk ? &kCombineDesksIcon : nullptr),
+                  GetVectorIconForType(type)),
       type_(type),
       pressed_callback_(std::move(pressed_callback)),
       desk_action_view_(desk_action_view) {
@@ -90,18 +101,31 @@ bool DeskActionButton::CanShow() const {
     return false;
   }
 
-  // The close desk button can always show, while the combine desk button shows
-  // when its desk contains app windows or there are all desk windows.
+  // The close desk button can always show, while the combine desk button or
+  // the context menu button only show when their desk contains app windows or
+  // there are all desk windows.
   return type_ == Type::kCloseDesk ||
          desk_action_view_->mini_view()->desk()->ContainsAppWindows() ||
          !desk_controller->visible_on_all_desks_windows().empty();
 }
 
 void DeskActionButton::UpdateTooltip(const std::u16string& tooltip) {
-  SetTooltipText(l10n_util::GetStringFUTF16(
-      type_ == Type::kCombineDesk ? IDS_ASH_DESKS_COMBINE_DESKS_DESCRIPTION
-                                  : IDS_ASH_DESKS_CLOSE_ALL_DESCRIPTION,
-      tooltip));
+  int message_id;
+  switch (type_) {
+    case DeskActionButton::Type::kContextMenu:
+      // TODO(hewer): Upload string for translation.
+      message_id = IDS_ASH_DESKS_CONTEXT_MENU_DESCRIPTION;
+      break;
+    case DeskActionButton::Type::kCloseDesk:
+      message_id = IDS_ASH_DESKS_CLOSE_ALL_DESCRIPTION;
+      break;
+    case DeskActionButton::Type::kCombineDesk:
+      message_id = IDS_ASH_DESKS_COMBINE_DESKS_DESCRIPTION;
+      break;
+  }
+  SetTooltipText(type_ == Type::kContextMenu
+                     ? l10n_util::GetStringUTF16(message_id)
+                     : l10n_util::GetStringFUTF16(message_id, tooltip));
 }
 
 BEGIN_METADATA(DeskActionButton)

ash/wm/desks/desk_action_button.h

@@ -25,6 +25,7 @@ class ASH_EXPORT DeskActionButton : public CloseButton,
 
  public:
   enum class Type {
+    kContextMenu,
     kCombineDesk,
     kCloseDesk,
   };

ash/wm/desks/desk_action_view.cc

@@ -23,6 +23,7 @@ constexpr int kCornerRadius = 20;
 
 DeskActionView::DeskActionView(const std::u16string& combine_desks_target_name,
                                const std::u16string& close_all_target_name,
+                               base::RepeatingClosure context_menu_callback,
                                base::RepeatingClosure combine_desks_callback,
                                base::RepeatingClosure close_all_callback,
                                base::RepeatingClosure focus_change_callback,
@@ -35,30 +36,49 @@ DeskActionView::DeskActionView(const std::u16string& combine_desks_target_name,
       this, kColorAshShieldAndBase80, ColorProvider::kBackgroundBlurSigma,
       gfx::RoundedCornersF(kCornerRadius));
 
-  combine_desks_button_ = AddChildView(std::make_unique<DeskActionButton>(
-      combine_desks_target_name, DeskActionButton::Type::kCombineDesk,
-      std::move(combine_desks_callback), this));
+  // The "Save desk as template" and "Save desk for later" buttons are being
+  // merged into the desk action context menu, behind the Forest feature flag.
+  // Thus, we replace the combine desks button with a button to open the context
+  // menu if the feature is enabled.
+  if (features::IsForestFeatureEnabled()) {
+    context_menu_button_ = AddChildView(std::make_unique<DeskActionButton>(
+        std::u16string(), DeskActionButton::Type::kContextMenu,
+        std::move(context_menu_callback), this));
+    context_menu_button_->AddObserver(this);
+  } else {
+    combine_desks_button_ = AddChildView(std::make_unique<DeskActionButton>(
+        combine_desks_target_name, DeskActionButton::Type::kCombineDesk,
+        std::move(combine_desks_callback), this));
+    combine_desks_button_->AddObserver(this);
+  }
 
   close_all_button_ = AddChildView(std::make_unique<DeskActionButton>(
       close_all_target_name, DeskActionButton::Type::kCloseDesk,
       std::move(close_all_callback), this));
-
-  combine_desks_button_->AddObserver(this);
   close_all_button_->AddObserver(this);
 }
 
 DeskActionView::~DeskActionView() {
-  combine_desks_button_->RemoveObserver(this);
+  if (features::IsForestFeatureEnabled()) {
+    context_menu_button_->RemoveObserver(this);
+  } else {
+    combine_desks_button_->RemoveObserver(this);
+  }
   close_all_button_->RemoveObserver(this);
 }
 
 bool DeskActionView::ChildHasFocus() const {
   if (mini_view_->owner_bar()->type() == DeskBarViewBase::Type::kOverview &&
       !features::IsOverviewNewFocusEnabled()) {
-    return combine_desks_button_->is_focused() ||
+    return (features::IsForestFeatureEnabled()
+                ? context_menu_button_->is_focused()
+                : combine_desks_button_->is_focused()) ||
            close_all_button_->is_focused();
   }
-  return combine_desks_button_->HasFocus() || close_all_button_->HasFocus();
+  return (features::IsForestFeatureEnabled()
+              ? context_menu_button_->HasFocus()
+              : combine_desks_button_->HasFocus()) ||
+         close_all_button_->HasFocus();
 }
 
 void DeskActionView::OnViewFocused(views::View* observed) {

ash/wm/desks/desk_action_view.h

@@ -28,6 +28,7 @@ class ASH_EXPORT DeskActionView : public views::BoxLayoutView,
  public:
   DeskActionView(const std::u16string& combine_desks_target_name,
                  const std::u16string& close_all_target_name,
+                 base::RepeatingClosure context_menu_callback,
                  base::RepeatingClosure combine_desks_callback,
                  base::RepeatingClosure close_all_callback,
                  base::RepeatingClosure focus_change_callback,
@@ -36,6 +37,8 @@ class ASH_EXPORT DeskActionView : public views::BoxLayoutView,
   DeskActionView& operator=(const DeskActionView&) = delete;
   ~DeskActionView() override;
 
+  DeskActionButton* context_menu_button() { return context_menu_button_; }
+
   const DeskActionButton* close_all_button() const { return close_all_button_; }
   DeskActionButton* close_all_button() { return close_all_button_; }
 
@@ -58,7 +61,11 @@ class ASH_EXPORT DeskActionView : public views::BoxLayoutView,
   void OnViewFocused(views::View* observed) override;
   void OnViewBlurred(views::View* observed) override;
 
+  // Only one of the following two buttons will be shown, based on if the Forest
+  // feature is enabled.
+  raw_ptr<DeskActionButton> context_menu_button_;
   raw_ptr<DeskActionButton> combine_desks_button_;
+
   raw_ptr<DeskActionButton> close_all_button_;
 
   // Maintains blurred rounded rect background without clipping. Useful when

ash/wm/desks/desk_bar_view_base.cc

@@ -1335,8 +1335,10 @@ void DeskBarViewBase::OnDeskRemoved(const Desk* desk) {
       focus_cycler->OnViewDestroyingOrDisabling((*iter)->desk_name_view());
       focus_cycler->OnViewDestroyingOrDisabling(
           (*iter)->desk_action_view()->close_all_button());
-      focus_cycler->OnViewDestroyingOrDisabling(
-          (*iter)->desk_action_view()->combine_desks_button());
+      if (auto* combine_desks_button =
+              (*iter)->desk_action_view()->combine_desks_button()) {
+        focus_cycler->OnViewDestroyingOrDisabling(combine_desks_button);
+      }
       if (auto* desk_profiles_button = (*iter)->desk_profiles_button()) {
         focus_cycler->OnViewDestroyingOrDisabling(desk_profiles_button);
       }
@@ -1731,8 +1733,13 @@ void DeskBarViewBase::MaybeUpdateDeskActionButtonTooltips() {
         desk->name().empty() && desk_index != -1
             ? desk_controller->GetDeskDefaultName(desk_index)
             : desk->name();
-    desk_action_view->combine_desks_button()->UpdateTooltip(
-        combine_desk_tooltip);
+    // The combine desks button only exists if the forest feature is disabled.
+    // The context menu button that would appear in its place does not need to
+    // update its tooltip as it doesn't use a formatted string.
+    if (!features::IsForestFeatureEnabled()) {
+      desk_action_view->combine_desks_button()->UpdateTooltip(
+          combine_desk_tooltip);
+    }
     desk_action_view->close_all_button()->UpdateTooltip(close_desk_tooltip);
   }
 }

ash/wm/desks/desk_mini_view.cc

@@ -200,9 +200,12 @@ DeskMiniView::DeskMiniView(
   }
 
   desk_action_view_ = AddChildView(std::make_unique<DeskActionView>(
-      /*combine_desks_target_name=*/DesksController::Get()
-          ->GetCombineDesksTargetName(desk_),
+      /*combine_desks_target_name=*/
+      DesksController::Get()->GetCombineDesksTargetName(desk_),
       /*close_all_target_name=*/desk_->name(),
+      /*context_menu_callback_=*/
+      base::BindRepeating(&DeskMiniView::OpenContextMenu,
+                          base::Unretained(this), ui::MENU_SOURCE_NONE),
       /*combine_desks_callback=*/
       base::BindRepeating(&DeskMiniView::OnRemovingDesk, base::Unretained(this),
                           DeskCloseType::kCombineDesks),
@@ -328,9 +331,14 @@ void DeskMiniView::UpdateDeskButtonVisibility() {
   // Only show the combine desks button if there are app windows in the desk,
   // or if the desk is active and there are windows that should be visible on
   // all desks.
-  auto* combine_desks_button = desk_action_view_->combine_desks_button();
+  if (features::IsForestFeatureEnabled()) {
+    auto* context_menu_button = desk_action_view_->context_menu_button();
+    context_menu_button->SetVisible(context_menu_button->CanShow());
+  } else {
+    auto* combine_desks_button = desk_action_view_->combine_desks_button();
+    combine_desks_button->SetVisible(combine_desks_button->CanShow());
+  }
   auto* close_all_button = desk_action_view_->close_all_button();
-  combine_desks_button->SetVisible(combine_desks_button->CanShow());
   close_all_button->SetVisible(close_all_button->CanShow());
   desk_action_view_->SetVisible(visible && !is_context_menu_open_);
 
@@ -577,8 +585,10 @@ void DeskMiniView::OnPreviewOrProfileAboutToBeFocusedByReverseTab() {
        (desk_profile_button_ == nullptr ||
         !desk_profile_button_->HasFocus()))) {
     auto* combine_desks_button = desk_action_view_->combine_desks_button();
+    if (combine_desks_button) {
+      combine_desks_button->SetVisible(combine_desks_button->CanShow());
+    }
     auto* close_all_button = desk_action_view_->close_all_button();
-    combine_desks_button->SetVisible(combine_desks_button->CanShow());
     close_all_button->SetVisible(close_all_button->CanShow());
     desk_action_view_->SetVisible(true);
     desk_action_view_->close_all_button()->RequestFocus();

ash/wm/desks/desks_test_util.cc

@@ -164,8 +164,9 @@ const CloseButton* GetCloseDeskButtonForMiniView(
   // When there are no windows on the desk, the `combine_desks_button` is not
   // visible, so we need to use the `close_all_button`
   const DeskActionView* desk_action_view = mini_view->desk_action_view();
-  return desk_action_view->combine_desks_button()->GetVisible()
-             ? desk_action_view->combine_desks_button()
+  auto* combine_desks_button = desk_action_view->combine_desks_button();
+  return (combine_desks_button && combine_desks_button->GetVisible())
+             ? combine_desks_button
              : desk_action_view->close_all_button();
 }
 

ash/wm/desks/templates/saved_desk_unittest.cc

@@ -414,7 +414,7 @@ class SavedDeskTest : public OverviewTestBase,
   // OverviewTestBase:
   void SetUp() override {
     scoped_feature_list_.InitWithFeatures(
-        {features::kDesksTemplates, features::kForestFeature,
+        {features::kDesksTemplates,
          features::kDeskBarWindowOcclusionOptimization},
         {});
     OverviewTestBase::SetUp();
@@ -455,8 +455,9 @@ class SavedDeskTest : public OverviewTestBase,
   // tear down.
   std::unique_ptr<ui::ScopedAnimationDurationScaleMode> animation_scale_;
 
- private:
   base::test::ScopedFeatureList scoped_feature_list_;
+
+ private:
   std::unique_ptr<MultiUserWindowManager> multi_user_window_manager_;
   AccountId account_id_test_;
 };
@@ -4838,9 +4839,27 @@ TEST_F(SavedDeskTest, NoCrashDuringGuest) {
   EnterTabletMode();
 }
 
+class ForestSavedDeskTest : public SavedDeskTest {
+ public:
+  ForestSavedDeskTest() = default;
+  ForestSavedDeskTest(const ForestSavedDeskTest&) = delete;
+  ForestSavedDeskTest& operator=(const ForestSavedDeskTest&) = delete;
+  ~ForestSavedDeskTest() override = default;
+
+  void TearDown() override {
+    // Due to the nested ScopedFeatureLists, `scoped_feature_list_` must be
+    // reset before `this` is destroyed and `forest_feature_list_` is reset.
+    scoped_feature_list_.Reset();
+    SavedDeskTest::TearDown();
+  }
+
+ private:
+  base::test::ScopedFeatureList forest_feature_list_{features::kForestFeature};
+};
+
 // Tests that the layout of the desk mini view context menu is correct, and the
 // items are enabled and visible.
-TEST_F(SavedDeskTest, ContextMenuLayout) {
+TEST_F(ForestSavedDeskTest, ContextMenuLayout) {
   // Add a window and an empty desk.
   auto test_window = CreateAppWindow();
   NewDesk();
@@ -4875,9 +4894,42 @@ TEST_F(SavedDeskTest, ContextMenuLayout) {
   }
 }
 
+// Tests that the context menu button in the desk action view is visible under
+// the right conditions.
+TEST_F(ForestSavedDeskTest, ContextMenuButtonVisibility) {
+  // Add a window and an empty desk.
+  auto test_window = CreateAppWindow();
+  NewDesk();
+
+  // Enter overview. There should be two mini views, one to represent the desk
+  // with the window, and one to represent the empty desk.
+  ToggleOverview();
+  ASSERT_TRUE(GetOverviewSession());
+  ASSERT_EQ(2u, GetPrimaryRootDesksBarView()->mini_views().size());
+
+  // The context menu button should be visible on the first desk as it contains
+  // a window.
+  DeskMiniView* window_mini_view =
+      GetPrimaryRootDesksBarView()->mini_views()[0];
+  auto* context_menu_button =
+      window_mini_view->desk_action_view()->context_menu_button();
+  ASSERT_TRUE(context_menu_button);
+  EXPECT_TRUE(context_menu_button->GetVisible());
+  EXPECT_FALSE(window_mini_view->desk_action_view()->combine_desks_button());
+
+  // The context menu button should not be visible on the second desk as it does
+  // not contain a window.
+  DeskMiniView* empty_mini_view = GetPrimaryRootDesksBarView()->mini_views()[1];
+  context_menu_button =
+      empty_mini_view->desk_action_view()->context_menu_button();
+  ASSERT_TRUE(context_menu_button);
+  EXPECT_FALSE(context_menu_button->GetVisible());
+  EXPECT_FALSE(empty_mini_view->desk_action_view()->combine_desks_button());
+}
+
 // Tests that the "Save As Template" option in the desk mini view context menu
 // works as intended.
-TEST_F(SavedDeskTest, ContextMenuSaveAsTemplate) {
+TEST_F(ForestSavedDeskTest, ContextMenuSaveAsTemplate) {
   // Add a window and an empty desk.
   auto test_window = CreateAppWindow();
   NewDesk();
@@ -4919,7 +4971,7 @@ TEST_F(SavedDeskTest, ContextMenuSaveAsTemplate) {
 
 // Tests that the "Save For Later" option in the desk mini view context menu
 // works as intended.
-TEST_F(SavedDeskTest, ContextMenuSaveForLater) {
+TEST_F(ForestSavedDeskTest, ContextMenuSaveForLater) {
   // Create a test window that we release immediately as it will be closed
   // automatically by the code under test. Also create an empty desk.
   CreateAppWindow().release();

ash/wm/overview/overview_focus_cycler_old.cc

@@ -57,7 +57,8 @@ void AddDesksBarTraversableViews(
         out_traversable_views.push_back(profiles_button);
       }
       auto* desk_action_view = mini_view->desk_action_view();
-      if (desk_action_view->combine_desks_button()->CanShow()) {
+      if (desk_action_view->combine_desks_button() &&
+          desk_action_view->combine_desks_button()->CanShow()) {
         out_traversable_views.push_back(
             desk_action_view->combine_desks_button());
       }

chrome/browser/ui/ash/desks/desks_interactive_uitest.cc

@@ -18,6 +18,7 @@
 #include "base/functional/bind.h"
 #include "base/memory/raw_ptr.h"
 #include "base/test/gtest_tags.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/test_switches.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/test/base/ash/interactive/interactive_ash_test.h"
@@ -53,6 +54,10 @@ DEFINE_LOCAL_STATE_IDENTIFIER_VALUE(DeskBarExpandedObserver,
 
 class DesksInteractiveUiTest : public InteractiveAshTest {
  public:
+  DesksInteractiveUiTest() {
+    scoped_feature_list_.InitAndDisableFeature({features::kForestFeature});
+  }
+
   // This function is used to name the DeskMiniView that is associated with the
   // desk at `desk_index`. This will find the right view regardless of how the
   // mini views may be organized as child views.
@@ -72,6 +77,9 @@ class DesksInteractiveUiTest : public InteractiveAshTest {
             },
             desk_index));
   }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
 };
 
 IN_PROC_BROWSER_TEST_F(DesksInteractiveUiTest, DesksBasic) {