Issue 2509: Duplicate tab should not create a new window.

Review URL: http://codereview.chromium.org/3147

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@2474 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 34 insertions, 29 deletions

diff --git a/chrome/browser/browser_commands.cc b/chrome/browser/browser_commands.cc
index cf8630b..a3f6268 100644
--- a/chrome/browser/browser_commands.cc
+++ b/chrome/browser/browser_commands.cc
@@ -1029,36 +1029,43 @@ bool Browser::CanDuplicateContentsAt(int index) {
 
 void Browser::DuplicateContentsAt(int index) {
   TabContents* contents = GetTabContentsAt(index);
+  TabContents* new_contents = NULL;
   DCHECK(contents);
 
-  Browser* new_browser = NULL;
-
-  BrowserType::Type t;
-
-  // Is this an application?
-  if (type_ == BrowserType::TABBED_BROWSER)
-    t = BrowserType::TABBED_BROWSER;
-  else
-    t = BrowserType::APPLICATION;
-
-  new_browser = new Browser(gfx::Rect(), SW_SHOWNORMAL, profile(), t,
-                            app_name_);
-
-  // We need to show the browser now. Otherwise HWNDViewContainer assumes
-  // the tab contents is invisible and won't size it.
-  new_browser->Show();
+  if (type_ == BrowserType::TABBED_BROWSER) {
+    // If this is a tabbed browser, just create a duplicate tab inside the same
+    // window next to the tab being duplicated.
+    new_contents = contents->controller()->Clone(
+        GetTopLevelHWND())->active_contents();
+    // If you duplicate a tab that is not selected, we need to make sure to
+    // select the tab being duplicated so that DetermineInsertionIndex returns
+    // the right index (if tab 5 is selected and we right-click tab 1 we want
+    // the new tab to appear in index position 2, not 6).
+    if (tabstrip_model_.selected_index() != index)
+      tabstrip_model_.SelectTabContentsAt(index, true);
+    tabstrip_model_.AddTabContents(new_contents, index + 1,
+                                   PageTransition::LINK, true);
+  } else {
+    Browser* new_browser = new Browser(gfx::Rect(), SW_SHOWNORMAL, profile(),
+                                       BrowserType::APPLICATION, app_name_);
+
+    // We need to show the browser now. Otherwise HWNDViewContainer assumes
+    // the tab contents is invisible and won't size it.
+    new_browser->Show();
+
+    // The page transition below is only for the purpose of inserting the tab.
+    new_contents = new_browser->AddTabWithNavigationController(
+        contents->controller()->Clone(new_browser->GetTopLevelHWND()),
+        PageTransition::LINK);
+
+    new_browser->MoveToFront(true);
+  }
 
-  // The page transition below is only for the purpose of inserting the tab.
-  TabContents* new_contents = new_browser->AddTabWithNavigationController(
-      contents->controller()->Clone(new_browser->GetTopLevelHWND()),
-      PageTransition::LINK);
   if (profile_->HasSessionService()) {
     SessionService* session_service = profile_->GetSessionService();
     if (session_service)
       session_service->TabRestored(new_contents->controller());
   }
-
-  new_browser->MoveToFront(true);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/browser_uitest.cc b/chrome/browser/browser_uitest.cc
index 6fd93cf..842b7d8 100644
--- a/chrome/browser/browser_uitest.cc
+++ b/chrome/browser/browser_uitest.cc
@@ -251,13 +251,12 @@ TEST_F(BrowserTest, DuplicateTab) {
   // Duplicate the tab.
   ASSERT_TRUE(browser_proxy->ApplyAccelerator(IDC_DUPLICATE));
 
-  // The duplicated tab ends up in a new window.
+  // The duplicated tab should not end up in a new window.
   int window_count;
   ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
-  ASSERT_TRUE(window_count > initial_window_count);
+  ASSERT_TRUE(window_count == initial_window_count);
 
-  browser_proxy.reset(automation()->GetBrowserWindow(1));
-  tab_proxy.reset(browser_proxy->GetTab(0));
+  tab_proxy.reset(browser_proxy->GetTab(1));
 
   ASSERT_TRUE(tab_proxy->WaitForTabToBeRestored(kWaitForActionMsec));
 
diff --git a/chrome/test/automation/automation_proxy_uitest.cc b/chrome/test/automation/automation_proxy_uitest.cc
index a6685f9..6fea02f 100644
--- a/chrome/test/automation/automation_proxy_uitest.cc
+++ b/chrome/test/automation/automation_proxy_uitest.cc
@@ -779,14 +779,13 @@ TEST_F(AutomationProxyTest, AutocompleteGetSetText) {
   EXPECT_EQ(text_to_set, actual_text);
 }
 
-TEST_F(AutomationProxyTest, AutocompleteParallelProxy)
-{
+TEST_F(AutomationProxyTest, AutocompleteParallelProxy) {
   scoped_ptr<BrowserProxy> browser1(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(browser1.get());
   scoped_ptr<AutocompleteEditProxy> edit1(
       automation()->GetAutocompleteEditForBrowser(browser1.get()));
   ASSERT_TRUE(edit1.get());
-  EXPECT_TRUE(browser1->ApplyAccelerator(IDC_DUPLICATE));
+  EXPECT_TRUE(browser1->ApplyAccelerator(IDC_NEWWINDOW));
   scoped_ptr<BrowserProxy> browser2(automation()->GetBrowserWindow(1));
   ASSERT_TRUE(browser2.get());
   scoped_ptr<AutocompleteEditProxy> edit2(