Once we've decided that a path doesn't exist, reserve it by creating an empty file so that we don't later think that the same path doesn't exist.

BUG=3662
R=paulg

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

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

2 files changed, 25 insertions, 24 deletions

diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 419bcfe..d746355 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -398,17 +398,6 @@ void DownloadManager::Shutdown() {
   shutdown_needed_ = false;
 }
 
-// Determines whether the "save as" dialog should be displayed to the user
-// when downloading a file.
-bool DownloadManager::ShouldDisplaySaveAsDialog(
-    const DownloadCreateInfo* info) {
-  return
-      *prompt_for_download_ ||      // User always wants a prompt.
-      info->save_as ||              // "Save as ..." operation.
-      info->path_uniquifier == -1;  // Last attempt to generate a unique file
-                                    // name failed.
-}
-
 // Issue a history query for downloads matching 'search_text'. If 'search_text'
 // is empty, return all downloads that we know about.
 void DownloadManager::GetDownloads(Observer* observer,
@@ -530,18 +519,24 @@ void DownloadManager::StartDownload(DownloadCreateInfo* info) {
   DCHECK(MessageLoop::current() == ui_loop_);
   DCHECK(info);
 
+  // Freeze the user's preference for showing a Save As dialog.  We're going to
+  // bounce around a bunch of threads and we don't want to worry about race
+  // conditions where the user changes this pref out from under us.
+  if (*prompt_for_download_)
+    info->save_as = true;
+
   // Determine the proper path for a download, by choosing either the default
   // download directory, or prompting the user.
   std::wstring generated_name;
   GenerateFilename(info, &generated_name);
-  if (ShouldDisplaySaveAsDialog(info) && !last_download_path_.empty())
+  if (info->save_as && !last_download_path_.empty())
     info->suggested_path = last_download_path_;
   else
     info->suggested_path = *download_path_;
   file_util::AppendToPath(&info->suggested_path, generated_name);
 
-  // Let's check if this download is dangerous, based on its name.
-  if (!ShouldDisplaySaveAsDialog(info)) {
+  if (!info->save_as) {
+    // Let's check if this download is dangerous, based on its name.
     const std::wstring filename =
         file_util::GetFilenameFromPath(info->suggested_path);
     info->is_dangerous = IsDangerous(filename);
@@ -581,10 +576,10 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info) {
     while (path.empty()) {
       SStringPrintf(&file_name, L"unconfirmed %d.download",
                     base::RandInt(0, 100000));
-       path = dir;
-       file_util::AppendToPath(&path, file_name);
-       if (file_util::PathExists(path))
-         path.clear();
+      path = dir;
+      file_util::AppendToPath(&path, file_name);
+      if (file_util::PathExists(path))
+        path.clear();
     }
     info->suggested_path = path;
   } else {
@@ -594,9 +589,19 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info) {
       // Setting path_uniquifier to 0 to make sure we don't try to unique it
       // later on.
       info->path_uniquifier = 0;
+    } else if (info->path_uniquifier == -1) {
+      // We failed to find a unique path.  We have to prompt the user.
+      info->save_as = true;
     }
   }
 
+  if (!info->save_as) {
+    // Create an empty file at the suggested path so that we don't allocate the
+    // same "non-existant" path to multiple downloads.
+    // See: http://code.google.com/p/chromium/issues/detail?id=3662
+    file_util::WriteFile(info->suggested_path, "", 0);
+  }
+
   // Now we return to the UI thread.
   ui_loop_->PostTask(FROM_HERE,
       NewRunnableMethod(this,
@@ -608,7 +613,7 @@ void DownloadManager::OnPathExistenceAvailable(DownloadCreateInfo* info) {
   DCHECK(MessageLoop::current() == ui_loop_);
   DCHECK(info);
 
-  if (ShouldDisplaySaveAsDialog(info)) {
+  if (info->save_as) {
     // We must ask the user for the place to put the download.
     if (!select_file_dialog_.get())
       select_file_dialog_ = SelectFileDialog::Create(this);
@@ -1226,7 +1231,7 @@ void DownloadManager::SaveAutoOpens() {
 
 void DownloadManager::FileSelected(const std::wstring& path, void* params) {
   DownloadCreateInfo* info = reinterpret_cast<DownloadCreateInfo*>(params);
-  if (ShouldDisplaySaveAsDialog(info))
+  if (info->save_as)
     last_download_path_ = file_util::GetDirectoryFromPath(path);
   ContinueStartDownload(info, path);
 }
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index d840957..e2d3b5b 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -411,10 +411,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   // Shutdown the download manager.  This call is needed only after Init.
   void Shutdown();
 
-  // Determines whether the "save as" dialog should be displayed to the user
-  // when downloading a file.
-  bool ShouldDisplaySaveAsDialog(const DownloadCreateInfo* info);
-
   // Called on the download thread to check whether the suggested file path
   // exists.  We don't check if the file exists on the UI thread to avoid UI
   // stalls from interacting with the file system.