Converted download UI tests to Browser tests.

Converted NavigateWithURLAsync/WaitForDownloadShelfVisible calls to NavigateToURLWithDisposition.  This waits until the navigation is done before returning, avoiding the need for the 'sleep and wait' loop.

BUG=none
TEST=Run browser_tests --gtest_filter=DownloadTest.*

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

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

5 files changed, 803 insertions, 49 deletions

diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
index 6f0d998..d337233 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -7,19 +7,25 @@
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
 #include "base/test/test_file_util.h"
-#include "chrome/browser/ui/browser.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_window.h"
 #include "chrome/browser/download/download_item.h"
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/download/download_prefs.h"
+#include "chrome/browser/download/download_shelf.h"
 #include "chrome/browser/net/url_request_mock_http_job.h"
+#include "chrome/browser/net/url_request_slow_download_job.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/page_transition_types.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
+#include "net/base/net_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -213,54 +219,142 @@ class DownloadsObserver : public DownloadManager::Observer,
 };
 
 class DownloadTest : public InProcessBrowserTest {
- protected:
-  void SetUpInProcessBrowserTestFixture() {
-    ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir_));
+ public:
+  enum SelectExpectation {
+    EXPECT_NO_SELECT_DIALOG = -1,
+    EXPECT_NOTHING,
+    EXPECT_SELECT_DIALOG
+  };
+
+  // Returning false indicates a failure of the setup, and should be asserted
+  // in the caller.
+  virtual bool InitialSetup(bool prompt_for_download) {
+    bool have_test_dir = PathService::Get(chrome::DIR_TEST_DATA, &test_dir_);
+    EXPECT_TRUE(have_test_dir);
+    if (!have_test_dir)
+      return false;
+
+    // Sanity check default values for window / tab count and shelf visibility.
+    int window_count = BrowserList::size();
+    EXPECT_EQ(1, window_count);
+    EXPECT_EQ(1, browser()->tab_count());
+    bool is_shelf_visible = browser()->window()->IsDownloadShelfVisible();
+    EXPECT_FALSE(is_shelf_visible);
+
+    // Set up the temporary download folder.
+    bool created_downloads_dir = CreateAndSetDownloadsDirectory(browser());
+    EXPECT_TRUE(created_downloads_dir);
+    if (!created_downloads_dir)
+      return false;
+    browser()->profile()->GetPrefs()->SetBoolean(prefs::kPromptForDownload,
+                                                 prompt_for_download);
+
+    return true;
   }
 
+ protected:
+
   // Must be called after browser creation.  Creates a temporary
   // directory for downloads that is auto-deleted on destruction.
