Add command to pop tab out into a new window without dragging

In this CL, it's exposed through the Mac tab menu. A follow-up change will also add it to the tab context menu.

Bug: 1021551
Change-Id: I3ca1e67fa5a2215a869016c2a2eeb9a946d79ea0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1954178
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726427}

chrome/app/chrome_command_ids.h

@@ -68,6 +68,7 @@
 #endif
 
 #define IDC_OPEN_IN_PWA_WINDOW          34053
+#define IDC_MOVE_TAB_TO_NEW_WINDOW          34054
 
 // Web app window commands
 #define IDC_COPY_URL                    34060

chrome/app/generated_resources.grd

@@ -989,6 +989,9 @@ are declared in tools/grit/grit_rule.gni.
           <message name="IDS_OPEN_IN_APP_WINDOW" desc="The text label of the menu item for moving the current tab to a standalone app window">
             Open in <ph name="APP">$1<ex>Gmail App</ex></ph>
           </message>
+          <message name="IDS_MOVE_TAB_TO_NEW_WINDOW" desc="The text label of the Move Tab to Window menu item.">
+            Move tab to new window
+          </message>
         </if>
         <if expr="use_titlecase">
           <message name="IDS_NEW_TAB" desc="In Title Case: The text label of a menu item for opening a new tab">
@@ -1060,6 +1063,9 @@ are declared in tools/grit/grit_rule.gni.
           <message name="IDS_OPEN_IN_APP_WINDOW" desc="In Title Case: The text label of the menu item for moving the current tab to a standalone app window">
             Open in <ph name="APP">$1<ex>Gmail App</ex></ph>
           </message>
+          <message name="IDS_MOVE_TAB_TO_NEW_WINDOW" desc="In Title Case: The text label of the Move Tab to New Window menu item.">
+            Move Tab to New Window
+          </message>
         </if>
       </if>
 

chrome/browser/ui/browser_command_controller.cc

@@ -464,6 +464,9 @@ bool BrowserCommandController::ExecuteCommandWithDisposition(
       base::RecordAction(base::UserMetricsAction("OpenActiveTabInPwaWindow"));
       web_app::ReparentWebAppForSecureActiveTab(browser_);
       break;
+    case IDC_MOVE_TAB_TO_NEW_WINDOW:
+      MoveTabToNewWindow(browser_);
+      break;
 
 #if defined(OS_CHROMEOS)
     case IDC_VISIT_DESKTOP_OF_LRU_USER_2:
@@ -1140,6 +1143,8 @@ void BrowserCommandController::UpdateCommandsForTabState() {
                                         CanCloseTabsToRight(browser_));
   command_updater_.UpdateCommandEnabled(IDC_WINDOW_CLOSE_OTHER_TABS,
                                         CanCloseOtherTabs(browser_));
+  command_updater_.UpdateCommandEnabled(IDC_MOVE_TAB_TO_NEW_WINDOW,
+                                        CanMoveTabToNewWindow(browser_));
 
   // Page-related commands
   window()->SetStarredState(

chrome/browser/ui/browser_commands.cc

@@ -165,6 +165,37 @@ translate::TranslateBubbleUiEvent TranslateBubbleResultToUiEvent(
   }
 }
 
+// Creates a new tabbed browser window, with the same size, type and profile as
+// |original_browser|'s window, inserts |contents| into it, and shows it.
+void CreateAndShowNewWindowWithContents(
+    std::unique_ptr<content::WebContents> contents,
+    const Browser* original_browser) {
+  Browser* new_browser = nullptr;
+  if (original_browser->deprecated_is_app()) {
+    new_browser = new Browser(Browser::CreateParams::CreateForApp(
+        original_browser->app_name(), original_browser->is_trusted_source(),
+        gfx::Rect(), original_browser->profile(), true));
+  } else {
+    new_browser = new Browser(Browser::CreateParams(
+        original_browser->type(), original_browser->profile(), true));
+  }
+  // Preserve the size of the original window. The new window has already
+  // been given an offset by the OS, so we shouldn't copy the old bounds.
+  BrowserWindow* new_window = new_browser->window();
+  new_window->SetBounds(
+      gfx::Rect(new_window->GetRestoredBounds().origin(),
+                original_browser->window()->GetRestoredBounds().size()));
+
+  // We need to show the browser now.  Otherwise ContainerWin assumes the
+  // WebContents is invisible and won't size it.
+  new_browser->window()->Show();
+
+  // The page transition below is only for the purpose of inserting the tab.
+  new_browser->tab_strip_model()->AddWebContents(std::move(contents), -1,
+                                                 ui::PAGE_TRANSITION_LINK,
+                                                 TabStripModel::ADD_ACTIVE);
+}
+
 }  // namespace
 
 using base::UserMetricsAction;
@@ -728,6 +759,17 @@ bool CanDuplicateKeyboardFocusedTab(const Browser* browser) {
   return CanDuplicateTabAt(browser, *GetKeyboardFocusedTabIndex(browser));
 }
 
