When the download folder does not exist, change the download folder to a user's "Downloads"

chrome/browser/download/download_browsertest.cc

Index: chrome/browser/download/download_browsertest.cc
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
index e5826c9d8a52474d6a5cc7d2a29be648361cacbb..e832a10260263e20814d6b42875062f641853b1b 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -39,6 +39,12 @@
 
 namespace {
 
+enum SelectExpectation {
+  EXPECT_NO_SELECT_DIALOG = -1,
+  EXPECT_NOTHING,
+  EXPECT_SELECT_DIALOG
+};
+
 // Construction of this class defines a system state, based on some number
 // of downloads being seen in a particular state + other events that
 // may occur in the download system.  That state will be recorded if it
@@ -88,8 +94,18 @@ class DownloadsObserver : public DownloadManager::Observer,
     download_manager_->RemoveObserver(this);
   }
 
-  // State accessors.
-  bool select_file_dialog_seen() { return select_file_dialog_seen_; }
+  // Checks if the select file dialog was displayed as we expected.
+  // If displayed, checks the suggested path in the dialog.
+  bool CheckSelectFileDialogState(SelectExpectation expectation,
+                                  const FilePath& expected_suggested_path) {
+    if (expectation == EXPECT_NO_SELECT_DIALOG) {
+      return !select_file_dialog_seen_;
+    } else if (expectation == EXPECT_SELECT_DIALOG) {
+      return select_file_dialog_seen_ &&
+        suggested_path_ == expected_suggested_path;
+    }
+    return true;
+  }
 
   // Wait for whatever state was specified in the constructor.
   void WaitForFinished() {
@@ -151,8 +167,10 @@ class DownloadsObserver : public DownloadManager::Observer,
     }
   }
 
-  virtual void SelectFileDialogDisplayed(int32 /* id */) {
+  virtual void SelectFileDialogDisplayed(
+      int32 /* id */, const FilePath& suggested_path) {
     select_file_dialog_seen_ = true;
+    suggested_path_ = suggested_path;
     SignalIfFinished();
   }
 
@@ -218,6 +236,9 @@ class DownloadsObserver : public DownloadManager::Observer,
   // True if we've seen the select file dialog.
   bool select_file_dialog_seen_;
 
+  // The suggested file path in the select file dialog.
+  FilePath suggested_path_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadsObserver);
 };
 
@@ -393,12 +414,6 @@ class CancelTestDataCollector
 
 class DownloadTest : public InProcessBrowserTest {
  public:
-  enum SelectExpectation {
-    EXPECT_NO_SELECT_DIALOG = -1,
-    EXPECT_NOTHING,
-    EXPECT_SELECT_DIALOG
-  };
-
   DownloadTest() {
     EnableDOMAutomation();
   }
@@ -467,6 +482,11 @@ class DownloadTest : public InProcessBrowserTest {
     return true;
   }
 
+  // Delete the default folder for downloaded files.
+  bool DeleteDownloadsDirectory() {
+    return downloads_directory_.Delete();
+  }
+
   DownloadPrefs* GetDownloadPrefs(Browser* browser) {
     return browser->profile()->GetDownloadManager()->download_prefs();
   }
@@ -503,16 +523,21 @@ class DownloadTest : public InProcessBrowserTest {
   // 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
+  // |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 routine exits.  The only effect |expectation|
   // has is whether or not the test succeeds.
+  // |expected_suggested_path| is the path expected to be suggested in the
+  // select file dialog. This |expected_suggested_path| must be specified
+  // if |expectation| is EXPECT_SELECT_DIALOG. If |expectation| is
+  // not EXPECT_SELECT_DIALOG, |expected_suggested_path| is ignored.
   // |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,
+                                      const FilePath& expected_suggested_path,
                                       int browser_test_flags) {
     // Setup notification, navigate, and block.
     scoped_ptr<DownloadsObserver> observer(CreateWaiter(browser, 1));
@@ -525,22 +550,22 @@ class DownloadTest : public InProcessBrowserTest {
     // 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());
-    }
+    // Checks the state of the select file dialog.
+    EXPECT_TRUE(observer->CheckSelectFileDialogState(expectation,
+                                                     expected_suggested_path));

[Randy Smith (Not in Mondays), 2011/06/06 15:31:29]

I'd suggest (i.e. I don't feel very strongly) that you expand the contents of
CheckSelectFileDialogState out here inline into a couple of separate EXPECT_*
calls.  I have two reasons for the suggestion:
* If a function's only used in one place and I don't see likely future uses of
it elsewhere, it can produce simpler code to have all the code in the same
place.  In this case I think it does, since you don't need to remember/know the
definition of CheckSelectFileDialogState.
* Multiple EXPECT statements mean that if there's a test failure, you get a
narrower pointer to what actually failed in the failure status.

[haraken1, 2011/06/08 04:12:08]

Done.

   }
 
   // Download a file in the current tab, then wait for the download to finish.
   void DownloadAndWait(Browser* browser,
                        const GURL& url,
-                       SelectExpectation expectation) {
+                       SelectExpectation expectation,
+                       const FilePath& expected_suggested_path) {

[Randy Smith (Not in Mondays), 2011/06/06 15:31:29]

I'm inclined to say you should make a new variant of DownloadAndWait rather than
expand the current one.  Most of the calls to this just get a null FilePath(),
so for them it's just extra work.  DownloadAndWaitWithDisposition() is complex
enough so duplicating it for caller convenience probably doesn't make sense, but
my judgement goes the other way for DownloadAndWait().

[haraken1, 2011/06/08 04:12:08]

I made DownloadAndWaitWithoutDialog() and DownloadAndWaitWithDialog(). 

     DownloadAndWaitWithDisposition(
         browser,
         url,
         CURRENT_TAB,
         expectation,
+        expected_suggested_path,
         ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
   }
 
@@ -778,8 +803,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadMimeType) {
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
   GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
 
-  // Download the file and wait.  We do not expect the Select File dialog.
-  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
+  // Download the file and wait.  We do not expect the select file dialog.
+  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG, FilePath());
 
   // Check state.
   EXPECT_EQ(1, browser()->tab_count());
