lacros: Supports "Move tab to another window" menu for lacros.
This CL enables "Move tab to another window" menu and shows
group windows by desk in the sub menu for lacros.
Bug: 1247900
Change-Id: I1c4e4925f77b0ed66db9923a6ba398ecdfe7ecb5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3157991
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Minju Kim <mkim@igalia.com>
Cr-Commit-Position: refs/heads/main@{#922374}
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 72ac8ef2..87cc710 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -7186,7 +7186,7 @@
<message name="IDS_TAB_CXMENU_MOVETOANOTHERNEWWINDOW" desc="The label for the Tab context item to move items to a new window from the 'Move tabs to another window' submenu.">
New window
</message>
- <if expr="chromeos">
+ <if expr="chromeos or lacros">
<message name="IDS_TAB_CXMENU_GROUPED_BY_DESK_MENU_ITEM_A11Y" desc="The a11y message for menu items in 'Move tabs to another window' when grouped by desk.">
<ph name="TAB_TITLE">$1<ex>Google News</ex></ph> belongs to desk <ph name="DESK_TITLE">$2<ex>School</ex></ph>
</message>
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index 3583d17..d322a5c 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -2210,8 +2210,6 @@
"settings_window_manager_chromeos.h",
"settings_window_manager_observer_chromeos.h",
"supervised_user/parent_permission_dialog.h",
- "tabs/existing_window_sub_menu_model_chromeos.cc",
- "tabs/existing_window_sub_menu_model_chromeos.h",
"views/apps/app_dialog/app_block_dialog_view.cc",
"views/apps/app_dialog/app_block_dialog_view.h",
"views/apps/app_dialog/app_pause_dialog_view.cc",
@@ -2973,6 +2971,8 @@
if (is_chromeos) {
sources += [
"platform_keys_certificate_selector_chromeos.h",
+ "tabs/existing_window_sub_menu_model_chromeos.cc",
+ "tabs/existing_window_sub_menu_model_chromeos.h",
"views/extensions/print_job_confirmation_dialog_view.cc",
"views/extensions/print_job_confirmation_dialog_view.h",
"views/frame/browser_frame_header_chromeos.cc",
@@ -2988,6 +2988,7 @@
"views/platform_keys_certificate_selector_chromeos.h",
]
deps += [
+ "//chrome/app:generated_resources",
"//chromeos/strings",
"//components/account_manager_core:account_manager_core",
]
diff --git a/chrome/browser/ui/tabs/existing_window_sub_menu_model.cc b/chrome/browser/ui/tabs/existing_window_sub_menu_model.cc
index 2835b2b..bfce20d 100644
--- a/chrome/browser/ui/tabs/existing_window_sub_menu_model.cc
+++ b/chrome/browser/ui/tabs/existing_window_sub_menu_model.cc
@@ -13,7 +13,7 @@
#include "chrome/grit/generated_resources.h"
#include "ui/base/accelerators/accelerator.h"
-#if BUILDFLAG(IS_CHROMEOS_ASH)
+#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
#include "chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.h"
#endif
@@ -30,8 +30,7 @@
TabMenuModelDelegate* tab_menu_model_delegate,
TabStripModel* model,
int context_index) {
- // TODO(crbug.com/1236618): Implement this for Lacros.
-#if BUILDFLAG(IS_CHROMEOS_ASH)
+#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
return std::make_unique<chromeos::ExistingWindowSubMenuModelChromeOS>(
GetPassKey(), parent_delegate, tab_menu_model_delegate, model,
context_index);
diff --git a/chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.cc b/chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.cc
index 5bfa16d..786c964 100644
--- a/chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.cc
+++ b/chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.cc
@@ -10,6 +10,7 @@
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/grit/generated_resources.h"
#include "chromeos/ui/wm/desks/desks_helper.h"
+#include "ui/aura/window.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/models/menu_model.h"
@@ -24,6 +25,21 @@
return desk_index;
}
+// Returns true if there are at least 2 desks.
+bool ShouldGroupByDesk(const DesksHelper* desks_helper) {
+ constexpr int kMinNumOfDesks = 2;
+ return desks_helper->GetNumberOfDesks() >= kMinNumOfDesks;
+}
+
+DesksHelper* GetDesksHelper(const std::vector<Browser*>& existing_browsers) {
+ DCHECK_GT(existing_browsers.size(), 0UL);
+ // It is OK to get DesksHelper from the window of the first existing browser
+ // since the APIs (GetNumberOfDesks, GetDeskName(index)) used by this class
+ // doesn't depend on the specific aura::Window.
+ return DesksHelper::Get(
+ (*existing_browsers.begin())->window()->GetNativeWindow());
+}
+
} // namespace
ExistingWindowSubMenuModelChromeOS::ExistingWindowSubMenuModelChromeOS(
@@ -36,15 +52,11 @@
parent_delegate,
tab_menu_model_delegate,
model,
- context_index),
- // This is currently ash only, but it might be under transion.
- // When this sub menu model is ported to Lacros in the future,
- // the corresponding aura::Window has to be passed for
- // DesksHelper::Get(window).
- desks_helper_(DesksHelper::Get(nullptr)) {
+ context_index) {
// If we shouldn't group by desk, ExistingWindowSubMenuModel's ctor has
// already built the menu.
- if (!ShouldGroupByDesk())
+ if (!ShouldGroupByDesk(GetDesksHelper(
+ tab_menu_model_delegate->GetExistingWindowsForMoveMenu())))
return;
ClearMenu();
@@ -59,7 +71,8 @@
const std::vector<Browser*>& existing_browsers) {
// Get the vector of MenuItemInfo for |existing_browsers| and then group them
// by desk.
- const int num_desks = desks_helper_->GetNumberOfDesks();
+ const DesksHelper* desks_helper = GetDesksHelper(existing_browsers);
+ const int num_desks = desks_helper->GetNumberOfDesks();
std::vector<std::vector<ExistingBaseSubMenuModel::MenuItemInfo>>
grouped_by_desk_menu_item_infos(num_desks);
for (ExistingBaseSubMenuModel::MenuItemInfo item :
@@ -68,7 +81,7 @@
existing_browsers[item.target_index.value()], num_desks);
item.accessible_name = l10n_util::GetStringFUTF16(
IDS_TAB_CXMENU_GROUPED_BY_DESK_MENU_ITEM_A11Y, item.text,
- desks_helper_->GetDeskName(desk));
+ desks_helper->GetDeskName(desk));
grouped_by_desk_menu_item_infos[desk].push_back(std::move(item));
}
@@ -82,7 +95,7 @@
continue;
// Create a MenuItemInfo for this desk.
- const std::u16string desk_name = desks_helper_->GetDeskName(desk);
+ const std::u16string desk_name = desks_helper->GetDeskName(desk);
ExistingBaseSubMenuModel::MenuItemInfo desk_heading(desk_name);
desk_heading.may_have_mnemonics = false;
desk_heading.accessible_name = l10n_util::GetStringFUTF16(
@@ -98,9 +111,4 @@
Build(IDS_TAB_CXMENU_MOVETOANOTHERNEWWINDOW, sorted_menu_items_with_headings);
}
-bool ExistingWindowSubMenuModelChromeOS::ShouldGroupByDesk() {
- constexpr int kMinNumOfDesks = 2;
- return desks_helper_->GetNumberOfDesks() >= kMinNumOfDesks;
-}
-
} // namespace chromeos
diff --git a/chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.h b/chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.h
index cb1e4a6b..d9fe2d5 100644
--- a/chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.h
+++ b/chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.h
@@ -39,11 +39,6 @@
// if browsers were provided in MRU order, then within desk groupings browsers
// will be in MRU order.
void BuildMenuGroupedByDesk(const std::vector<Browser*>& existing_browsers);
-
- // Returns true if there are at least 2 desks.
- bool ShouldGroupByDesk();
-
- const DesksHelper* const desks_helper_;
};
} // namespace chromeos
diff --git a/chrome/browser/ui/tabs/existing_window_sub_menu_model_unittest.cc b/chrome/browser/ui/tabs/existing_window_sub_menu_model_unittest.cc
index f6caea0..7ed00b5 100644
--- a/chrome/browser/ui/tabs/existing_window_sub_menu_model_unittest.cc
+++ b/chrome/browser/ui/tabs/existing_window_sub_menu_model_unittest.cc
@@ -20,8 +20,55 @@
#include "chrome/browser/ui/tabs/tab_menu_model_delegate.h"
#endif
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+#include "ui/views/widget/desktop_aura/desktop_native_widget_aura.h"
+#include "ui/views/widget/widget.h"
+#endif
+
namespace {
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+class TestBrowserWindowViewsWithDesktopNativeWidgetAura
+ : public TestBrowserWindow {
+ public:
+ explicit TestBrowserWindowViewsWithDesktopNativeWidgetAura(
+ bool popup = false);
+ TestBrowserWindowViewsWithDesktopNativeWidgetAura(
+ const TestBrowserWindowViewsWithDesktopNativeWidgetAura&) = delete;
+ TestBrowserWindowViewsWithDesktopNativeWidgetAura& operator=(
+ const TestBrowserWindowViewsWithDesktopNativeWidgetAura&) = delete;
+ ~TestBrowserWindowViewsWithDesktopNativeWidgetAura() override;
+
+ private:
+ std::unique_ptr<views::Widget> CreateDesktopWidget(bool popup);
+
+ std::unique_ptr<views::Widget> widget_;
+};
+
+TestBrowserWindowViewsWithDesktopNativeWidgetAura::
+ TestBrowserWindowViewsWithDesktopNativeWidgetAura(bool popup) {
+ widget_ = CreateDesktopWidget(popup);
+ SetNativeWindow(widget_->GetNativeWindow());
+}
+
+TestBrowserWindowViewsWithDesktopNativeWidgetAura::
+ ~TestBrowserWindowViewsWithDesktopNativeWidgetAura() = default;
+
+std::unique_ptr<views::Widget>
+TestBrowserWindowViewsWithDesktopNativeWidgetAura::CreateDesktopWidget(
+ bool popup) {
+ auto widget = std::make_unique<views::Widget>();
+ views::Widget::InitParams params;
+ params.type = popup ? views::Widget::InitParams::TYPE_POPUP
+ : views::Widget::InitParams::TYPE_WINDOW;
+ params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+ params.native_widget = new views::DesktopNativeWidgetAura(widget.get());
+ params.bounds = gfx::Rect(0, 0, 20, 20);
+ widget->Init(std::move(params));
+ return widget;
+}
+#endif
+
class ExistingWindowSubMenuModelTest : public BrowserWithTestWindowTest {
public:
ExistingWindowSubMenuModelTest() : BrowserWithTestWindowTest() {}
@@ -41,7 +88,12 @@
std::unique_ptr<Browser> ExistingWindowSubMenuModelTest::CreateTestBrowser(
bool incognito,
bool popup) {
+#if BUILDFLAG(IS_CHROMEOS_LACROS)
+ TestBrowserWindow* window =
+ new TestBrowserWindowViewsWithDesktopNativeWidgetAura(popup);
+#else
TestBrowserWindow* window = new TestBrowserWindow;
+#endif
new TestBrowserWindowOwner(window);
Profile* profile = incognito ? browser()->profile()->GetPrimaryOTRProfile(
/*create_if_needed=*/true)
