Add 'move tab to existing window' submenu for tabbed apps

The menu will only show other windows that belong to the same app.

Bug: 1469955
Change-Id: I167202a8aae52a008e5004fc245554e0558ea156
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4778024
Commit-Queue: Louise Brett <loubrett@google.com>
Reviewed-by: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1187593}

chrome/browser/ui/browser_tab_menu_model_delegate.cc

@@ -7,6 +7,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tab_strip_model_delegate.h"
+#include "chrome/browser/ui/web_applications/app_browser_controller.h"
 
 namespace chrome {
 
@@ -15,16 +16,21 @@ BrowserTabMenuModelDelegate::BrowserTabMenuModelDelegate(Browser* browser)
 
 BrowserTabMenuModelDelegate::~BrowserTabMenuModelDelegate() = default;
 
-std::vector<Browser*>
-BrowserTabMenuModelDelegate::GetOtherTabbedBrowserWindows() {
+std::vector<Browser*> BrowserTabMenuModelDelegate::GetOtherBrowserWindows(
+    bool is_app) {
   std::vector<Browser*> browsers;
 
   for (Browser* browser : BrowserList::GetInstance()->OrderedByActivation()) {
     // We can only move into a tabbed view of the same profile, and not the same
     // window we're currently in.
-    if (browser != browser_ && browser->is_type_normal() &&
-        browser->profile() == browser_->profile()) {
-      browsers.push_back(browser);
+    if (browser != browser_ && browser->profile() == browser_->profile()) {
+      if (is_app && browser->is_type_app() &&
+          browser->app_controller()->app_id() ==
+              browser_->app_controller()->app_id()) {
+        browsers.push_back(browser);
+      } else if (!is_app && browser->is_type_normal()) {
+        browsers.push_back(browser);
+      }
     }
   }
   return browsers;

chrome/browser/ui/browser_tab_menu_model_delegate.h

@@ -22,7 +22,7 @@ class BrowserTabMenuModelDelegate : public TabMenuModelDelegate {
 
  private:
   // TabMenuModelDelegate:
-  std::vector<Browser*> GetOtherTabbedBrowserWindows() override;
+  std::vector<Browser*> GetOtherBrowserWindows(bool is_app) override;
 
   const raw_ptr<Browser, DanglingUntriaged> browser_;
 };

chrome/browser/ui/browser_tab_strip_model_delegate.cc

@@ -128,8 +128,9 @@ void BrowserTabStripModelDelegate::DuplicateContentsAt(int index) {
 void BrowserTabStripModelDelegate::MoveToExistingWindow(
     const std::vector<int>& indices,
     int browser_index) {
-  auto existing_browsers =
-      browser_->tab_menu_model_delegate()->GetOtherTabbedBrowserWindows();
+  std::vector<Browser*> existing_browsers =
+      browser_->tab_menu_model_delegate()->GetOtherBrowserWindows(
+          web_app::AppBrowserController::IsWebApp(browser_));
   size_t existing_browser_count = existing_browsers.size();
   if (static_cast<size_t>(browser_index) < existing_browser_count &&
       existing_browsers[browser_index]) {

chrome/browser/ui/tabs/existing_tab_group_sub_menu_model.cc

@@ -66,7 +66,7 @@ ExistingTabGroupSubMenuModel::ExistingTabGroupSubMenuModel(
   // For each window, append the tab groups to the end of the menu items.
   if (tab_menu_model_delegate_) {
     for (Browser* browser :
-         tab_menu_model_delegate_->GetOtherTabbedBrowserWindows()) {
+         tab_menu_model_delegate_->GetOtherBrowserWindows(/*is_app=*/false)) {
       if (browser->tab_strip_model() == model)
         continue;
       const std::vector<MenuItemInfo> retrieved_menu_item_infos =
@@ -159,7 +159,7 @@ bool ExistingTabGroupSubMenuModel::ShouldShowSubmenu(
   // Look at tab groups in all other windows
   if (tab_menu_model_delegate) {
     for (Browser* browser :
-         tab_menu_model_delegate->GetOtherTabbedBrowserWindows()) {
+         tab_menu_model_delegate->GetOtherBrowserWindows(/*is_app=*/false)) {
       TabGroupModel* browser_group_model =
           browser->tab_strip_model()->group_model();
       if (!browser_group_model)
@@ -201,7 +201,7 @@ void ExistingTabGroupSubMenuModel::ExecuteExistingCommand(size_t target_index) {
   // Find the index of the browser with the group we are looking for.
   absl::optional<size_t> browser_index;
   std::vector<Browser*> browsers =
-      tab_menu_model_delegate_->GetOtherTabbedBrowserWindows();
+      tab_menu_model_delegate_->GetOtherBrowserWindows(/*is_app=*/false);
   for (size_t i = 0; i < browsers.size(); ++i) {
     TabStripModel* potential_model = browsers[i]->tab_strip_model();
     if (potential_model && potential_model != model() &&

chrome/browser/ui/tabs/existing_window_sub_menu_model.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/tabs/tab_menu_model_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_delegate.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/accelerators/accelerator.h"
 
@@ -53,7 +54,8 @@ ExistingWindowSubMenuModel::ExistingWindowSubMenuModel(
                                TabStripModel::CommandMoveTabsToNewWindow) {
   Build(IDS_TAB_CXMENU_MOVETOANOTHERNEWWINDOW,
         BuildMenuItemInfoVectorForBrowsers(
-            tab_menu_model_delegate->GetOtherTabbedBrowserWindows()));
+            tab_menu_model_delegate->GetOtherBrowserWindows(
+                model->delegate()->IsForWebApp())));
 }
 
 ExistingWindowSubMenuModel::~ExistingWindowSubMenuModel() = default;
@@ -89,6 +91,12 @@ bool ExistingWindowSubMenuModel::ShouldShowSubmenu(Profile* profile) {
   return chrome::GetTabbedBrowserCount(profile) > 1;
 }
 
+bool ExistingWindowSubMenuModel::ShouldShowSubmenuForApp(
+    TabMenuModelDelegate* tab_menu_model_delegate) {
+  return tab_menu_model_delegate->GetOtherBrowserWindows(/*is_app=*/true)
+             .size() >= 1;
+}
+
 // static:
 base::PassKey<ExistingWindowSubMenuModel>
 ExistingWindowSubMenuModel::GetPassKey() {

chrome/browser/ui/tabs/existing_window_sub_menu_model.h

@@ -53,6 +53,8 @@ class ExistingWindowSubMenuModel : public ExistingBaseSubMenuModel {
   // the submenu would show at least one window. Does not assume ownership of
   // |model|; |model| must outlive this instance.
   static bool ShouldShowSubmenu(Profile* profile);
+  static bool ShouldShowSubmenuForApp(
+      TabMenuModelDelegate* tab_menu_model_delegate);
 
  protected:
   // Retrieves a base::Passkey which can be used to construct an instance of

chrome/browser/ui/tabs/existing_window_sub_menu_model_chromeos.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_menu_model_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_delegate.h"
 #include "chrome/grit/generated_resources.h"
 #include "chromeos/ui/wm/desks/desks_helper.h"
 #include "ui/aura/window.h"
@@ -64,8 +65,9 @@ ExistingWindowSubMenuModelChromeOS::ExistingWindowSubMenuModelChromeOS(
                                  context_index) {
   // If we shouldn't group by desk, ExistingWindowSubMenuModel's ctor has
   // already built the menu.
-  const std::vector<Browser*> tabbed_browser_windows =
-      tab_menu_model_delegate->GetOtherTabbedBrowserWindows();
+  std::vector<Browser*> tabbed_browser_windows =
+      tab_menu_model_delegate->GetOtherBrowserWindows(
+          model->delegate()->IsForWebApp());
   if (!ShouldGroupByDesk(GetDesksHelper(tabbed_browser_windows))) {
     return;
   }

chrome/browser/ui/tabs/existing_window_sub_menu_model_unittest.cc

@@ -387,7 +387,8 @@ TEST_F(ExistingWindowSubMenuModelTest, BuildSubmenuGroupedByDesks) {
       browser_7.get(), browser_5.get(), browser_4.get(),
       browser_2.get(), browser_3.get(), browser_6.get()};
   const auto& mru_ordered_windows =
-      browser()->tab_menu_model_delegate()->GetOtherTabbedBrowserWindows();
+      browser()->tab_menu_model_delegate()->GetOtherBrowserWindows(
+          /*is_app=*/false);
   ASSERT_EQ(6u, mru_ordered_windows.size());
   ASSERT_EQ(mru_ordered_windows, kExpectedMRUOrder);
 
@@ -444,7 +445,8 @@ TEST_F(ExistingWindowSubMenuModelTest, EnsureGroupedByDesksCommands) {
   const std::vector<Browser*> kExpectedMRUOrder{
       browser_4.get(), browser_2.get(), browser_3.get(), browser_5.get()};
   const auto& mru_ordered_windows =
-      browser()->tab_menu_model_delegate()->GetOtherTabbedBrowserWindows();
+      browser()->tab_menu_model_delegate()->GetOtherBrowserWindows(
+          /*is_app=*/false);
   ASSERT_EQ(4u, mru_ordered_windows.size());
   ASSERT_EQ(mru_ordered_windows, kExpectedMRUOrder);
 

chrome/browser/ui/tabs/tab_menu_model.cc

@@ -56,9 +56,20 @@ void TabMenuModel::BuildForWebApp(TabStripModel* tab_strip, int index) {
       (!web_app::HasPinnedHomeTab(tab_strip) ||
        *tab_strip->selection_model().selected_indices().begin() != 0)) {
     int num_tabs = tab_strip->selection_model().selected_indices().size();
-    AddItem(TabStripModel::CommandMoveTabsToNewWindow,
-            l10n_util::GetPluralStringFUTF16(
-                IDS_TAB_CXMENU_MOVE_TABS_TO_NEW_WINDOW, num_tabs));
+    if (ExistingWindowSubMenuModel::ShouldShowSubmenuForApp(
+            tab_menu_model_delegate_)) {
+      // Create submenu with existing windows
+      add_to_existing_window_submenu_ = ExistingWindowSubMenuModel::Create(
+          delegate(), tab_menu_model_delegate_, tab_strip, index);
+      AddSubMenu(TabStripModel::CommandMoveToExistingWindow,
+                 l10n_util::GetPluralStringFUTF16(
+                     IDS_TAB_CXMENU_MOVETOANOTHERWINDOW, num_tabs),
+                 add_to_existing_window_submenu_.get());
+    } else {
+      AddItem(TabStripModel::CommandMoveTabsToNewWindow,
+              l10n_util::GetPluralStringFUTF16(
+                  IDS_TAB_CXMENU_MOVE_TABS_TO_NEW_WINDOW, num_tabs));
+    }
   }
 
   AddSeparator(ui::NORMAL_SEPARATOR);

chrome/browser/ui/tabs/tab_menu_model_delegate.h

@@ -28,7 +28,7 @@ class TabMenuModelDelegate {
   // Returns a list of other existing browser windows that can accept menu
   // operations (i.e. Move tab to new window, Add tab to group) that are not the
   // current browser this was called on.
-  virtual std::vector<Browser*> GetOtherTabbedBrowserWindows() = 0;
+  virtual std::vector<Browser*> GetOtherBrowserWindows(bool is_app) = 0;
 };
 
 #endif  // CHROME_BROWSER_UI_TABS_TAB_MENU_MODEL_DELEGATE_H_

chrome/browser/ui/views/web_apps/web_app_tab_strip_browsertest.cc

@@ -16,6 +16,8 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tab_ui_helper.h"
+#include "chrome/browser/ui/tabs/existing_window_sub_menu_model.h"
+#include "chrome/browser/ui/tabs/tab_menu_model.h"
 #include "chrome/browser/ui/unload_controller.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
@@ -636,6 +638,7 @@ IN_PROC_BROWSER_TEST_F(WebAppTabStripBrowserTest, MoveTabsToNewWindow) {
   Browser* new_browser = BrowserList::GetInstance()->GetLastActive();
   EXPECT_NE(app_browser, new_browser);
   EXPECT_TRUE(AppBrowserController::IsForWebApp(new_browser, app_id));
+  EXPECT_EQ(app_browser->tab_strip_model()->count(), 1);
 
   // Check the new browser contains the moved tab and a pinned home tab.
   EXPECT_EQ(new_browser->tab_strip_model()->count(), 2);
@@ -646,6 +649,40 @@ IN_PROC_BROWSER_TEST_F(WebAppTabStripBrowserTest, MoveTabsToNewWindow) {
   EXPECT_EQ(new_browser->tab_strip_model()->active_index(), 1);
 }
 
+IN_PROC_BROWSER_TEST_F(WebAppTabStripBrowserTest, MoveTabsToExistingWindow) {
+  GURL start_url =
+      embedded_test_server()->GetURL("/web_apps/tab_strip_customizations.html");
+  AppId app_id = InstallTestWebApp(start_url);
+  Browser* app_browser = LaunchWebAppBrowser(app_id);
+  chrome::NewTab(app_browser);
+
+  // Open a second app browser window.
+  chrome::MoveTabsToNewWindow(app_browser, {1});
+  Browser* app_browser2 = BrowserList::GetInstance()->GetLastActive();
+
+  EXPECT_EQ(app_browser->tab_strip_model()->count(), 1);
+  EXPECT_EQ(app_browser2->tab_strip_model()->count(), 2);
+
+  // Test the "open in existing window" menu option.
+  TabMenuModel menu(nullptr, app_browser2->tab_menu_model_delegate(),
+                    app_browser2->tab_strip_model(), 1);
+  size_t submenu_index =
+      menu.GetIndexOfCommandId(TabStripModel::CommandMoveToExistingWindow)
+          .value();
+  ExistingWindowSubMenuModel* submenu =
+      static_cast<ExistingWindowSubMenuModel*>(
+          menu.GetSubmenuModelAt(submenu_index));
+  EXPECT_EQ(submenu->GetItemCount(), 3u);
+  EXPECT_EQ(submenu->GetLabelAt(0), u"New window");
+  EXPECT_EQ(submenu->GetTypeAt(1), ui::MenuModel::TYPE_SEPARATOR);
+  EXPECT_EQ(submenu->GetLabelAt(2), u"Manifest with tab strip customizations");
+
+  submenu->ExecuteCommand(submenu->GetCommandIdAt(2), 0);
+
+  EXPECT_EQ(app_browser2->tab_strip_model()->count(), 1);
+  EXPECT_EQ(app_browser->tab_strip_model()->count(), 2);
+}
+
 IN_PROC_BROWSER_TEST_F(WebAppTabStripBrowserTest,
                        OnlyNavigateHomeTabIfDifferentUrl) {
   GURL start_url =