BUG=43066

TEST=Download UI tests are much less flaky on build bots.

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

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

3 files changed, 58 insertions, 12 deletions

diff --git a/chrome/browser/download/download_uitest.cc b/chrome/browser/download/download_uitest.cc
index f5bae1f..e37bfb4 100644
--- a/chrome/browser/download/download_uitest.cc
+++ b/chrome/browser/download/download_uitest.cc
@@ -17,6 +17,7 @@
 #include "base/string_util.h"
 #include "base/test/test_file_util.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/download/download_util.h"
 #include "chrome/browser/net/url_request_mock_http_job.h"
 #include "chrome/browser/net/url_request_slow_download_job.h"
 #include "chrome/common/chrome_constants.h"
@@ -85,6 +86,12 @@ class DownloadTest : public UITest {
     CheckDownload(file, file);
   }
 
+  void CleanupDownloadFiles(const FilePath& prefix) {
+    FilePath path_prefix(download_prefix_);
+    path_prefix = path_prefix.Append(prefix);
+    download_util::EraseUniqueDownloadFiles(path_prefix);
+  }
+
   virtual void SetUp() {
     UITest::SetUp();
     download_prefix_ = GetDownloadDirectory();
@@ -94,6 +101,11 @@ class DownloadTest : public UITest {
   void RunSizeTest(const GURL& url,
                    const std::wstring& expected_title_in_progress,
                    const std::wstring& expected_title_finished) {
+    FilePath filename;
+    net::FileURLToFilePath(url, &filename);
+    filename = filename.BaseName();
+    CleanupDownloadFiles(filename);
+
     {
       EXPECT_EQ(1, GetTabCount());
 
@@ -115,9 +127,6 @@ class DownloadTest : public UITest {
       EXPECT_TRUE(WaitForDownloadShelfVisible(window.get()));
     }
 
-    FilePath filename;
-    net::FileURLToFilePath(url, &filename);
-    filename = filename.BaseName();
     FilePath download_path = download_prefix_.Append(filename);
     EXPECT_TRUE(file_util::PathExists(download_path));
 
@@ -177,6 +186,7 @@ class DownloadTest : public UITest {
 // Additionally, there is Windows-specific flake, http://crbug.com/20809.
 TEST_F(DownloadTest, DISABLED_DownloadMimeType) {
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  CleanupDownloadFiles(file);
 
   EXPECT_EQ(1, GetTabCount());
 
@@ -199,9 +209,7 @@ TEST_F(DownloadTest, DISABLED_DownloadMimeType) {
 TEST_F(DownloadTest, FLAKY_NoDownload) {
   FilePath file(FILE_PATH_LITERAL("download-test2.html"));
   FilePath file_path = download_prefix_.Append(file);
-
-  if (file_util::PathExists(file_path))
-    ASSERT_TRUE(file_util::Delete(file_path, false));
+  CleanupDownloadFiles(file);
 
   NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(file));
   WaitUntilTabCount(1);
@@ -227,6 +235,7 @@ TEST_F(DownloadTest, FLAKY_NoDownload) {
 TEST_F(DownloadTest, DISABLED_ContentDisposition) {
   FilePath file(FILE_PATH_LITERAL("download-test3.gif"));
   FilePath download_file(FILE_PATH_LITERAL("download-test3-attachment.gif"));
+  CleanupDownloadFiles(file);
 
   NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(file));
   WaitUntilTabCount(1);
@@ -249,6 +258,7 @@ TEST_F(DownloadTest, DISABLED_ContentDisposition) {
 TEST_F(DownloadTest, DISABLED_PerWindowShelf) {
   FilePath file(FILE_PATH_LITERAL("download-test3.gif"));
   FilePath download_file(FILE_PATH_LITERAL("download-test3-attachment.gif"));
+  CleanupDownloadFiles(download_file);
 
   NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(file));
   WaitUntilTabCount(1);
@@ -339,6 +349,7 @@ TEST_F(DownloadTest, DISABLED_IncognitoDownload) {
 
   // Download something.
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  CleanupDownloadFiles(file);
   scoped_refptr<TabProxy> tab(incognito->GetTab(0));
   ASSERT_TRUE(tab.get());
   ASSERT_TRUE(tab->NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(file)));
@@ -396,6 +407,7 @@ TEST_F(DownloadTest, FLAKY_CloseNewTab1) {
   ASSERT_TRUE(tab_proxy.get());
 
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  CleanupDownloadFiles(file);
   ASSERT_TRUE(tab_proxy->NavigateToURLAsyncWithDisposition(
       URLRequestMockHTTPJob::GetMockUrl(file),
       NEW_BACKGROUND_TAB));
@@ -425,6 +437,7 @@ TEST_F(DownloadTest, FLAKY_DontCloseNewTab2) {
       FilePath(FILE_PATH_LITERAL("download_page1.html")))));
 
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  CleanupDownloadFiles(file);
   ASSERT_TRUE(tab_proxy->NavigateToURLAsync(GURL("javascript:openNew()")));
 
   ASSERT_TRUE(WaitForDownloadShelfVisible(browser));
@@ -454,6 +467,7 @@ TEST_F(DownloadTest, FLAKY_DontCloseNewTab3) {
   ASSERT_TRUE(tab_proxy->NavigateToURLAsync(GURL("javascript:openNew()")));
 
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  CleanupDownloadFiles(file);
   ASSERT_TRUE(tab_proxy->NavigateToURLAsync(
       URLRequestMockHTTPJob::GetMockUrl(file)));
 
@@ -482,6 +496,7 @@ TEST_F(DownloadTest, FLAKY_CloseNewTab2) {
       FilePath(FILE_PATH_LITERAL("download_page3.html")))));
 
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  CleanupDownloadFiles(file);
   ASSERT_TRUE(tab_proxy->NavigateToURLAsync(GURL("javascript:openNew()")));
 
   ASSERT_TRUE(WaitForDownloadShelfVisible(browser));
@@ -509,6 +524,7 @@ TEST_F(DownloadTest, FLAKY_CloseNewTab3) {
       FilePath(FILE_PATH_LITERAL("download_page4.html")))));
 
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  CleanupDownloadFiles(file);
   ASSERT_TRUE(tab_proxy->NavigateToURLAsync(
       GURL("javascript:document.getElementById('form').submit()")));
 
@@ -533,6 +549,7 @@ TEST_F(DownloadTest, DISABLED_DontCloseNewWindow) {
   ASSERT_TRUE(tab_proxy.get());
 
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  CleanupDownloadFiles(file);
   ASSERT_TRUE(tab_proxy->NavigateToURLAsyncWithDisposition(
       URLRequestMockHTTPJob::GetMockUrl(file), NEW_WINDOW));
 
@@ -558,6 +575,7 @@ TEST_F(DownloadTest, DISABLED_NewWindow) {
   ASSERT_TRUE(tab_proxy.get());
 
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  CleanupDownloadFiles(file);
   ASSERT_TRUE(tab_proxy->NavigateToURLAsyncWithDisposition(
     URLRequestMockHTTPJob::GetMockUrl(file), NEW_WINDOW));
 
diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc
index 1235285..489cb78 100644
--- a/chrome/browser/download/download_util.cc
+++ b/chrome/browser/download/download_util.cc
@@ -86,6 +86,13 @@ namespace download_util {
 // so that the animation ends faded out.
 static const int kCompleteAnimationCycles = 5;
 
+// The maximum number of 'uniquified' files we will try to create.
+// This is used when the filename we're trying to download is already in use,
+// so we create a new unique filename by appending " (nnn)" before the
+// extension, where 1 <= nnn <= kMaxUniqueFiles.
+// Also used by code that cleans up said files.
+static const int kMaxUniqueFiles = 100;
+
 // Download temporary file creation --------------------------------------------
 
 class DefaultDownloadDirectory {
@@ -647,13 +654,11 @@ void AppendNumberToPath(FilePath* path, int number) {
 // unique. If |path| does not exist, 0 is returned.  If it fails to find such
 // a number, -1 is returned.
 int GetUniquePathNumber(const FilePath& path) {
-  const int kMaxAttempts = 100;
-
   if (!file_util::PathExists(path))
     return 0;
 
   FilePath new_path;
-  for (int count = 1; count <= kMaxAttempts; ++count) {
+  for (int count = 1; count <= kMaxUniqueFiles; ++count) {
     new_path = FilePath(path);
     AppendNumberToPath(&new_path, count);
 
@@ -695,14 +700,12 @@ void CancelDownloadRequest(ResourceDispatcherHost* rdh,
 }
 
 int GetUniquePathNumberWithCrDownload(const FilePath& path) {
-  const int kMaxAttempts = 100;
-
   if (!file_util::PathExists(path) &&
       !file_util::PathExists(GetCrDownloadPath(path)))
     return 0;
 
   FilePath new_path;
-  for (int count = 1; count <= kMaxAttempts; ++count) {
+  for (int count = 1; count <= kMaxUniqueFiles; ++count) {
     new_path = FilePath(path);
     AppendNumberToPath(&new_path, count);
 
@@ -714,6 +717,27 @@ int GetUniquePathNumberWithCrDownload(const FilePath& path) {
   return -1;
 }
 
+namespace {
+

+// NOTE: If index is 0, deletes files that do not have the " (nnn)" appended.

+void DeleteUniqueDownloadFile(const FilePath& path, int index) {

+  FilePath new_path(path);

+  if (index > 0)

+    AppendNumberToPath(&new_path, index);

+  file_util::Delete(new_path, false);

+}

+

+}

+

+void EraseUniqueDownloadFiles(const FilePath& path) {

+  FilePath cr_path = GetCrDownloadPath(path);

+

+  for (int index = 0; index <= kMaxUniqueFiles; ++index) {

+    DeleteUniqueDownloadFile(path, index);

+    DeleteUniqueDownloadFile(cr_path, index);

+  }

+}

+

 FilePath GetCrDownloadPath(const FilePath& suggested_path) {
   FilePath::StringType file_name;
   base::SStringPrintf(
diff --git a/chrome/browser/download/download_util.h b/chrome/browser/download/download_util.h
index a32f171..0224a26 100644
--- a/chrome/browser/download/download_util.h
+++ b/chrome/browser/download/download_util.h
@@ -210,6 +210,10 @@ void CancelDownloadRequest(ResourceDispatcherHost* rdh,
 // a number, -1 is returned.
 int GetUniquePathNumberWithCrDownload(const FilePath& path);
 
+// Erases all downloaded files with the specified path and name prefix.
+// Used by download UI tests to clean up the download directory.
+void EraseUniqueDownloadFiles(const FilePath& path_prefix);
+
 // Returns a .crdownload intermediate path for the |suggested_path|.
 FilePath GetCrDownloadPath(const FilePath& suggested_path);