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

chrome/browser/download/save_page_browsertest.cc

Index: chrome/browser/download/save_page_browsertest.cc
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index c2420ba5eb08c07de08f4fedb52d396cbf998351..8be4a9afc9a14d5d6858c3800e30f6bde48dad02 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -4,8 +4,10 @@
 
 #include "base/file_path.h"
 #include "base/file_util.h"
+#include "base/i18n/file_util_icu.h"
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
+#include "base/string_util.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/net/url_request_mock_http_job.h"
 #include "chrome/browser/ui/browser.h"
@@ -141,6 +143,86 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveCompleteHTML) {
       dir.AppendASCII("1.css")));
 }
 
+IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SavedDirectory) {

[Randy Smith (Not in Mondays), 2011/05/31 23:03:10]

I know it's not consistent with the rest of the file, but could you put a
sentence or two comment before this test describing what it's testing?

[haraken1, 2011/06/02 09:13:22]

I added a detailed comment for the test. Also, for clarity, I split this test
into three tests, SaveToDirectoryForHTMLPage, SaveToDirectoryForDownloadedFiles
and SaveToUsersDownloadsFolder.

+  FilePath file_name(FILE_PATH_LITERAL("a.htm"));
+  GURL url = URLRequestMockHTTPJob::GetMockUrl(
+      FilePath(kTestDir).Append(file_name));
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  TabContentsWrapper* current_tab = browser()->GetSelectedTabContentsWrapper();
+  ASSERT_TRUE(current_tab);
+
+  ScopedTempDir website_save_temp_dir, download_save_temp_dir;
+  // The default folder for saving HTML.
+  ASSERT_TRUE(website_save_temp_dir.CreateUniqueTempDir());
+  // The default folder for saving downloaded files.
+  ASSERT_TRUE(download_save_temp_dir.CreateUniqueTempDir());
+  FilePath website_save_dir = website_save_temp_dir.path();
+  FilePath download_save_dir = download_save_temp_dir.path();
+  FilePath default_save_dir;
+  PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &default_save_dir);

[Paweł Hajdan Jr., 2011/05/31 17:15:53]

Check the return value.

[haraken1, 2011/06/02 09:13:22]

Done.

+  ASSERT_TRUE(file_util::PathExists(default_save_dir));
+  FilePath full_file_name;
+  std::string title;
+
+  title = UTF16ToASCII(

[Paweł Hajdan Jr., 2011/05/31 17:15:53]

nit: Why not initialize title when declaring it then?

Also, please put blank lines in places that results in logical blocks.
full_file_name and title are more related to this block rather than the block
above.

[haraken1, 2011/06/02 09:13:22]

Done.

+      current_tab->download_tab_helper()->SavePageToProperDirectory(
+          website_save_dir, download_save_dir, SavePackage::SAVE_AS_ONLY_HTML));
+  file_util::ReplaceIllegalCharactersInPath(&title, ' ');
+  full_file_name = website_save_dir.Append(FilePath(title + ".html"));
+
+  EXPECT_EQ(url, WaitForSavePackageToFinish());
+
+  if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
+    EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
+  EXPECT_TRUE(file_util::PathExists(full_file_name));
+  EXPECT_TRUE(file_util::ContentsEqual(
+      test_dir_.Append(FilePath(kTestDir)).Append(file_name),
+      full_file_name));
+
+  // Delete the default folder for saving HTML.

[Paweł Hajdan Jr., 2011/05/31 17:15:53]

This is weird. Why don't we just pass a nonexistent folder at the beginning?

[Randy Smith (Not in Mondays), 2011/05/31 23:03:10]

Is there a way to create a unique temp dir that doesn't exist?  I'd think that'd
require creating a new version of CreateUniqueTempDir() that (like that
function) figured out where it could put a temp dir that wasn't already being
used, but (unlike that function) didn't create it.

[Paweł Hajdan Jr., 2011/06/01 07:08:09]

I'd say just pass some dir name like
/this/dir/does/not/exist/cyuwvuvcudrandomness and assert that it indeed doesn't
exist.

[haraken1, 2011/06/02 09:13:22]

Done.

+  ASSERT_TRUE(website_save_temp_dir.Delete());
+
+  title = UTF16ToASCII(
+      current_tab->download_tab_helper()->SavePageToProperDirectory(
+          website_save_dir, download_save_dir, SavePackage::SAVE_AS_ONLY_HTML));
+  file_util::ReplaceIllegalCharactersInPath(&title, ' ');
+  full_file_name = download_save_dir.Append(FilePath(title + ".html"));
+
+  EXPECT_EQ(url, WaitForSavePackageToFinish());
+
+  if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
+    EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
+  EXPECT_TRUE(file_util::PathExists(full_file_name));
+  EXPECT_TRUE(file_util::ContentsEqual(
+      test_dir_.Append(FilePath(kTestDir)).Append(file_name),
+      full_file_name));
+
+  // Delete the default folder for downloaded files.
+  ASSERT_TRUE(download_save_temp_dir.Delete());

[Paweł Hajdan Jr., 2011/05/31 17:15:53]

Why do we explicitly do that here?

[haraken1, 2011/06/02 09:13:22]

I removed it.

+
+  title = UTF16ToASCII(
+      current_tab->download_tab_helper()->SavePageToProperDirectory(
+          website_save_dir, download_save_dir, SavePackage::SAVE_AS_ONLY_HTML));
+  file_util::ReplaceIllegalCharactersInPath(&title, ' ');
+  full_file_name = default_save_dir.Append(FilePath(title + ".html"));
+
+  EXPECT_EQ(url, WaitForSavePackageToFinish());
+
+  if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
+    EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
+  EXPECT_TRUE(file_util::PathExists(full_file_name));
+  EXPECT_TRUE(file_util::ContentsEqual(
+      test_dir_.Append(FilePath(kTestDir)).Append(file_name),
+      full_file_name));
+
+  // Delete the file that we created in a user's "Downloads" directory.
+  ASSERT_TRUE(file_util::Delete(full_file_name, false));

[Paweł Hajdan Jr., 2011/05/31 17:15:53]

Why do we explicitly do that here?

[haraken1, 2011/06/02 09:13:22]

Done.

+}
+
 IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, NoSave) {
   ui_test_utils::NavigateToURL(browser(), GURL(chrome::kAboutBlankURL));
   ASSERT_TRUE(browser()->command_updater()->SupportsCommand(IDC_SAVE_PAGE));