Only close a newly opened download tab if there's more than one tab.

BUG=45464,10764
TEST=See first bug for manual test.

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

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

6 files changed, 33 insertions, 10 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 3c9361b..c3a320a 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -2819,7 +2819,7 @@ int Browser::GetExtraRenderViewHeight() const {
   return window_->GetExtraRenderViewHeight();
 }
 
-void Browser::OnStartDownload(DownloadItem* download) {
+void Browser::OnStartDownload(DownloadItem* download, TabContents* tab) {
   if (!window())
     return;
 
@@ -2857,6 +2857,12 @@ void Browser::OnStartDownload(DownloadItem* download) {
     DownloadStartedAnimation::Show(current_tab);
   }
 #endif
+
+  // If the download occurs in a new tab, close it
+  if (tab->controller().IsInitialNavigation() &&
+      GetConstrainingContents(tab) == tab && tab_count() > 1) {
+    CloseContents(tab);
+  }
 }
 
 void Browser::ConfirmAddSearchProvider(const TemplateURL* template_url,
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 593fa8d..c8f746c 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -719,7 +719,7 @@ class Browser : public TabStripModelDelegate,
   virtual void SetFocusToLocationBar(bool select_all);
   virtual void RenderWidgetShowing();
   virtual int GetExtraRenderViewHeight() const;
-  virtual void OnStartDownload(DownloadItem* download);
+  virtual void OnStartDownload(DownloadItem* download, TabContents* tab);
   virtual void ConfirmAddSearchProvider(const TemplateURL* template_url,
                                         Profile* profile);
   virtual void ShowPageInfo(Profile* profile,
diff --git a/chrome/browser/download/download_uitest.cc b/chrome/browser/download/download_uitest.cc
index 77bbd29..cc4200b 100644
--- a/chrome/browser/download/download_uitest.cc
+++ b/chrome/browser/download/download_uitest.cc
@@ -492,6 +492,26 @@ TEST_F(DownloadTest, DISABLED_CloseNewTab3) {
   CheckDownload(file);
 }
 
+TEST_F(DownloadTest, DISABLED_DontCloseNewWindow) {
+  scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(browser.get());
+  int window_count = 0;
+  ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count));
+  ASSERT_EQ(1, window_count);
+  EXPECT_EQ(1, GetTabCount());
+
+  scoped_refptr<TabProxy> tab_proxy(GetActiveTab());
+  ASSERT_TRUE(tab_proxy.get());
+
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  ASSERT_TRUE(tab_proxy->NavigateToURLAsyncWithDisposition(
+      URLRequestMockHTTPJob::GetMockUrl(file), NEW_WINDOW));
+
+  ASSERT_TRUE(automation()->WaitForWindowCountToBecome(2));
+
+  CheckDownload(file);
+}
+
 // http://crbug.com/50060
 #if defined(OS_MACOSX)
 #define MAYBE_NewWindow DISABLED_NewWindow
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 18050bf..0111447 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -1144,12 +1144,8 @@ void TabContents::OnStartDownload(DownloadItem* download) {
   // Download in a constrained popup is shown in the tab that opened it.
   TabContents* tab_contents = delegate()->GetConstrainingContents(this);
 
-  if (tab_contents && tab_contents->delegate()) {
-    tab_contents->delegate()->OnStartDownload(download);
-    // If the download occurs in a new tab, close it
-    if (controller_.IsInitialNavigation() && (tab_contents == this))
-      delegate()->CloseContents(this);
-  }
+  if (tab_contents && tab_contents->delegate())
+    tab_contents->delegate()->OnStartDownload(download, this);
 }
 
 void TabContents::WillClose(ConstrainedWindow* window) {
diff --git a/chrome/browser/tab_contents/tab_contents_delegate.cc b/chrome/browser/tab_contents/tab_contents_delegate.cc
index 9cf9f88..eb3a81f 100644
--- a/chrome/browser/tab_contents/tab_contents_delegate.cc
+++ b/chrome/browser/tab_contents/tab_contents_delegate.cc
@@ -83,7 +83,8 @@ bool TabContentsDelegate::CanDownload(int request_id) {
   return true;
 }
 
-void TabContentsDelegate::OnStartDownload(DownloadItem* download) {
+void TabContentsDelegate::OnStartDownload(DownloadItem* download,
+                                          TabContents* tab) {
 }
 
 bool TabContentsDelegate::HandleContextMenu(const ContextMenuParams& params) {
diff --git a/chrome/browser/tab_contents/tab_contents_delegate.h b/chrome/browser/tab_contents/tab_contents_delegate.h
index ff1f49b..f3805a4 100644
--- a/chrome/browser/tab_contents/tab_contents_delegate.h
+++ b/chrome/browser/tab_contents/tab_contents_delegate.h
@@ -195,7 +195,7 @@ class TabContentsDelegate : public AutomationResourceRoutingDelegate {
 
   virtual bool CanDownload(int request_id);
 
-  virtual void OnStartDownload(DownloadItem* download);
+  virtual void OnStartDownload(DownloadItem* download, TabContents* tab);
 
   // Returns true if the context menu operation was handled by the delegate.
   virtual bool HandleContextMenu(const ContextMenuParams& params);