@@ -787,6 +812,72 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadMimeType) {
   EXPECT_TRUE(IsDownloadUIVisible(browser()));
 }
 
+// Checks if a file is saved to the user's "Downloads" folder
+// in the following situation:
+// The default folder for downloaded files does not exist.
+// The user's "Downloads" folder exists.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadedFolder1) {
+  ASSERT_TRUE(InitialSetup(false));
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+
+  // Temporarily change the user's "Downloads" folder.
+  FilePath prev_default_downloads_dir;
+  ASSERT_TRUE(PathService::Get(
+      chrome::DIR_DEFAULT_DOWNLOADS, &prev_default_downloads_dir));
+  ScopedTempDir default_downloads_dir;
+  ASSERT_TRUE(default_downloads_dir.CreateUniqueTempDir());
+  PathService::Set(chrome::DIR_DEFAULT_DOWNLOADS, default_downloads_dir.path());
+  FilePath downloaded_file = default_downloads_dir.path().Append(file);
+
+  // Delete the default folder for downloaded files.
+  ASSERT_TRUE(DeleteDownloadsDirectory());
+  ASSERT_FALSE(file_util::PathExists(GetDownloadDirectory(browser())));
+
+  // Download the file and wait.  We expect the select file dialog.
+  DownloadAndWait(browser(), url, EXPECT_SELECT_DIALOG, downloaded_file);
+
+  EXPECT_FALSE(file_util::PathExists(downloaded_file));
+  EXPECT_FALSE(file_util::PathExists(GetDownloadDirectory(browser())));
+  EXPECT_EQ(1, browser()->tab_count());
+
+  // Restore the user's "Downloads" folder.
+  PathService::Set(chrome::DIR_DEFAULT_DOWNLOADS, prev_default_downloads_dir);
+}
+
+// Checks if the default folder for downloaded files is created
+// and a file is saved to the folder in the following situation:
+// The default folder for downloaded files does not exist.
+// The user's "Downloads" folder does not exist.
+IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadedFolder2) {
+  ASSERT_TRUE(InitialSetup(false));
+  FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
+  GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
+  FilePath downloaded_file(DestinationFile(browser(), file));
+
+  // Temporarily change the user's "Downloads" folder.
+  FilePath prev_default_downloads_dir;
+  ASSERT_TRUE(PathService::Get(
+      chrome::DIR_DEFAULT_DOWNLOADS, &prev_default_downloads_dir));
+  FilePath nonexistent_dir("/tmp/koakuma_andre_moemoe_nyannyannyan");
+  PathService::Set(chrome::DIR_DEFAULT_DOWNLOADS, nonexistent_dir);
+
+  // Delete the default folder for downloaded files.
+  ASSERT_TRUE(DeleteDownloadsDirectory());
+  ASSERT_FALSE(file_util::PathExists(GetDownloadDirectory(browser())));
+
+  // Download the file and wait.  We expect the select file dialog.
+  DownloadAndWait(browser(), url, EXPECT_SELECT_DIALOG, downloaded_file);
+
+  EXPECT_FALSE(file_util::PathExists(downloaded_file));
+  EXPECT_FALSE(file_util::PathExists(nonexistent_dir));
+  EXPECT_TRUE(file_util::PathExists(GetDownloadDirectory(browser())));
+  EXPECT_EQ(1, browser()->tab_count());
+
+  // Restore the user's "Downloads" folder.
+  PathService::Set(chrome::DIR_DEFAULT_DOWNLOADS, prev_default_downloads_dir);
+}
+
 #if defined(OS_WIN)
 // Download a file and confirm that the zone identifier (on windows)
 // is set to internet.
@@ -795,8 +886,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CheckInternetZone) {
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
   GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
 
-  // Download the file and wait.  We do not expect the Select File dialog.
-  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
+  // Download the file and wait.  We do not expect the select file dialog.
+  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG, FilePath());
 
   // Check state.  Special file state must be checked before CheckDownload,
   // as CheckDownload will delete the output file.
@@ -809,7 +900,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CheckInternetZone) {
 }
 #endif
 
