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

chrome/browser/download/save_package.cc

Index: chrome/browser/download/save_package.cc
diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 81bdfcfdfe5a2dd5d252165196836d0e304fa532..5970d7401fe10a285568f58b6b3ba4426d101cfa 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -11,6 +11,7 @@
 #include "base/i18n/file_util_icu.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "base/path_service.h"
 #include "base/stl_util-inl.h"
 #include "base/string_piece.h"
 #include "base/string_split.h"
@@ -1267,7 +1268,25 @@ void SavePackage::GetSaveInfo() {
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(this, &SavePackage::CreateDirectoryOnFileThread,
-          website_save_dir, download_save_dir, mime_type));
+                        website_save_dir, download_save_dir, mime_type));
+}
+
+// Used in automated testing to explicitly specify the default folder
+// for saving HTML and the default folder for saving downloaded files.
+void SavePackage::GetSaveInfo(const FilePath& website_save_dir,

[Paweł Hajdan Jr., 2011/05/30 14:15:49]

This duplicates most of SavePackage::GetSaveInfo(). Can we avoid that?

[haraken1, 2011/05/31 12:41:31]

I also first thought that I should avoid this duplication, but gave up because
this GetSaveInfo(...) is just for tests and is doing something special in it
(e.g. ignore the return value of GetSaveDirPreference()). I am afraid that if we
remove this duplication, then the original logic of GetSaveInfo() may become
hard to follow. However, if you think we should remove the duplication, I will
remove it. 

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

Why do we need to have this special variant for tests?

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

Done.

+                              const FilePath& download_save_dir,
+                              SavePackageType save_type) {
+  // Can't use tab_contents_ in the file thread, so get the data that we need
+  // before calling to it.
+  PrefService* prefs = tab_contents()->profile()->GetPrefs();
+  GetSaveDirPreference(prefs);
+  prefs->SetInteger(prefs::kSaveFileType, save_type);
+  std::string mime_type = tab_contents()->contents_mime_type();
+
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      NewRunnableMethod(this, &SavePackage::CreateDirectoryOnFileThread,
+                        website_save_dir, download_save_dir, mime_type));
 }
 
 void SavePackage::CreateDirectoryOnFileThread(
@@ -1275,15 +1294,25 @@ void SavePackage::CreateDirectoryOnFileThread(
     const FilePath& download_save_dir,
     const std::string& mime_type) {
   FilePath save_dir;
-  // If the default html/websites save folder doesn't exist...
-  if (!file_util::DirectoryExists(website_save_dir)) {
-    // If the default download dir doesn't exist, create it.
-    if (!file_util::DirectoryExists(download_save_dir))
-      file_util::CreateDirectory(download_save_dir);
+  if (file_util::DirectoryExists(website_save_dir)) {
+    // If the default html/websites save folder exists,
+    // then use the default html/websites save folder.
+    save_dir = website_save_dir;
+  } else if (file_util::DirectoryExists(download_save_dir)) {
+    // If the default html/websites save folder does not exist
+    // but the default download folder exists,
+    // then use the default download folder.
     save_dir = download_save_dir;
   } else {
-    // If it does exist, use the default save dir param.
-    save_dir = website_save_dir;
+    // If both the above folders do not exist,
+    // use the user's "Downloads" folder.
+    if (!PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &save_dir)) {
+      // Create the |download_save_dir| folder if we cannot get
+      // DIR_DEFAULT_DOWNLOADS (This will be a rare case).
+      save_dir = download_save_dir;
+    }
+    // Make sure that the folder does exist.
+    file_util::CreateDirectory(save_dir);

[Paweł Hajdan Jr., 2011/05/30 14:15:49]

This call can still fail (for example because of a race, or just some error).
Are you sure we should just ignore such failure?

[haraken1, 2011/05/31 12:41:31]

I just added an error message when CreateDirectory fails. I think that ignoring
this error may be acceptable here, since in any way we cannot avoid the
situation that we fail to save a file because the targeted directory does not
exist. For example, even if |save_dir| exists here, the |save_dir| may be
removed when a file is actually saved to the directory.

   }
 
   bool can_save_as_complete = CanSaveAsComplete(mime_type);