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

chrome/browser/download/download_manager.cc

Index: chrome/browser/download/download_manager.cc
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 58090f2d492e18f00c5dc16dca08589f9e43e6e1..8d26a29c04fcb3369ac6546cd2be3ffd621a0cee 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -8,7 +8,6 @@
 #include "base/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
-#include "base/path_service.h"
 #include "base/rand_util.h"
 #include "base/stl_util-inl.h"
 #include "base/stringprintf.h"
@@ -368,20 +367,27 @@ void DownloadManager::CheckIfSuggestedPathExists(int32 download_id,
                                                  const FilePath& default_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  // Make sure the default download directory exists.
-  // TODO(phajdan.jr): only create the directory when we're sure the user
-  // is going to save there and not to another directory of his choice.
-  file_util::CreateDirectory(default_path);
-
   // Check writability of the suggested path. If we can't write to it, default
-  // to the user's "My Documents" directory. We'll prompt them in this case.
+  // to the user's "Downloads" directory. We'll prompt them in this case.
   FilePath dir = state.suggested_path.DirName();
   FilePath filename = state.suggested_path.BaseName();
   if (!file_util::PathIsWritable(dir)) {
     VLOG(1) << "Unable to write to directory \"" << dir.value() << "\"";
     state.prompt_user_for_save_location = true;
-    PathService::Get(chrome::DIR_USER_DOCUMENTS, &state.suggested_path);
-    state.suggested_path = state.suggested_path.Append(filename);
+    if (!download_util::DefaultDownloadDirectory::Get(&state.suggested_path) ||
+        !file_util::PathIsWritable(state.suggested_path)) {
+      VLOG(1) << "Cannot find the user's writable \"Downloads\" folder.";
+      // If the user's writable "Downloads" folder does not exist, use the
+      // originally suggested path even if the path does not exist
+      // (This is a rare case).
+      state.suggested_path = dir.Append(filename);
+    } else {
+      state.suggested_path = state.suggested_path.Append(filename);
+    }
+    // Make sure that the folder does exist.
+    if (!file_util::CreateDirectory(state.suggested_path.DirName()))

[Paweł Hajdan Jr., 2011/06/08 09:50:23]

nit: Please use braces {} for multi-line "if" body.

[haraken1, 2011/06/09 10:16:56]

Done.

+      LOG(ERROR) << "Failed to create " <<

[Paweł Hajdan Jr., 2011/06/08 09:50:23]

Logging the error is fine, but shouldn't we exit at this point and propagate the
error? I'd like to avoid downloads mysteriously failing with the only error
message being in the logs.

[Randy Smith (Not in Mondays), 2011/06/08 22:31:18]

I'm inclined to think that the right behavior is to make sure that the user is
prompted for the file location and not worry about it beyond that.  Ideally (and
maybe actually, based on the file selection dialog on the different systems) the
file selection dialog won't allow the user to pick a location that can't be
written to.  This is a corner case, and seeding the UI with a bad location
doesn't strike me as a horrible way to deal, given that we tried really hard and
couldn't find a good location.

[haraken1, 2011/06/09 10:16:56]

I would like to agree with Randy's idea, also considering the fact that we
cannot anyway avoid the "downloads mysteriously failing" situation, since even
if the download folder exists here, the download folder may be removed when a
file is actually downloaded to the folder. 

In conclusion, I made no modification here for now. 

+          state.suggested_path.DirName().value();
   }
 
   // If the download is deemed dangerous, we'll use a temporary name for it.
@@ -495,7 +501,7 @@ void DownloadManager::OnPathExistenceAvailable(int32 download_id,
                                     contents, owning_window,
                                     reinterpret_cast<void*>(id_ptr));
     FOR_EACH_OBSERVER(Observer, observers_,
-                      SelectFileDialogDisplayed(download_id));
+                      SelectFileDialogDisplayed(download_id, suggested_path));
   } else {
     // No prompting for download, just continue with the suggested name.
     ContinueDownloadWithPath(download, suggested_path);