Download filename determination refactor (3/3)

chrome/browser/download/chrome_download_manager_delegate.cc

Index: chrome/browser/download/chrome_download_manager_delegate.cc
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index 9a7f18ee80c9b598105018017b320f4f160acfd5..13d80418dba1ffe95cc3abe07bdcb91bcbbec30d 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -135,7 +135,8 @@ DownloadId ChromeDownloadManagerDelegate::GetNextId() {
       GetDelegate()->GetNextId();
 }
 
-bool ChromeDownloadManagerDelegate::ShouldStartDownload(int32 download_id) {
+bool ChromeDownloadManagerDelegate::DetermineDownloadTarget(
+    DownloadItem* download) {
   // We create a download item and store it in our download map, and inform the
   // history system of a new download. Since this method can be called while the
   // history service thread is still reading the persistent state, we do not
@@ -143,11 +144,6 @@ bool ChromeDownloadManagerDelegate::ShouldStartDownload(int32 download_id) {
   // observers at this point. OnCreateDownloadEntryComplete() handles that
   // finalization of the the download creation as a callback from the history
   // thread.
-  DownloadItem* download =
-      download_manager_->GetActiveDownloadItem(download_id);
-  if (!download)
-    return false;
-
 #if defined(ENABLE_SAFE_BROWSING)
   DownloadProtectionService* service = GetDownloadProtectionService();
   if (service) {
@@ -159,14 +155,17 @@ bool ChromeDownloadManagerDelegate::ShouldStartDownload(int32 download_id) {
             &ChromeDownloadManagerDelegate::CheckDownloadUrlDone,
             this,
             download->GetId()));
-    return false;
+    return true;
   }
 #endif
-  CheckDownloadUrlDone(download_id, DownloadProtectionService::SAFE);
-  return false;
+  CheckDownloadUrlDone(download->GetId(), DownloadProtectionService::SAFE);
+  return true;
 }
 
-void ChromeDownloadManagerDelegate::ChooseDownloadPath(DownloadItem* item) {
+void ChromeDownloadManagerDelegate::ChooseDownloadPath(
+    DownloadItem* item,
+    const FilePath& suggested_path,
+    const base::Callback<void(const FilePath&)>& file_selected_callback) {
   // Deletes itself.
   DownloadFilePicker* file_picker =
 #if defined(OS_CHROMEOS)
@@ -174,20 +173,22 @@ void ChromeDownloadManagerDelegate::ChooseDownloadPath(DownloadItem* item) {
 #else
       new DownloadFilePicker();
 #endif
-  file_picker->Init(download_manager_, item);
+  file_picker->Init(download_manager_, item, suggested_path,
+                    file_selected_callback);
 }
 
 FilePath ChromeDownloadManagerDelegate::GetIntermediatePath(
-    const DownloadItem& download) {
+    const FilePath& target_path,
+    content::DownloadDangerType danger_type) {
   // If the download is not dangerous, just append .crdownload to the target
   // path.
-  if (download.GetDangerType() == content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS)
-    return download_util::GetCrDownloadPath(download.GetTargetFilePath());
+  if (danger_type == content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS)
+    return download_util::GetCrDownloadPath(target_path);
 
   // If the download is potentially dangerous we create a filename of the form
   // 'Unconfirmed <random>.crdownload'.
   FilePath::StringType file_name;
-  FilePath dir = download.GetTargetFilePath().DirName();
+  FilePath dir = target_path.DirName();
 #if defined(OS_WIN)
   string16 unconfirmed_prefix =
       l10n_util::GetStringUTF16(IDS_DOWNLOAD_UNCONFIRMED_PREFIX);
@@ -655,8 +656,8 @@ void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone(
     // 1) using the default download directory.
     // 2) prompting the user.
     FilePath target_directory;
-    if (should_prompt && !download_manager_->LastDownloadPath().empty())
-      target_directory = download_manager_->LastDownloadPath();
+    if (should_prompt && !last_download_path_.empty())
+      target_directory = last_download_path_;
     else
       target_directory = download_prefs_->download_path();
     suggested_path = target_directory.Append(generated_name);
@@ -710,6 +711,7 @@ void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone(
 }
 
 #if defined (OS_CHROMEOS)
+// TODO(asanka): Merge this logic with the logic in DownloadFilePickerChromeOS.
 void ChromeDownloadManagerDelegate::SubstituteGDataDownloadPathCallback(
     int32 download_id,
     bool should_prompt,
@@ -734,22 +736,56 @@ void ChromeDownloadManagerDelegate::OnPathReservationAvailable(
     int32 download_id,
     bool should_prompt,
     content::DownloadDangerType danger_type,
-    const FilePath& target_path,
-    bool target_path_verified) {
+    const FilePath& reserved_path,
+    bool reserved_path_verified) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download =
       download_manager_->GetActiveDownloadItem(download_id);
   if (!download)
     return;
-  DownloadItem::TargetDisposition disposition;
-  // If the target path could not be verified then the path was non-existant,
-  // non writeable or could not be uniquified. Prompt the user.
-  if (should_prompt || !target_path_verified)
-    disposition = DownloadItem::TARGET_DISPOSITION_PROMPT;
-  else
-    disposition = DownloadItem::TARGET_DISPOSITION_OVERWRITE;
-  download->OnTargetPathDetermined(target_path, disposition, danger_type);
-  download_manager_->RestartDownload(download_id);
+  if (should_prompt || !reserved_path_verified) {
+    // If the target path could not be verified then the path was non-existant,
+    // non writeable or could not be uniquified. Prompt the user.
+    ChooseDownloadPath(
+        download, reserved_path,
+        base::Bind(&ChromeDownloadManagerDelegate::OnTargetPathAvailable,
+                   this, download_id, DownloadItem::TARGET_DISPOSITION_PROMPT,
+                   danger_type));
+  } else {
+    OnTargetPathAvailable(download_id,
+                          DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                          danger_type, reserved_path);
+  }
+}
+
+void ChromeDownloadManagerDelegate::OnTargetPathAvailable(
+    int32 download_id,
+    DownloadItem::TargetDisposition disposition,
+    content::DownloadDangerType danger_type,
+    const FilePath& target_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DownloadItem* download =
+      download_manager_->GetActiveDownloadItem(download_id);
+  if (!download)
+    return;
+
+  // If |target_path| is empty, then that means that the user wants to cancel
+  // the download.
+  if (target_path.empty()) {
+    DCHECK_EQ(DownloadItem::TARGET_DISPOSITION_PROMPT, disposition);
+    download->Cancel(true);
+    return;
+  }
+
+  // Retain the last directory. Exclude temporary downloads since the path
+  // likely points at the location of a temporary file.
+  if (disposition == DownloadItem::TARGET_DISPOSITION_PROMPT &&
+      !download->IsTemporary())
+    last_download_path_ = target_path.DirName();
+
+  FilePath intermediate_path = GetIntermediatePath(target_path, danger_type);
+  download->OnDownloadTargetDetermined(target_path, disposition, danger_type,
+                                       intermediate_path);
 }
 
 void ChromeDownloadManagerDelegate::OnItemAddedToPersistentStore(
@@ -763,3 +799,7 @@ void ChromeDownloadManagerDelegate::OnItemAddedToPersistentStore(
     db_handle = download_history_->GetNextFakeDbHandle();
   download_manager_->OnItemAddedToPersistentStore(download_id, db_handle);
 }
+
+void ChromeDownloadManagerDelegate::ClearTransientState() {
+  last_download_path_.clear();
+}