-  bool CreateAndSetDownloadsDirectory() {
-    if (downloads_directory_.CreateUniqueTempDir()) {
-      browser()->profile()->GetPrefs()->SetFilePath(
-          prefs::kDownloadDefaultDirectory,
-          downloads_directory_.path());
-      return true;
-    }
-    return false;
+  // Returning false indicates a failure of the function, and should be asserted
+  // in the caller.
+  bool CreateAndSetDownloadsDirectory(Browser* browser) {
+    if (!browser)
+      return false;
+
+    if (!downloads_directory_.CreateUniqueTempDir())
+      return false;
+
+    browser->profile()->GetPrefs()->SetFilePath(
+        prefs::kDownloadDefaultDirectory,
+        downloads_directory_.path());
+
+    return true;
   }
 
-  // May only be called inside of an individual test; browser() is NULL
-  // outside of that context.
-  FilePath GetDownloadDirectory() {
+  FilePath GetDownloadDirectory(Browser* browser) {
     DownloadManager* download_mananger =
-        browser()->profile()->GetDownloadManager();
+        browser->profile()->GetDownloadManager();
     return download_mananger->download_prefs()->download_path();
   }
 
-  DownloadsObserver* CreateWaiter(int num_downloads) {
+  DownloadsObserver* CreateWaiter(Browser* browser, int num_downloads) {
     DownloadManager* download_manager =
-        browser()->profile()->GetDownloadManager();
+        browser->profile()->GetDownloadManager();
     return new DownloadsObserver(
         download_manager, num_downloads,
         DownloadsObserver::FILE_RENAME,  // Really done
-        true);                          // Bail on select file
+        true);                           // Bail on select file
+  }
+
+  // Download |url|, then wait for the download to finish.
+  // |disposition| indicates where the navigation occurs (current tab, new
+  // foreground tab, etc).
+  // |expectation| indicates whether or not a Select File dialog should be
+  // open when the download is finished, or if we don't care.
+  // If the dialog appears, the test completes.  The only effect |expectation|
+  // has is whether or not the test succeeds.
+  // |browser_test_flags| indicate what to wait for, and is an OR of 0 or more
+  // values in the ui_test_utils::BrowserTestWaitFlags enum.
+  void DownloadAndWaitWithDisposition(Browser* browser,
+                                      const GURL& url,
+                                      WindowOpenDisposition disposition,
+                                      SelectExpectation expectation,
+                                      int browser_test_flags) {
+    // Setup notification, navigate, and block.
+    scoped_ptr<DownloadsObserver> observer(CreateWaiter(browser, 1));
+    // This call will block until the condition specified by
+    // |browser_test_flags|, but will not wait for the download to finish.
+    ui_test_utils::NavigateToURLWithDisposition(browser,
+                                                url,
+                                                disposition,
+                                                browser_test_flags);
+    // Waits for the download to complete.
+    observer->WaitForFinished();
+
+    // If specified, check the state of the select file dialog.
+    if (expectation != EXPECT_NOTHING) {
+      EXPECT_EQ(expectation == EXPECT_SELECT_DIALOG,
+                observer->select_file_dialog_seen());
+    }
+  }
+
+  // Download a file in the current tab, then wait for the download to finish.
+  void DownloadAndWait(Browser* browser,
+                       const GURL& url,
+                       SelectExpectation expectation) {
+    DownloadAndWaitWithDisposition(
+        browser,
+        url,
+        CURRENT_TAB,
+        expectation,
+        ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
   }
 
   // Should only be called when the download is known to have finished
   // (in error or not).
-  void CheckDownload(const FilePath& downloaded_filename,
+  // Returning false indicates a failure of the function, and should be asserted
+  // in the caller.
+  bool CheckDownload(Browser* browser,
+                     const FilePath& downloaded_filename,
                      const FilePath& origin_filename) {
     // Find the path to which the data will be downloaded.
     FilePath downloaded_file =
-        GetDownloadDirectory().Append(downloaded_filename);
+        GetDownloadDirectory(browser).Append(downloaded_filename);
 
     // Find the origin path (from which the data comes).
     FilePath origin_file(test_dir_.Append(origin_filename));
-    ASSERT_TRUE(file_util::PathExists(origin_file));
+    bool origin_file_exists = file_util::PathExists(origin_file);
+    EXPECT_TRUE(origin_file_exists);
+    if (!origin_file_exists)
+      return false;
 
     // Confirm the downloaded data file exists.
-    ASSERT_TRUE(file_util::PathExists(downloaded_file));
+    bool downloaded_file_exists = file_util::PathExists(downloaded_file);
+    EXPECT_TRUE(downloaded_file_exists);
+    if (!downloaded_file_exists)
+      return false;
+
     int64 origin_file_size = 0;
     int64 downloaded_file_size = 0;
     EXPECT_TRUE(file_util::GetFileSize(origin_file, &origin_file_size));
@@ -275,7 +369,68 @@ class DownloadTest : public InProcessBrowserTest {
 #endif
 
     // Delete the downloaded copy of the file.
-    EXPECT_TRUE(file_util::DieFileDie(downloaded_file, false));
+    bool downloaded_file_deleted =
+        file_util::DieFileDie(downloaded_file, false);
+    EXPECT_TRUE(downloaded_file_deleted);
+    return downloaded_file_deleted;
+  }
+
+  // TODO(ahendrickson) -- |expected_title_in_progress| and
+  // |expected_title_in_finished| need to be checked.
+  bool RunSizeTest(Browser* browser,
+                   const GURL& url,
+                   const std::wstring& expected_title_in_progress,
+                   const std::wstring& expected_title_finished) {
+    if (!InitialSetup(false))
+      return false;
+
+    // Download a partial web page in a background tab and wait.
+    // The mock system will not complete until it gets a special URL.
+    scoped_ptr<DownloadsObserver> observer(CreateWaiter(browser, 1));
+    ui_test_utils::NavigateToURL(browser, url);
+
+    // TODO(ahendrickson): check download status text before downloading.
+    // Need to:
+    //  - Add a member function to the |DownloadShelf| interface class, that
+    //    indicates how many members it has.
+    //  - Add a member function to |DownloadShelf| to get the status text
+    //    of a given member (for example, via |DownloadItemView|'s
+    //    GetAccessibleName() member function), by index.
+    //  - Iterate over browser->window()->GetDownloadShelf()'s members
+    //    to see if any match the status text we want.  Start with the last one.
+
+    // Complete sending the request.  We do this by loading a second URL in a
+    // separate tab.
+    GURL finish_url(URLRequestSlowDownloadJob::kFinishDownloadUrl);
+    ui_test_utils::NavigateToURLWithDisposition(
+        browser,
+        finish_url,
+        NEW_FOREGROUND_TAB,
+        ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+    observer->WaitForFinished();
+
+    EXPECT_EQ(2, browser->tab_count());
+
+    // TODO(ahendrickson): check download status text after downloading.
+
+    // Make sure the download shelf is showing.
+    EXPECT_TRUE(browser->window()->IsDownloadShelfVisible());
+
+    FilePath filename;
+    net::FileURLToFilePath(url, &filename);
+    filename = filename.BaseName();
+    FilePath download_path = downloads_directory_.path().Append(filename);
+
+    bool downloaded_path_exists = file_util::PathExists(download_path);
+    EXPECT_TRUE(downloaded_path_exists);
+    if (!downloaded_path_exists)
+      return false;
+
+    // Delete the file we just downloaded.
+    EXPECT_TRUE(file_util::DieFileDie(download_path, true));
+    EXPECT_FALSE(file_util::PathExists(download_path));
+
+    return true;
   }
 
  private:
@@ -286,51 +441,506 @@ class DownloadTest : public InProcessBrowserTest {
   ScopedTempDir downloads_directory_;
 };
 
+// NOTES:
+//
+// Files for these tests are found in DIR_TEST_DATA (currently
+// "chrome\test\data\", see chrome_paths.cc).
+// Mock responses have extension .mock-http-headers appended to the file name.
+
+// Download a file due to the associated MIME type.
+//
 // Test is believed good (non-flaky) in itself, but it
 // sometimes trips over underlying flakiness in the downloads
 // subsystem in in http://crbug.com/63237.  Until that bug is
 // fixed, this test should be considered flaky.  It's entered as
 // DISABLED since if 63237 does cause a failure, it'll be a timeout.
 IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_DownloadMimeType) {
+  ASSERT_TRUE(InitialSetup(false));
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
-  ASSERT_TRUE(CreateAndSetDownloadsDirectory());
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
 
-  EXPECT_EQ(1, browser()->tab_count());
-
-  // Setup notification, navigate, and block.
-  scoped_ptr<DownloadsObserver> observer(CreateWaiter(1));
-  ui_test_utils::NavigateToURL(
-      browser(), URLRequestMockHTTPJob::GetMockUrl(file));
-  observer->WaitForFinished();
+  // Download the file and wait.  We do not expect the Select File dialog.
+  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
 
-  // Download should be finished; check state.
-  EXPECT_FALSE(observer->select_file_dialog_seen());
+  // Check state.
   EXPECT_EQ(1, browser()->tab_count());
-  CheckDownload(file, file);
+  CheckDownload(browser(), file, file);
   EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
 }
 
+// Put up a Select File dialog when the file is downloaded, due to its MIME
+// type.
+//
 // This test runs correctly, but leaves behind turds in the test user's
 // download directory because of http://crbug.com/62099.  No big loss; it
 // was primarily confirming DownloadsObserver wait on select file dialog
 // functionality anyway.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
 IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_DownloadMimeTypeSelect) {
+  ASSERT_TRUE(InitialSetup(true));
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+
+  // Download the file and wait.  We expect the Select File dialog to appear
+  // due to the MIME type.
+  DownloadAndWait(browser(), url, EXPECT_SELECT_DIALOG);
+
+  // Check state.
+  EXPECT_EQ(1, browser()->tab_count());
+  // Since we exited while the Select File dialog was visible, there should not
+  // be anything in the download shelf and so it should not be visible.
+  EXPECT_FALSE(browser()->window()->IsDownloadShelfVisible());
+}
+
+// Access a file with a viewable mime-type, verify that a download
+// did not initiate.
+IN_PROC_BROWSER_TEST_F(DownloadTest, NoDownload) {
+  ASSERT_TRUE(InitialSetup(false));
+  FilePath file(FILE_PATH_LITERAL("download-test2.html"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+  FilePath file_path = GetDownloadDirectory(browser()).Append(file);
+
+  // Open a web page and wait.
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // Check that we did not download the web page.
+  EXPECT_FALSE(file_util::PathExists(file_path));
+
+  // Check state.
+  EXPECT_EQ(1, browser()->tab_count());
+  EXPECT_FALSE(browser()->window()->IsDownloadShelfVisible());
+}
+
+// Download a 0-size file with a content-disposition header, verify that the
+// download tab opened and the file exists as the filename specified in the
+// header.  This also ensures we properly handle empty file downloads.
+// The download shelf should be visible in the current tab.
+//
+// Test is believed mostly good (non-flaky) in itself, but it
+// sometimes trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  Until that bug is
+// fixed, this test should be considered flaky.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_ContentDisposition) {
+  ASSERT_TRUE(InitialSetup(false));
+  FilePath file(FILE_PATH_LITERAL("download-test3.gif"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+  FilePath download_file(FILE_PATH_LITERAL("download-test3-attachment.gif"));
+
+  // Download a file and wait.
+  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
+
+  CheckDownload(browser(), download_file, file);
+
+  // Check state.
+  EXPECT_EQ(1, browser()->tab_count());
+  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+}
+
+// Test that the download shelf is per-window by starting a download in one
+// tab, opening a second tab, closing the shelf, going back to the first tab,
+// and checking that the shelf is closed.
+//
+// The test sometimes trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_PerWindowShelf) {
+  ASSERT_TRUE(InitialSetup(false));
+  FilePath file(FILE_PATH_LITERAL("download-test3.gif"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+  FilePath download_file(FILE_PATH_LITERAL("download-test3-attachment.gif"));
+
+  // Download a file and wait.
+  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
+
+  CheckDownload(browser(), download_file, file);
+
+  // Check state.
+  EXPECT_EQ(1, browser()->tab_count());
+  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
+  // Open a second tab and wait.
+  EXPECT_NE(static_cast<TabContentsWrapper*>(NULL),
+            browser()->AddSelectedTabWithURL(GURL(), PageTransition::TYPED));
+  EXPECT_EQ(2, browser()->tab_count());
+  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
+  // Hide the download shelf.
+  browser()->window()->GetDownloadShelf()->Close();
+  EXPECT_FALSE(browser()->window()->IsDownloadShelfVisible());
+
+  // Go to the first tab.
+  browser()->SelectTabContentsAt(0, true);
+  EXPECT_EQ(2, browser()->tab_count());
+
+  // The download shelf should not be visible.
+  EXPECT_FALSE(browser()->window()->IsDownloadShelfVisible());
+}
+
+// UnknownSize and KnownSize are tests which depend on
+// URLRequestSlowDownloadJob to serve content in a certain way. Data will be
+// sent in two chunks where the first chunk is 35K and the second chunk is 10K.
+// The test will first attempt to download a file; but the server will "pause"
+// in the middle until the server receives a second request for
+// "download-finish".  At that time, the download will finish.
+// These tests don't currently test much due to holes in |RunSizeTest()|.  See
+// comments in that routine for details.
+
+// Test is believed mostly good (non-flaky) in itself, but it
+// very occasionally trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  Until that bug is
+// fixed, this test should be considered flaky.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_UnknownSize) {
+  GURL url(URLRequestSlowDownloadJob::kUnknownSizeUrl);
+  FilePath filename;
+  net::FileURLToFilePath(url, &filename);
+  filename = filename.BaseName();
+  ASSERT_TRUE(RunSizeTest(browser(),
+                          url,
+                          L"32.0 KB - " + filename.ToWStringHack(),
+                          L"100% - " + filename.ToWStringHack()));
+}
+
+// Test is believed mostly good (non-flaky) in itself, but it
+// very occasionally trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  Until that bug is
+// fixed, this test should be considered flaky.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_KnownSize) {
+  GURL url(URLRequestSlowDownloadJob::kKnownSizeUrl);
+  FilePath filename;
+  net::FileURLToFilePath(url, &filename);
+  filename = filename.BaseName();
+  ASSERT_TRUE(RunSizeTest(browser(),
+                          url,
+                          L"71% - " + filename.ToWStringHack(),
+                          L"100% - " + filename.ToWStringHack()));
+}
+
+// Test that when downloading an item in Incognito mode, we don't crash when
+// closing the last Incognito window (http://crbug.com/13983).
+// Also check that the download shelf is not visible after closing the
+// Incognito window.
+//
+// Test is believed mostly good (non-flaky) in itself, but it
+// sometimes trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  Until that bug is
+// fixed, this test should be considered flaky.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_IncognitoDownload) {
+  ASSERT_TRUE(InitialSetup(false));
+
+  // Open an Incognito window.
+  Browser* incognito = CreateIncognitoBrowser();  // Waits.
+  ASSERT_TRUE(incognito);
+  int window_count = BrowserList::size();
+  EXPECT_EQ(2, window_count);
+
+  // Download a file in the Incognito window and wait.
+  CreateAndSetDownloadsDirectory(incognito);
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+  // Since |incognito| is a separate browser, we have to set it up explicitly.
+  incognito->profile()->GetPrefs()->SetBoolean(prefs::kPromptForDownload,
+                                               false);
+  DownloadAndWait(incognito, url, EXPECT_NO_SELECT_DIALOG);
+
+  // We should still have 2 windows.
+  window_count = BrowserList::size();
+  EXPECT_EQ(2, window_count);
+
+  // Verify that the download shelf is showing for the Incognito window.
+  bool is_shelf_visible = incognito->window()->IsDownloadShelfVisible();
+  EXPECT_TRUE(is_shelf_visible);
+
+  // Close the Incognito window and don't crash.
+  incognito->CloseWindow();
+  ui_test_utils::WaitForNotificationFrom(NotificationType::BROWSER_CLOSED,
+                                         Source<Browser>(incognito));
+
+  window_count = BrowserList::size();
+  EXPECT_EQ(1, window_count);
+
+  // Verify that the regular window does not have a download shelf.
+  is_shelf_visible = browser()->window()->IsDownloadShelfVisible();
+  EXPECT_FALSE(is_shelf_visible);
+
+  CheckDownload(browser(), file, file);
+}
+
+// Navigate to a new background page, but don't download.  Confirm that the
+// download shelf is not visible and that we have two tabs.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DontCloseNewTab1) {
+  ASSERT_TRUE(InitialSetup(false));
+  // Because it's an HTML link, it should open a web page rather than
+  // downloading.
+  FilePath file1(FILE_PATH_LITERAL("download-test2.html"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file1));
+
+  // Open a web page and wait.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      url,
+      NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // We should have two tabs now.
+  EXPECT_EQ(2, browser()->tab_count());
+  EXPECT_FALSE(browser()->window()->IsDownloadShelfVisible());
+}
+
+// Download a file in a background tab. Verify that the tab is closed
+// automatically, and that the download shelf is visible in the current tab.
+//
+// The test sometimes trips over underlying flakiness in the downloads
+// subsystem in http://crbug.com/63237.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_CloseNewTab1) {
+  ASSERT_TRUE(InitialSetup(false));
+
+  // Download a file in a new background tab and wait.  The tab is automatically
+  // closed when the download begins.
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+  DownloadAndWaitWithDisposition(
+      browser(),
+      url,
+      NEW_BACKGROUND_TAB,
+      EXPECT_NO_SELECT_DIALOG,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // When the download finishes, we should still have one tab.
+  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+  EXPECT_EQ(1, browser()->tab_count());
+
+  CheckDownload(browser(), file, file);
+}
+
+// Open a web page in the current tab, then download a file in another tab via
+// a Javascript call.
+// Verify that we have 2 tabs, and the download shelf is visible in the current
+// tab.
+//
+// The download_page1.html page contains an openNew() function that opens a
+// tab and then downloads download-test1.lib.
+//
+// The test sometimes trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_DontCloseNewTab2) {
+  ASSERT_TRUE(InitialSetup(false));
+  // Because it's an HTML link, it should open a web page rather than
+  // downloading.
+  FilePath file1(FILE_PATH_LITERAL("download_page1.html"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file1));
+
+  // Open a web page and wait.
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // Download a file in a new tab and wait (via Javascript).
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  DownloadAndWaitWithDisposition(browser(),
+                                 GURL("javascript:openNew()"),
+                                 CURRENT_TAB,
+                                 EXPECT_NO_SELECT_DIALOG,
+                                 ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
+
+  // When the download finishes, we should have two tabs.
+  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+  EXPECT_EQ(2, browser()->tab_count());
+
+  CheckDownload(browser(), file, file);
+}
+
+// Open a web page in the current tab, open another tab via a Javascript call,
+// then download a file in the new tab.
+// Verify that we have 2 tabs, and the download shelf is visible in the current
+// tab.
+//
+// The download_page2.html page contains an openNew() function that opens a
+// tab.
+//
+// The test sometimes trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_DontCloseNewTab3) {
+  ASSERT_TRUE(InitialSetup(false));
+  // Because it's an HTML link, it should open a web page rather than
+  // downloading.
+  FilePath file1(FILE_PATH_LITERAL("download_page2.html"));
+  GURL url1(URLRequestMockHTTPJob::GetMockUrl(file1));
+
+  // Open a web page and wait.
+  ui_test_utils::NavigateToURL(browser(), url1);
+
+  // Open a new tab and wait.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      GURL("javascript:openNew()"),
+      CURRENT_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
+
+  EXPECT_EQ(2, browser()->tab_count());
+
+  // Download a file and wait.
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
-  ASSERT_TRUE(CreateAndSetDownloadsDirectory());
-  browser()->profile()->GetPrefs()->SetBoolean(prefs::kPromptForDownload, true);
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+  DownloadAndWaitWithDisposition(browser(),
+                                 url,
+                                 CURRENT_TAB,
+                                 EXPECT_NO_SELECT_DIALOG,
+                                 ui_test_utils::BROWSER_TEST_NONE);
+
+  // When the download finishes, we should have two tabs.
+  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+  EXPECT_EQ(2, browser()->tab_count());
+
+  CheckDownload(browser(), file, file);
+}
+
+// Open a web page in the current tab, then download a file via Javascript,
+// which will do so in a temporary tab.
+// Verify that we have 1 tab, and the download shelf is visible.
+//
+// The download_page3.html page contains an openNew() function that opens a
+// tab with download-test1.lib in the URL.  When the URL is determined to be
+// a download, the tab is closed automatically.
+//
+// The test sometimes trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_CloseNewTab2) {
+  ASSERT_TRUE(InitialSetup(false));
+  // Because it's an HTML link, it should open a web page rather than
+  // downloading.
+  FilePath file1(FILE_PATH_LITERAL("download_page3.html"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file1));
+
+  // Open a web page and wait.
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // Download a file and wait.
+  // The file to download is "download-test1.lib".
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  DownloadAndWaitWithDisposition(browser(),
+                                 GURL("javascript:openNew()"),
+                                 CURRENT_TAB,
+                                 EXPECT_NO_SELECT_DIALOG,
+                                 ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
+
+  // When the download finishes, we should still have one tab.
+  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+  EXPECT_EQ(1, browser()->tab_count());
+
+  CheckDownload(browser(), file, file);
+}
 
+// Open a web page in the current tab, then call Javascript via a button to
+// download a file in a new tab, which is closed automatically when the
+// download begins.
+// Verify that we have 1 tab, and the download shelf is visible.
+//
+// The download_page4.html page contains a form with download-test1.lib as the
+// action.
+//
+// The test sometimes trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_CloseNewTab3) {
+  ASSERT_TRUE(InitialSetup(false));
+  // Because it's an HTML link, it should open a web page rather than
+  // downloading.
+  FilePath file1(FILE_PATH_LITERAL("download_page4.html"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file1));
+
+  // Open a web page and wait.
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // Download a file in a new tab and wait.  The tab will automatically close
+  // when the download begins.
+  // The file to download is "download-test1.lib".
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  DownloadAndWaitWithDisposition(
+      browser(),
+      GURL("javascript:document.getElementById('form').submit()"),
+      CURRENT_TAB,
+      EXPECT_NO_SELECT_DIALOG,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
+
+  // When the download finishes, we should still have one tab.
+  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
   EXPECT_EQ(1, browser()->tab_count());
 
-  // Setup notification, navigate, and block.
-  scoped_ptr<DownloadsObserver> observer(CreateWaiter(1));
-  ui_test_utils::NavigateToURL(
-      browser(), URLRequestMockHTTPJob::GetMockUrl(file));
-  observer->WaitForFinished();
+  CheckDownload(browser(), file, file);
+}
+
+// Download a file in a new window.
+// Verify that we have 2 windows, and the download shelf is not visible in the
+// first window, but is visible in the second window.
+// Close the new window.
+// Verify that we have 1 window, and the download shelf is not visible.
+//
+// Regression test for http://crbug.com/44454
+//
+// Test is believed mostly good (non-flaky) in itself, but it
+// sometimes trips over underlying flakiness in the downloads
+// subsystem in in http://crbug.com/63237.  Until that bug is
+// fixed, this test should be considered flaky.  It's entered as
+// DISABLED since if 63237 does cause a failure, it'll be a timeout.
+// Additionally, there is Windows-specific flake, http://crbug.com/20809.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_NewWindow) {
+  ASSERT_TRUE(InitialSetup(false));
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+  Browser* first_browser = browser();
+
+  // Download a file in a new window and wait.
+  DownloadAndWaitWithDisposition(browser(),
+                                 url,
+                                 NEW_WINDOW,
+                                 EXPECT_NO_SELECT_DIALOG,
+                                 ui_test_utils::BROWSER_TEST_NONE);
+
+  // When the download finishes, the download shelf SHOULD NOT be visible in
+  // the first window.
+  int window_count = BrowserList::size();
+  EXPECT_EQ(2, window_count);
+  EXPECT_EQ(1, browser()->tab_count());
+  EXPECT_FALSE(browser()->window()->IsDownloadShelfVisible());
 
-  // Download should not be finished; check state.
-  EXPECT_TRUE(observer->select_file_dialog_seen());
+  // The download shelf SHOULD be visible in the second window.
+  std::set<Browser*> original_browsers;
+  original_browsers.insert(browser());
+  Browser* download_browser =
+      ui_test_utils::GetBrowserNotInSet(original_browsers);
+  ASSERT_TRUE(download_browser != NULL);
+  EXPECT_NE(download_browser, browser());
+  EXPECT_EQ(1, download_browser->tab_count());
+  EXPECT_TRUE(download_browser->window()->IsDownloadShelfVisible());
+
+  // Close the new window.
+  download_browser->CloseWindow();
+  ui_test_utils::WaitForNotificationFrom(NotificationType::BROWSER_CLOSED,
+                                         Source<Browser>(download_browser));
+  EXPECT_EQ(first_browser, browser());
+  window_count = BrowserList::size();
+  EXPECT_EQ(1, window_count);
   EXPECT_EQ(1, browser()->tab_count());
+  // The download shelf should not be visible in the remaining window.
   EXPECT_FALSE(browser()->window()->IsDownloadShelfVisible());
+
+  CheckDownload(browser(), file, file);
 }
 
 }  // namespace
diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc
index 5815fad..ff428c9 100644
--- a/chrome/test/in_process_browser_test.cc
+++ b/chrome/test/in_process_browser_test.cc
@@ -274,6 +274,14 @@ Browser* InProcessBrowserTest::CreateBrowser(Profile* profile) {
   return browser;
 }
 
+Browser* InProcessBrowserTest::CreateIncognitoBrowser() {
+  // Create a new browser with using the incognito profile.
+  Browser* incognito =
+      Browser::Create(browser()->profile()->GetOffTheRecordProfile());
+  InitializeBrowser(incognito);
+  return incognito;
+}
+
 Browser* InProcessBrowserTest::CreateBrowserForPopup(Profile* profile) {
   Browser* browser = Browser::CreateForType(Browser::TYPE_POPUP, profile);
   InitializeBrowser(browser);
diff --git a/chrome/test/in_process_browser_test.h b/chrome/test/in_process_browser_test.h
index 42cadbb..169ad47 100644
--- a/chrome/test/in_process_browser_test.h
+++ b/chrome/test/in_process_browser_test.h
@@ -145,6 +145,9 @@ class InProcessBrowserTest : public testing::Test {
   // This is invoked from Setup.
   virtual Browser* CreateBrowser(Profile* profile);
 
+  // Similar to |CreateBrowser|, but creates an incognito browser.
+  virtual Browser* CreateIncognitoBrowser();
+
   // Creates a browser for a popup window with a single tab (about:blank), waits
   // for the tab to finish loading, and shows the browser.
   Browser* CreateBrowserForPopup(Profile* profile);
diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
index 0be4bd8..7eba05c 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -432,6 +432,17 @@ Browser* WaitForNewBrowser() {
   return Source<Browser>(observer.source()).ptr();
 }
 
+Browser* WaitForBrowserNotInSet(std::set<Browser*> excluded_browsers) {
+  TestNotificationObserver observer;
+  Browser* new_browser = GetBrowserNotInSet(excluded_browsers);
+  if (new_browser == NULL) {
+    new_browser = WaitForNewBrowser();
+    // The new browser should never be in |excluded_browsers|.
+    DCHECK(!ContainsKey(excluded_browsers, new_browser));
+  }
+  return new_browser;
+}
+
 void OpenURLOffTheRecord(Profile* profile, const GURL& url) {
   Browser::OpenURLOffTheRecord(profile, url);
   Browser* browser = BrowserList::FindBrowserWithType(
@@ -440,16 +451,80 @@ void OpenURLOffTheRecord(Profile* profile, const GURL& url) {
 }
 
 void NavigateToURL(Browser* browser, const GURL& url) {
-  NavigateToURLBlockUntilNavigationsComplete(browser, url, 1);
+  NavigateToURLWithDisposition(browser, url, CURRENT_TAB,
+                               BROWSER_TEST_WAIT_FOR_NAVIGATION);
+}
+
+// Navigates the specified tab (via |disposition|) of |browser| to |url|,
+// blocking until the |number_of_navigations| specified complete.
+// |disposition| indicates what tab the download occurs in, and
+// |browser_test_flags| controls what to wait for before continuing.
+static void NavigateToURLWithDispositionBlockUntilNavigationsComplete(
+    Browser* browser,
+    const GURL& url,
+    int number_of_navigations,
+    WindowOpenDisposition disposition,
+    int browser_test_flags) {
+  std::set<Browser*> initial_browsers;
+  for (std::vector<Browser*>::const_iterator iter = BrowserList::begin();
+       iter != BrowserList::end();
+       ++iter) {
+    initial_browsers.insert(*iter);
+  }
+  browser->OpenURL(url, GURL(), disposition, PageTransition::TYPED);
+  if (browser_test_flags & BROWSER_TEST_WAIT_FOR_BROWSER)
+    browser = WaitForBrowserNotInSet(initial_browsers);
+  if (browser_test_flags & BROWSER_TEST_WAIT_FOR_TAB)
+    WaitForNotification(NotificationType::TAB_ADDED);
+  if (!(browser_test_flags & BROWSER_TEST_WAIT_FOR_NAVIGATION)) {
+    // Some other flag caused the wait prior to this.
+    return;
+  }
+  TabContents* tab_contents = NULL;
+  if (disposition == NEW_BACKGROUND_TAB) {
+    // We've opened up a new tab, but not selected it.
+    tab_contents = browser->GetTabContentsAt(browser->selected_index() + 1);
+    EXPECT_TRUE(tab_contents != NULL)
+        << " Unable to wait for navigation to \"" << url.spec()
+        << "\" because the new tab is not available yet";
+    return;
+  } else if ((disposition == CURRENT_TAB) ||
+      (disposition == NEW_FOREGROUND_TAB) ||
+      (disposition == SINGLETON_TAB)) {
+    // The currently selected tab is the right one.
+    tab_contents = browser->GetSelectedTabContents();
+  }
+  if (tab_contents) {
+    NavigationController* controller = &tab_contents->controller();
+    WaitForNavigations(controller, number_of_navigations);
+    return;
+  }
+  EXPECT_TRUE(NULL != tab_contents) << " Unable to wait for navigation to \""
+                                    << url.spec() << "\""
+                                    << " because we can't get the tab contents";
+}
+
+void NavigateToURLWithDisposition(Browser* browser,
+                                  const GURL& url,
+                                  WindowOpenDisposition disposition,
+                                  int browser_test_flags) {
+  NavigateToURLWithDispositionBlockUntilNavigationsComplete(
+      browser,
+      url,
+      1,
+      disposition,
+      browser_test_flags);
 }
 
 void NavigateToURLBlockUntilNavigationsComplete(Browser* browser,
                                                 const GURL& url,
                                                 int number_of_navigations) {
-  NavigationController* controller =
-      &browser->GetSelectedTabContents()->controller();
-  browser->OpenURL(url, GURL(), CURRENT_TAB, PageTransition::TYPED);
-  WaitForNavigations(controller, number_of_navigations);
+  NavigateToURLWithDispositionBlockUntilNavigationsComplete(
+      browser,
+      url,
+      number_of_navigations,
+      CURRENT_TAB,
+      BROWSER_TEST_WAIT_FOR_NAVIGATION);
 }
 
 DOMElementProxyRef GetActiveDOMDocument(Browser* browser) {
@@ -563,6 +638,12 @@ void WaitForNotification(NotificationType type) {
   RegisterAndWait(&observer, type, NotificationService::AllSources());
 }
 
+void WaitForNotificationFrom(NotificationType type,
+                             const NotificationSource& source) {
+  TestNotificationObserver observer;
+  RegisterAndWait(&observer, type, source);
+}
+
 void RegisterAndWait(NotificationObserver* observer,
                      NotificationType type,
                      const NotificationSource& source) {
@@ -606,6 +687,17 @@ bool BringBrowserWindowToFront(const Browser* browser) {
   return true;
 }
 
+Browser* GetBrowserNotInSet(std::set<Browser*> excluded_browsers) {
+  for (BrowserList::const_iterator iter = BrowserList::begin();
+       iter != BrowserList::end();
+       ++iter) {
+    if (excluded_browsers.find(*iter) == excluded_browsers.end())
+      return *iter;
+  }
+
+  return NULL;
+}
+
 bool SendKeyPressSync(const Browser* browser,
                       app::KeyboardCode key,
                       bool control,
diff --git a/chrome/test/ui_test_utils.h b/chrome/test/ui_test_utils.h
index 5299451..4902396 100644
--- a/chrome/test/ui_test_utils.h
+++ b/chrome/test/ui_test_utils.h
@@ -24,6 +24,7 @@
 #include "chrome/test/automation/dom_element_proxy.h"
 #include "gfx/native_widget_types.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/glue/window_open_disposition.h"
 
 class AppModalDialog;
 class BookmarkModel;
@@ -52,6 +53,23 @@ class Size;
 // A collections of functions designed for use with InProcessBrowserTest.
 namespace ui_test_utils {
 
+// Flags to indicate what to wait for in a navigation test.
+// They can be ORed together.
+// The order in which the waits happen when more than one is selected, is:
+//    Browser
+//    Tab
+//    Navigation
+enum BrowserTestWaitFlags {
+  BROWSER_TEST_NONE = 0,                      // Don't wait for anything.
+  BROWSER_TEST_WAIT_FOR_BROWSER = 1 << 0,     // Wait for a new browser.
+  BROWSER_TEST_WAIT_FOR_TAB = 1 << 1,         // Wait for a new tab.
+  BROWSER_TEST_WAIT_FOR_NAVIGATION = 1 << 2,  // Wait for navigation to finish.
+
+  BROWSER_TEST_MASK = BROWSER_TEST_WAIT_FOR_BROWSER |
+                      BROWSER_TEST_WAIT_FOR_TAB |
+                      BROWSER_TEST_WAIT_FOR_NAVIGATION
+};
+
 // Turns on nestable tasks, runs the message loop, then resets nestable tasks
 // to what they were originally. Prefer this over MessageLoop::Run for in
 // process browser tests that need to block until a condition is met.
@@ -96,6 +114,12 @@ void WaitForLoadStop(NavigationController* controller);
 // Waits for a new browser to be created, returning the browser.
 Browser* WaitForNewBrowser();
 
+// Waits for a new browser to be created, returning the browser.
+// Pass in the number of browsers that exist before the navigation starts in
+// |start_count|, and it will exit even if the notification occurs before it's
+// called.
+Browser* WaitForNewBrowserWithCount(size_t start_count);
+
 // Opens |url| in an incognito browser window with the off the record profile of
 // |profile|, blocking until the navigation finishes. This will create a new
 // browser if a browser with the off the record profile does not exist.
@@ -105,6 +129,15 @@ void OpenURLOffTheRecord(Profile* profile, const GURL& url);
 // navigation finishes.
 void NavigateToURL(Browser* browser, const GURL& url);
 
+// Navigates the specified tab of |browser| to |url|, blocking until the
+// navigation finishes.
+// |disposition| indicates what tab the navigation occurs in, and
+// |browser_test_flags| controls what to wait for before continuing.
+void NavigateToURLWithDisposition(Browser* browser,
+                                  const GURL& url,
+                                  WindowOpenDisposition disposition,
+                                  int browser_test_flags);
+
 // Navigates the selected tab of |browser| to |url|, blocking until the
 // number of navigations specified complete.
 void NavigateToURLBlockUntilNavigationsComplete(Browser* browser,
@@ -191,6 +224,11 @@ void ClickOnView(const Browser* browser, ViewID vid);
 // Blocks until a notification for given |type| is received.
 void WaitForNotification(NotificationType type);
 
+// Blocks until a notification for given |type| from the specified |source|
+// is received.
+void WaitForNotificationFrom(NotificationType type,
+                             const NotificationSource& source);
+
 // Register |observer| for the given |type| and |source| and run
 // the message loop until the observer posts a quit task.
 void RegisterAndWait(NotificationObserver* observer,
@@ -212,6 +250,9 @@ bool GetNativeWindow(const Browser* browser, gfx::NativeWindow* native_window)
 // success.
 bool BringBrowserWindowToFront(const Browser* browser) WARN_UNUSED_RESULT;
 
+// Gets the first browser that is not in the specified set.
+Browser* GetBrowserNotInSet(std::set<Browser*> excluded_browsers);
+
 // Sends a key press, blocking until the key press is received or the test times
 // out. This uses ui_controls::SendKeyPress, see it for details. Returns true
 // if the event was successfully sent and received.