-// Put up a Select File dialog when the file is downloaded, due to its MIME
+// 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
@@ -820,14 +911,15 @@ 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));
+  FilePath file_path(DestinationFile(browser(), file));
 
-  // Download the file and wait.  We expect the Select File dialog to appear
+  // Download the file and wait.  We expect the select file dialog to appear
   // due to the MIME type.
-  DownloadAndWait(browser(), url, EXPECT_SELECT_DIALOG);
+  DownloadAndWait(browser(), url, EXPECT_SELECT_DIALOG, file_path);
 
   // Check state.
   EXPECT_EQ(1, browser()->tab_count());
-  // Since we exited while the Select File dialog was visible, there should not
+  // 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(IsDownloadUIVisible(browser()));
 }
@@ -862,7 +954,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, ContentDisposition) {
   FilePath download_file(FILE_PATH_LITERAL("download-test3-attachment.gif"));
 
   // Download a file and wait.
-  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
+  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG, FilePath());
 
   CheckDownload(browser(), download_file, file);
 
@@ -882,7 +974,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, PerWindowShelf) {
   FilePath download_file(FILE_PATH_LITERAL("download-test3-attachment.gif"));
 
   // Download a file and wait.
-  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
+  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG, FilePath());
 
   CheckDownload(browser(), download_file, file);
 
@@ -947,7 +1039,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, IncognitoDownload) {
   // 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);
+  DownloadAndWait(incognito, url, EXPECT_NO_SELECT_DIALOG, FilePath());
 
   // We should still have 2 windows.
   ExpectWindowCountAfterDownload(2);
@@ -1013,6 +1105,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CloseNewTab1) {
       url,
       NEW_BACKGROUND_TAB,
       EXPECT_NO_SELECT_DIALOG,
+      FilePath(),
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
 
   // When the download finishes, we should still have one tab.
@@ -1045,6 +1138,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DontCloseNewTab2) {
                                  GURL("javascript:openNew()"),
                                  CURRENT_TAB,
                                  EXPECT_NO_SELECT_DIALOG,
+                                 FilePath(),
                                  ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
 
   // When the download finishes, we should have two tabs.
@@ -1087,6 +1181,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DontCloseNewTab3) {
                                  url,
                                  CURRENT_TAB,
                                  EXPECT_NO_SELECT_DIALOG,
+                                 FilePath(),
                                  ui_test_utils::BROWSER_TEST_NONE);
 
   // When the download finishes, we should have two tabs.
@@ -1120,6 +1215,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CloseNewTab2) {
                                  GURL("javascript:openNew()"),
                                  CURRENT_TAB,
                                  EXPECT_NO_SELECT_DIALOG,
+                                 FilePath(),
                                  ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
 
   // When the download finishes, we should still have one tab.
@@ -1155,6 +1251,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CloseNewTab3) {
       GURL("javascript:document.getElementById('form').submit()"),
       CURRENT_TAB,
       EXPECT_NO_SELECT_DIALOG,
+      FilePath(),
       ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
 
   // When the download finishes, we should still have one tab.
@@ -1185,6 +1282,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, NewWindow) {
                                  url,
                                  NEW_WINDOW,
                                  EXPECT_NO_SELECT_DIALOG,
+                                 FilePath(),
                                  ui_test_utils::BROWSER_TEST_NONE);
 
   // When the download finishes, the download shelf SHOULD NOT be visible in
@@ -1306,8 +1404,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadHistoryCheck) {
   int64 origin_size;
   file_util::GetFileSize(origin_file, &origin_size);
 
-  // Download the file and wait.  We do not expect the Select File dialog.
-  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
+  // Download the file and wait.  We do not expect the select file dialog.
+  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG, FilePath());
 
   // Get details of what downloads have just happened.
   std::vector<DownloadItem*> downloads;
@@ -1344,7 +1442,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, ChromeURLAfterDownload) {
   GURL extensions_url(chrome::kChromeUIExtensionsURL);
 
   ui_test_utils::NavigateToURL(browser(), flags_url);
-  DownloadAndWait(browser(), download_url, EXPECT_NO_SELECT_DIALOG);
+  DownloadAndWait(browser(), download_url, EXPECT_NO_SELECT_DIALOG, FilePath());
   ui_test_utils::NavigateToURL(browser(), extensions_url);
   TabContents* contents = browser()->GetSelectedTabContents();
   ASSERT_TRUE(contents);
@@ -1381,7 +1479,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DISABLED_BrowserCloseAfterDownload) {
       &result));
   EXPECT_TRUE(result);
 
-  DownloadAndWait(browser(), download_url, EXPECT_NO_SELECT_DIALOG);
+  DownloadAndWait(browser(), download_url, EXPECT_NO_SELECT_DIALOG, FilePath());
 
   ui_test_utils::WindowedNotificationObserver signal(
       NotificationType::BROWSER_CLOSED,
@@ -1409,7 +1507,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, MAYBE_AutoOpen) {
   MockDownloadOpeningObserver observer(
       browser()->profile()->GetDownloadManager());
 
-  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
+  DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG, FilePath());
 
   // Find the download and confirm it was opened.
   std::vector<DownloadItem*> downloads;