+bool CanMoveTabToNewWindow(Browser* browser) {
+  return browser->tab_strip_model()->count() > 1;
+}
+
+void MoveTabToNewWindow(Browser* browser) {
+  int index = browser->tab_strip_model()->active_index();
+  auto contents = browser->tab_strip_model()->DetachWebContentsAt(index);
+  CHECK(contents);
+  CreateAndShowNewWindowWithContents(std::move(contents), browser);
+}
+
 bool CanCloseTabsToRight(const Browser* browser) {
   return browser->tab_strip_model()->IsContextMenuCommandEnabled(
       browser->tab_strip_model()->active_index(),
@@ -760,30 +802,7 @@ WebContents* DuplicateTabAt(Browser* browser, int index) {
     tab_strip_model->InsertWebContentsAt(index + 1, std::move(contents_dupe),
                                          add_types, old_group);
   } else {
-    Browser* new_browser = nullptr;
-    if (browser->deprecated_is_app()) {
-      new_browser = new Browser(Browser::CreateParams::CreateForApp(
-          browser->app_name(), browser->is_trusted_source(), gfx::Rect(),
-          browser->profile(), true));
-    } else {
-      new_browser = new Browser(
-          Browser::CreateParams(browser->type(), browser->profile(), true));
-    }
-    // Preserve the size of the original window. The new window has already
-    // been given an offset by the OS, so we shouldn't copy the old bounds.
-    BrowserWindow* new_window = new_browser->window();
-    new_window->SetBounds(
-        gfx::Rect(new_window->GetRestoredBounds().origin(),
-                  browser->window()->GetRestoredBounds().size()));
-
-    // We need to show the browser now.  Otherwise ContainerWin assumes the
-    // WebContents is invisible and won't size it.
-    new_browser->window()->Show();
-
-    // The page transition below is only for the purpose of inserting the tab.
-    new_browser->tab_strip_model()->AddWebContents(std::move(contents_dupe), -1,
-                                                   ui::PAGE_TRANSITION_LINK,
-                                                   TabStripModel::ADD_ACTIVE);
+    CreateAndShowNewWindowWithContents(std::move(contents_dupe), browser);
   }
 
   SessionService* session_service =

chrome/browser/ui/browser_commands.h

@@ -105,6 +105,8 @@ void SelectLastTab(
 void DuplicateTab(Browser* browser);
 bool CanDuplicateTab(const Browser* browser);
 bool CanDuplicateKeyboardFocusedTab(const Browser* browser);
+void MoveTabToNewWindow(Browser* browser);
+bool CanMoveTabToNewWindow(Browser* browser);
 bool CanCloseTabsToRight(const Browser* browser);
 bool CanCloseOtherTabs(const Browser* browser);
 content::WebContents* DuplicateTabAt(Browser* browser, int index);

chrome/browser/ui/browser_commands_browsertest.cc

@@ -6,8 +6,10 @@
 
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
 
 namespace chrome {
 
@@ -65,4 +67,41 @@ IN_PROC_BROWSER_TEST_F(BrowserCommandsTest, ReloadSelectedTabs) {
   EXPECT_EQ(kTabCount, load_sum);
 }
 
+// Tests IDC_MOVE_TAB_TO_NEW_WINDOW. This is a browser test and not a unit test
+// since it needs to create a new browser window, which doesn't work with a
+// TestingProfile.
+IN_PROC_BROWSER_TEST_F(BrowserCommandsTest, MoveTabToNewWindow) {
+  GURL url1("chrome://version");
+  GURL url2("chrome://about");
+  ui_test_utils::NavigateToURL(browser(), url1);
+
+  // Should be disabled with 1 tab.
+  EXPECT_FALSE(chrome::IsCommandEnabled(browser(), IDC_MOVE_TAB_TO_NEW_WINDOW));
+  AddTabAtIndex(1, url2, ui::PAGE_TRANSITION_LINK);
+  // Two tabs is enough for it to be meaningful to pop one out.
+  EXPECT_TRUE(chrome::IsCommandEnabled(browser(), IDC_MOVE_TAB_TO_NEW_WINDOW));
+
+  BrowserList* browser_list = BrowserList::GetInstance();
+  // Pre-command, assert that we have one browser, with two tabs, with the
+  // url2 tab active.
+  EXPECT_EQ(browser_list->size(), 1u);
+  EXPECT_EQ(browser()->tab_strip_model()->count(), 2);
+  EXPECT_EQ(browser()->tab_strip_model()->GetActiveWebContents()->GetURL(),
+            url2);
+
+  chrome::ExecuteCommand(browser(), IDC_MOVE_TAB_TO_NEW_WINDOW);
+
+  // Now we should have: two browsers, each with one tab (url1 in browser(),
+  // and url2 in the new one).
+  Browser* active_browser = browser_list->GetLastActive();
+  EXPECT_EQ(browser_list->size(), 2u);
+  EXPECT_NE(active_browser, browser());
+  EXPECT_EQ(browser()->tab_strip_model()->count(), 1);
+  EXPECT_EQ(active_browser->tab_strip_model()->count(), 1);
+  EXPECT_EQ(browser()->tab_strip_model()->GetActiveWebContents()->GetURL(),
+            url1);
+  EXPECT_EQ(active_browser->tab_strip_model()->GetActiveWebContents()->GetURL(),
+            url2);
+}
+
 }  // namespace chrome

chrome/browser/ui/cocoa/main_menu_builder.mm

@@ -424,6 +424,8 @@ base::scoped_nsobject<NSMenuItem> BuildTabMenu(
                   .command_id(IDC_WINDOW_CLOSE_OTHER_TABS),
               Item(IDS_TAB_CXMENU_CLOSETABSTORIGHT)
                   .command_id(IDC_WINDOW_CLOSE_TABS_TO_RIGHT),
+              Item(IDS_MOVE_TAB_TO_NEW_WINDOW)
+                  .command_id(IDC_MOVE_TAB_TO_NEW_WINDOW),
               Item().is_separator(),
           })
           .Build();