Create the download folder if it doesn't exist. That is, the actual download folder, not the default one.

BUG=21759
TEST=none
Review URL: http://codereview.chromium.org/219017

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

5 files changed, 88 insertions, 53 deletions

diff --git a/chrome/browser/download/save_file_manager.cc b/chrome/browser/download/save_file_manager.cc
index 20b97bb..2e35292 100644
--- a/chrome/browser/download/save_file_manager.cc
+++ b/chrome/browser/download/save_file_manager.cc
@@ -36,7 +36,7 @@ SaveFileManager::SaveFileManager(MessageLoop* ui_loop,
   DCHECK(ui_loop_);
   // Need to make sure that we are in UI thread because using g_browser_process
   // on a non-UI thread can cause crashes during shutdown.
-  DCHECK(ui_loop_ == MessageLoop::current());
+  DCHECK_EQ(ui_loop_, MessageLoop::current());
   // Cache the message loop of file thread.
   base::Thread* thread = g_browser_process->file_thread();
   if (thread)
@@ -65,7 +65,7 @@ void SaveFileManager::Shutdown() {
 
 // Stop file thread operations.
 void SaveFileManager::OnShutdown() {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
   STLDeleteValues(&save_file_map_);
 }
 
@@ -81,14 +81,14 @@ SaveFile* SaveFileManager::LookupSaveFile(int save_id) {
 // file a request from the file thread to the IO thread to generate a
 // unique save ID.
 int SaveFileManager::GetNextId() {
-  DCHECK(MessageLoop::current() == io_loop_);
+  DCHECK_EQ(MessageLoop::current(), io_loop_);
   return next_id_++;
 }
 
 void SaveFileManager::RegisterStartingRequest(const GURL& save_url,
                                               SavePackage* save_package) {
   // Make sure it runs in the UI thread.
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   int tab_id = save_package->tab_id();
 
   // Register this starting request.
@@ -101,7 +101,7 @@ void SaveFileManager::RegisterStartingRequest(const GURL& save_url,
 SavePackage* SaveFileManager::UnregisterStartingRequest(
     const GURL& save_url, int tab_id) {
   // Make sure it runs in UI thread.
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
 
   TabToStartingRequestsMap::iterator it = tab_starting_requests_.find(tab_id);
   if (it != tab_starting_requests_.end()) {
@@ -127,8 +127,8 @@ void SaveFileManager::RequireSaveJobFromOtherSource(SaveFileCreateInfo* info) {
   // pointer may be junk when we use it on file thread. We can only rely on the
   // io_loop_ pointer being valid when we run code on the UI thread (or on
   // the IO thread.
-  DCHECK(MessageLoop::current() == ui_loop_);
-  DCHECK(info->save_id == -1);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
+  DCHECK_EQ(info->save_id, -1);
   // Since the data will come from render process, so we need to start
   // this kind of save job by ourself.
   io_loop_->PostTask(FROM_HERE,
@@ -139,7 +139,7 @@ void SaveFileManager::RequireSaveJobFromOtherSource(SaveFileCreateInfo* info) {
 
 // Look up a SavePackage according to a save id.
 SavePackage* SaveFileManager::LookupPackage(int save_id) {
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   SavePackageMap::iterator it = packages_.find(save_id);
   if (it != packages_.end())
     return it->second;
@@ -155,7 +155,7 @@ void SaveFileManager::SaveURL(const GURL& url,
                               const FilePath& file_full_path,
                               URLRequestContext* request_context,
                               SavePackage* save_package) {
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   if (!io_loop_) {
     NOTREACHED();  // Net IO thread must exist.
     return;
@@ -194,13 +194,14 @@ void SaveFileManager::SaveURL(const GURL& url,
 // to remove it from the tracking map.
 void SaveFileManager::RemoveSaveFile(int save_id, const GURL& save_url,
                                      SavePackage* package) {
-  DCHECK(MessageLoop::current() == ui_loop_ && package);
+  DCHECK(package);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   // A save page job(SavePackage) can only have one manager,
   // so remove it if it exists.
   if (save_id == -1) {
     SavePackage* old_package = UnregisterStartingRequest(save_url,
                                                          package->tab_id());
-    DCHECK(old_package == package);
+    DCHECK_EQ(old_package, package);
   } else {
     SavePackageMap::iterator it = packages_.find(save_id);
     if (it != packages_.end())
@@ -224,7 +225,7 @@ SavePackage* SaveFileManager::GetSavePackageFromRenderIds(
 // Utility function for deleting specified file.
 void SaveFileManager::DeleteDirectoryOrFile(const FilePath& full_path,
                                             bool is_dir) {
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   MessageLoop* loop = file_loop();
   DCHECK(loop);
   loop->PostTask(FROM_HERE,
@@ -236,7 +237,7 @@ void SaveFileManager::DeleteDirectoryOrFile(const FilePath& full_path,
 
 void SaveFileManager::SendCancelRequest(int save_id) {
   // Cancel the request which has specific save id.
-  DCHECK(save_id > -1);
+  DCHECK_GT(save_id, -1);
   MessageLoop* loop = file_loop();
   DCHECK(loop);
   loop->PostTask(FROM_HERE,
@@ -251,10 +252,10 @@ void SaveFileManager::SendCancelRequest(int save_id) {
 // to create a SaveFile which will hold and finally destroy |info|. It will
 // then passes |info| to the UI thread for reporting saving status.
 void SaveFileManager::StartSave(SaveFileCreateInfo* info) {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
   DCHECK(info);
   SaveFile* save_file = new SaveFile(info);
-  DCHECK(LookupSaveFile(info->save_id) == NULL);
+  DCHECK(!LookupSaveFile(info->save_id));
   save_file_map_[info->save_id] = save_file;
   info->path = save_file->full_path();
 
@@ -271,7 +272,7 @@ void SaveFileManager::StartSave(SaveFileCreateInfo* info) {
 void SaveFileManager::UpdateSaveProgress(int save_id,
                                          net::IOBuffer* data,
                                          int data_len) {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
   SaveFile* save_file = LookupSaveFile(save_id);
   if (save_file) {
     bool write_success = save_file->AppendDataToFile(data->data(), data_len);
@@ -295,7 +296,7 @@ void SaveFileManager::SaveFinished(int save_id,
                                    GURL save_url,
                                    int render_process_id,
                                    bool is_success) {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
   SaveFileMap::iterator it = save_file_map_.find(save_id);
   if (it != save_file_map_.end()) {
     SaveFile* save_file = it->second;
@@ -321,7 +322,7 @@ void SaveFileManager::SaveFinished(int save_id,
 // Notifications sent from the file thread and run on the UI thread.
 
 void SaveFileManager::OnStartSave(const SaveFileCreateInfo* info) {
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   SavePackage* save_package =
       GetSavePackageFromRenderIds(info->render_process_id,
                                   info->render_view_id);
@@ -343,7 +344,7 @@ void SaveFileManager::OnStartSave(const SaveFileCreateInfo* info) {
       SendCancelRequest(info->save_id);
       return;
     }
-    DCHECK(old_save_package == save_package);
+    DCHECK_EQ(old_save_package, save_package);
     packages_[info->save_id] = save_package;
   } else {
     NOTREACHED();
@@ -355,7 +356,7 @@ void SaveFileManager::OnStartSave(const SaveFileCreateInfo* info) {
 
 void SaveFileManager::OnUpdateSaveProgress(int save_id, int64 bytes_so_far,
                                            bool write_success) {
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   SavePackage* package = LookupPackage(save_id);
   if (package)
     package->UpdateSaveProgress(save_id, bytes_so_far, write_success);
@@ -366,14 +367,14 @@ void SaveFileManager::OnUpdateSaveProgress(int save_id, int64 bytes_so_far,
 void SaveFileManager::OnSaveFinished(int save_id,
                                      int64 bytes_so_far,
                                      bool is_success) {
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   SavePackage* package = LookupPackage(save_id);
   if (package)
     package->SaveFinished(save_id, bytes_so_far, is_success);
 }
 
 void SaveFileManager::OnErrorFinished(GURL save_url, int tab_id) {
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   SavePackage* save_package = UnregisterStartingRequest(save_url, tab_id);
   if (save_package)
     save_package->SaveFailed(save_url);
@@ -381,7 +382,7 @@ void SaveFileManager::OnErrorFinished(GURL save_url, int tab_id) {
 
 void SaveFileManager::OnCancelSaveRequest(int render_process_id,
                                           int request_id) {
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
   DCHECK(io_loop_);
   io_loop_->PostTask(FROM_HERE,
       NewRunnableMethod(this,
@@ -397,7 +398,7 @@ void SaveFileManager::OnSaveURL(const GURL& url,
                                 int render_process_host_id,
                                 int render_view_id,
                                 URLRequestContext* request_context) {
-  DCHECK(MessageLoop::current() == io_loop_);
+  DCHECK_EQ(MessageLoop::current(), io_loop_);
   resource_dispatcher_host_->BeginSaveFile(url,
                                            referrer,
                                            render_process_host_id,
@@ -407,8 +408,8 @@ void SaveFileManager::OnSaveURL(const GURL& url,
 
 void SaveFileManager::OnRequireSaveJobFromOtherSource(
     SaveFileCreateInfo* info) {
-  DCHECK(MessageLoop::current() == io_loop_);
-  DCHECK(info->save_id == -1);
+  DCHECK_EQ(MessageLoop::current(), io_loop_);
+  DCHECK_EQ(info->save_id, -1);
   // Generate a unique save id.
   info->save_id = GetNextId();
   // Start real saving action.
@@ -422,7 +423,7 @@ void SaveFileManager::OnRequireSaveJobFromOtherSource(
 
 void SaveFileManager::ExecuteCancelSaveRequest(int render_process_id,
                                                int request_id) {
-  DCHECK(MessageLoop::current() == io_loop_);
+  DCHECK_EQ(MessageLoop::current(), io_loop_);
   resource_dispatcher_host_->CancelRequest(render_process_id,
                                            request_id,
                                            false);
@@ -436,7 +437,7 @@ void SaveFileManager::ExecuteCancelSaveRequest(int render_process_id,
 // sent from the UI thread, the saving job may have already completed and
 // won't exist in our map.
 void SaveFileManager::CancelSave(int save_id) {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
   SaveFileMap::iterator it = save_file_map_.find(save_id);
   if (it != save_file_map_.end()) {
     SaveFile* save_file = it->second;
@@ -473,7 +474,7 @@ void SaveFileManager::CancelSave(int save_id) {
 void SaveFileManager::SaveLocalFile(const GURL& original_file_url,
                                     int save_id,
                                     int render_process_id) {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
   SaveFile* save_file = LookupSaveFile(save_id);
   if (!save_file)
     return;
@@ -503,7 +504,7 @@ void SaveFileManager::SaveLocalFile(const GURL& original_file_url,
 
 void SaveFileManager::OnDeleteDirectoryOrFile(const FilePath& full_path,
                                               bool is_dir) {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
   DCHECK(!full_path.empty());
 
   file_util::Delete(full_path, is_dir);
@@ -513,7 +514,7 @@ void SaveFileManager::OnDeleteDirectoryOrFile(const FilePath& full_path,
 // We run on this thread to avoid blocking the UI with slow Shell operations.
 #if !defined(OS_MACOSX)
 void SaveFileManager::OnShowSavedFileInShell(const FilePath full_path) {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
   platform_util::ShowItemInFolder(full_path);
 }
 #endif
@@ -523,7 +524,7 @@ void SaveFileManager::RenameAllFiles(
     const FilePath& resource_dir,
     int render_process_id,
     int render_view_id) {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
 
   if (!resource_dir.empty() && !file_util::PathExists(resource_dir))
     file_util::CreateDirectory(resource_dir);
@@ -549,7 +550,7 @@ void SaveFileManager::RenameAllFiles(
 
 void SaveFileManager::OnFinishSavePageJob(int render_process_id,
                                           int render_view_id) {
-  DCHECK(MessageLoop::current() == ui_loop_);
+  DCHECK_EQ(MessageLoop::current(), ui_loop_);
 
   SavePackage* save_package =
       GetSavePackageFromRenderIds(render_process_id, render_view_id);
@@ -562,7 +563,7 @@ void SaveFileManager::OnFinishSavePageJob(int render_process_id,
 
 void SaveFileManager::RemoveSavedFileFromFileMap(
     const SaveIDList& save_ids) {
-  DCHECK(MessageLoop::current() == file_loop());
+  DCHECK_EQ(MessageLoop::current(), file_loop());
 
   for (SaveIDList::const_iterator i = save_ids.begin();
       i != save_ids.end(); ++i) {
@@ -576,3 +577,12 @@ void SaveFileManager::RemoveSavedFileFromFileMap(
     }
   }
 }
+
+void SaveFileManager::CreateDownloadDirectory(FilePath save_dir,
+                                              SavePackage* save_package) {
+  file_util::CreateDirectory(save_dir);
+  ui_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(save_package,
+                        &SavePackage::ContinueGetSaveInfo,
+                        save_dir));
+}
diff --git a/chrome/browser/download/save_file_manager.h b/chrome/browser/download/save_file_manager.h
index db1e866..71067a5 100644
--- a/chrome/browser/download/save_file_manager.h
+++ b/chrome/browser/download/save_file_manager.h
@@ -58,6 +58,8 @@
 #ifndef CHROME_BROWSER_DOWNLOAD_SAVE_FILE_MANAGER_H__
 #define CHROME_BROWSER_DOWNLOAD_SAVE_FILE_MANAGER_H__
 
+#include <string>
+
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/hash_tables.h"
@@ -122,6 +124,9 @@ class SaveFileManager
   void OnShowSavedFileInShell(const FilePath full_path);
 #endif
 
+  // Helper to create the download directory.
+  void CreateDownloadDirectory(FilePath save_dir,
+                               SavePackage* save_package);
   // Helper function for deleting specified file.
   void DeleteDirectoryOrFile(const FilePath& full_path, bool is_dir);
 
@@ -262,7 +267,7 @@ class SaveFileManager
   typedef base::hash_map<int, StartingRequestsMap> TabToStartingRequestsMap;
   TabToStartingRequestsMap tab_starting_requests_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(SaveFileManager);
+  DISALLOW_COPY_AND_ASSIGN(SaveFileManager);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_SAVE_FILE_MANAGER_H__
diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 2fd35ee..33c8bfb 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -144,6 +144,7 @@ SavePackage::SavePackage(TabContents* web_content,
          saved_main_file_path_.value().length() <= kMaxFilePathLength);
   DCHECK(!saved_main_directory_path_.empty() &&
          saved_main_directory_path_.value().length() < kMaxFilePathLength);
+  InternalInit();
 }
 
 SavePackage::SavePackage(TabContents* tab_contents)
@@ -160,10 +161,12 @@ SavePackage::SavePackage(TabContents* tab_contents)
   const GURL& current_page_url = tab_contents_->GetURL();
   DCHECK(current_page_url.is_valid());
   page_url_ = current_page_url;
+  InternalInit();
 }
 
 // This is for testing use. Set |finished_| as true because we don't want
 // method Cancel to be be called in destructor in test mode.
+// We also don't call InternalInit().
 SavePackage::SavePackage(const FilePath& file_full_path,
                          const FilePath& directory_full_path)
     : file_manager_(NULL),
@@ -233,6 +236,22 @@ void SavePackage::Cancel(bool user_action) {
   }
 }
 
+// Init() can be called directly, or indirectly via GetSaveInfo(). In both
+// cases, we need file_manager_ to be initialized, so we do this first.
+void SavePackage::InternalInit() {
+  ResourceDispatcherHost* rdh = g_browser_process->resource_dispatcher_host();
+  if (!rdh) {
+    NOTREACHED();
+    return;
+  }
+
+  file_manager_ = rdh->save_file_manager();
+  if (!file_manager_) {
+    NOTREACHED();
+    return;
+  }
+}
+
 // Initialize the SavePackage.
 bool SavePackage::Init() {
   // Set proper running state.
@@ -250,18 +269,6 @@ bool SavePackage::Init() {
 
   request_context_ = profile->GetRequestContext();
 
-  ResourceDispatcherHost* rdh = g_browser_process->resource_dispatcher_host();
-  if (!rdh) {
-    NOTREACHED();
-    return false;
-  }
-
-  file_manager_ = rdh->save_file_manager();
-  if (!file_manager_) {
-    NOTREACHED();
-    return false;
-  }
-
   // Create the fake DownloadItem and display the view.
   download_ = new DownloadItem(1, saved_main_file_path_, 0, page_url_, GURL(),
       "", FilePath(), Time::Now(), 0, -1, -1, false, false);
@@ -1046,6 +1053,17 @@ FilePath SavePackage::GetSaveDirPreference(PrefService* prefs) {
 }
 
 void SavePackage::GetSaveInfo() {
+  FilePath save_dir =
+      GetSaveDirPreference(tab_contents_->profile()->GetPrefs());
+  file_manager_->file_loop()->PostTask(FROM_HERE,
+      NewRunnableMethod(file_manager_,
+                        &SaveFileManager::CreateDownloadDirectory,
+                        save_dir,
+                        this));
+  // CreateDownloadDirectory() calls ContinueGetSaveInfo() below.
+}
+
+void SavePackage::ContinueGetSaveInfo(FilePath save_dir) {
   // Use "Web Page, Complete" option as default choice of saving page.
   int file_type_index = 2;
   SelectFileDialog::FileTypeInfo file_type_info;
@@ -1059,8 +1077,6 @@ void SavePackage::GetSaveInfo() {
 
   FilePath title =
       FilePath::FromWStringHack(UTF16ToWideHack(tab_contents_->GetTitle()));
-  FilePath save_dir =
-      GetSaveDirPreference(tab_contents_->profile()->GetPrefs());
   FilePath suggested_path =
       save_dir.Append(GetSuggestedNameForSaveAs(title, can_save_as_complete));
 
diff --git a/chrome/browser/download/save_package.h b/chrome/browser/download/save_package.h
index 224cee2..4b74c14 100644
--- a/chrome/browser/download/save_package.h
+++ b/chrome/browser/download/save_package.h
@@ -130,6 +130,7 @@ class SavePackage : public base::RefCountedThreadSafe<SavePackage>,
   int tab_id() const { return tab_id_; }
 
   void GetSaveInfo();
+  void ContinueGetSaveInfo(FilePath save_dir);
   void ContinueSave(SavePackageParam* param,
                     const FilePath& final_name,
                     int index);
@@ -199,6 +200,9 @@ class SavePackage : public base::RefCountedThreadSafe<SavePackage>,
   SavePackage(const FilePath& file_full_path,
               const FilePath& directory_full_path);
 
+  // Notes from Init() above applies here as well.
+  void InternalInit();
+
   void Stop();
   void CheckFinish();
   void SaveNextFile(bool process_all_remainder_items);
diff --git a/chrome/common/chrome_paths.cc b/chrome/common/chrome_paths.cc
index 51ff8f5..f504b4a 100644
--- a/chrome/common/chrome_paths.cc
+++ b/chrome/common/chrome_paths.cc
@@ -5,7 +5,6 @@
 #include "chrome/common/chrome_paths.h"
 
 #include "base/command_line.h"
-#include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
@@ -61,8 +60,8 @@ bool PathProvider(int key, FilePath* result) {
       return true;
 #else
       return PathService::Get(base::DIR_EXE, result);
-#endif // defined(OS_MACOSX)
-#endif // NDEBUG
+#endif  // defined(OS_MACOSX)
+#endif  // NDEBUG
     case chrome::FILE_RESOURCE_MODULE:
       return PathService::Get(base::FILE_MODULE, result);
   }
@@ -98,7 +97,8 @@ bool PathProvider(int key, FilePath* result) {
     case chrome::DIR_DEFAULT_DOWNLOADS:
       if (!GetUserDownloadsDirectory(&cur))
         return false;
-      create_dir = true;
+      // Do not create the download directory here, we have done it twice now
+      // and annoyed a lot of users.
       break;
     case chrome::DIR_CRASH_DUMPS:
       // The crash reports are always stored relative to the default user data