Download filename determination refactor (3/3)

chrome/browser/download/chrome_download_manager_delegate.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 117 matching lines @@
 }
 
 DownloadId ChromeDownloadManagerDelegate::GetNextId() {
   if (!profile_->IsOffTheRecord())
     return DownloadId(this, next_download_id_++);
 
   return BrowserContext::GetDownloadManager(profile_->GetOriginalProfile())->
       GetDelegate()->GetNextId();
 }
 
-bool ChromeDownloadManagerDelegate::ShouldStartDownload(int32 download_id) {
-  // 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
-  // insert the new DownloadItem into 'history_downloads_' or inform our
-  // 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;
-
+bool ChromeDownloadManagerDelegate::DetermineDownloadTarget(
+    DownloadItem* download,
+    const content::DownloadTargetCallback& callback) {
 #if defined(ENABLE_SAFE_BROWSING)
   DownloadProtectionService* service = GetDownloadProtectionService();
   if (service) {
     VLOG(2) << __FUNCTION__ << "() Start SB URL check for download = "
             << download->DebugString(false);
     service->CheckDownloadUrl(
         DownloadProtectionService::DownloadInfo::FromDownloadItem(*download),
         base::Bind(
             &ChromeDownloadManagerDelegate::CheckDownloadUrlDone,
             this,
-            download->GetId()));
-    return false;
+            download->GetId(),
+            callback));
+    return true;
   }
 #endif
-  CheckDownloadUrlDone(download_id, DownloadProtectionService::SAFE);
-  return false;
+  CheckDownloadUrlDone(download->GetId(), callback,
+                       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)
       new DownloadFilePickerChromeOS();
 #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);
 #else
   std::string unconfirmed_prefix =
       l10n_util::GetStringUTF8(IDS_DOWNLOAD_UNCONFIRMED_PREFIX);
 #endif
   base::SStringPrintf(
       &file_name,
       unconfirmed_prefix.append(
@@ skipping 272 matching lines @@
     const content::SavePackagePathPickedCallback& callback) {
   // Deletes itself.
 #if defined(OS_CHROMEOS)
   new SavePackageFilePickerChromeOS(web_contents, suggested_path, callback);
 #else
   new SavePackageFilePicker(web_contents, suggested_path, default_extension,
       can_save_as_complete, download_prefs_.get(), callback);
 #endif
 }
 
+void ChromeDownloadManagerDelegate::ClearLastDownloadPath() {
+  last_download_path_.clear();
+}
+
 DownloadProtectionService*
     ChromeDownloadManagerDelegate::GetDownloadProtectionService() {
 #if defined(ENABLE_SAFE_BROWSING)
   SafeBrowsingService* sb_service = g_browser_process->safe_browsing_service();
   if (sb_service && sb_service->download_protection_service() &&
       profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
     return sb_service->download_protection_service();
   }
 #endif
   return NULL;
@@ skipping 32 matching lines @@
     return !download.HasUserGesture() || !visited_referrer_before;
 
   return danger_level == download_util::Dangerous;
 }
 
 void ChromeDownloadManagerDelegate::GetReservedPath(
     DownloadItem& download,
     const FilePath& target_path,
     const FilePath& default_download_path,
     bool should_uniquify_path,
-    const DownloadPathReservationTracker::ReservedPathCallback callback) {
+    const DownloadPathReservationTracker::ReservedPathCallback& callback) {
   DownloadPathReservationTracker::GetReservedPath(
       download, target_path, default_download_path, should_uniquify_path,
       callback);
 }
 
 void ChromeDownloadManagerDelegate::CheckDownloadUrlDone(
     int32 download_id,
+    const content::DownloadTargetCallback& callback,
     DownloadProtectionService::DownloadCheckResult result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download =
       download_manager_->GetActiveDownloadItem(download_id);
   if (!download)
     return;
 
   VLOG(2) << __FUNCTION__ << "() download = " << download->DebugString(false)
           << " verdict = " << result;
   content::DownloadDangerType danger_type = download->GetDangerType();
   if (result != DownloadProtectionService::SAFE)
     danger_type = content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL;
 
   download_history_->CheckVisitedReferrerBefore(
       download_id, download->GetReferrerUrl(),
       base::Bind(&ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone,
-                 base::Unretained(this), download_id, danger_type));
+                 base::Unretained(this), download_id, callback, danger_type));
 }
 
 void ChromeDownloadManagerDelegate::CheckClientDownloadDone(
     int32 download_id,
     DownloadProtectionService::DownloadCheckResult result) {
   DownloadItem* item = download_manager_->GetActiveDownloadItem(download_id);
   if (!item)
     return;
 
   VLOG(2) << __FUNCTION__ << "() download = " << item->DebugString(false)
@@ skipping 37 matching lines @@
   int download_id = crx_installers_[installer];
   crx_installers_.erase(installer.get());
 
   DownloadItem* item = download_manager_->GetActiveDownloadItem(download_id);
   if (item)
     item->DelayedDownloadOpened(installer->did_handle_successfully());
 }
 
 void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone(
     int32 download_id,
+    const content::DownloadTargetCallback& callback,
     content::DownloadDangerType danger_type,
     bool visited_referrer_before) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DownloadItem* download =
       download_manager_->GetActiveDownloadItem(download_id);
   if (!download)
     return;
 
   bool should_prompt = (download->GetTargetDisposition() ==
@@ skipping 21 matching lines @@
           !ShouldOpenFileBasedOnExtension(generated_name))
         should_prompt = true;
     }
     if (download_prefs_->IsDownloadPathManaged())
       should_prompt = false;
 
     // Determine the proper path for a download, by either one of the following:
     // 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);
   } else {
     DCHECK(!should_prompt);
     suggested_path = download->GetForcedFilePath();
   }
 
   // If the download hasn't already been marked dangerous (could be
   // DANGEROUS_URL), check if it is a dangerous file.
@@ skipping 22 matching lines @@
   } else {
     // Currently we only expect this case.
     DCHECK_EQ(content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL, danger_type);
   }
 
 #if defined (OS_CHROMEOS)
   gdata::GDataDownloadObserver::SubstituteGDataDownloadPath(
       profile_, suggested_path, download,
       base::Bind(
           &ChromeDownloadManagerDelegate::SubstituteGDataDownloadPathCallback,
-          this, download->GetId(), should_prompt, is_forced_path, danger_type));
+          this, download->GetId(), callback, should_prompt, is_forced_path,
+          danger_type));
 #else
   GetReservedPath(
       *download, suggested_path, download_prefs_->download_path(),
       !is_forced_path,
       base::Bind(&ChromeDownloadManagerDelegate::OnPathReservationAvailable,
-                 this, download->GetId(), should_prompt, danger_type));
+                 this, download->GetId(), callback, should_prompt,
+                 danger_type));
 #endif
 }
 
 #if defined (OS_CHROMEOS)
+// TODO(asanka): Merge this logic with the logic in DownloadFilePickerChromeOS.

[Randy Smith (Not in Mondays), 2012/07/12 17:01:22]

Probably worth pulling Achuith into the review at some point, to keep him
updated and in case he feels like taking this one.

[asanka, 2012/07/18 21:50:56]

Yeah. Hopefully we can get it down to a single GData path substitution instead
of two for a single download. The code change seems non-trivial so I wanted to
do it (or someone else should) on a separate CL.

 void ChromeDownloadManagerDelegate::SubstituteGDataDownloadPathCallback(
     int32 download_id,
+    const content::DownloadTargetCallback& callback,
     bool should_prompt,
     bool is_forced_path,
     content::DownloadDangerType danger_type,
     const FilePath& suggested_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download =
       download_manager_->GetActiveDownloadItem(download_id);
   if (!download)
     return;
 
   GetReservedPath(
       *download, suggested_path, download_prefs_->download_path(),
       !is_forced_path,
       base::Bind(&ChromeDownloadManagerDelegate::OnPathReservationAvailable,
-                 this, download->GetId(), should_prompt, danger_type));
+                 this, download->GetId(), callback, should_prompt,
+                 danger_type));
 }
 #endif
 
 void ChromeDownloadManagerDelegate::OnPathReservationAvailable(
     int32 download_id,
+    const content::DownloadTargetCallback& callback,
     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::OnTargetPathDetermined,
+                   this, download_id, callback,
+                   DownloadItem::TARGET_DISPOSITION_PROMPT, danger_type));
+  } else {
+    OnTargetPathDetermined(download_id, callback,
+                           DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                           danger_type, reserved_path);
+  }
+}
+
+void ChromeDownloadManagerDelegate::OnTargetPathDetermined(
+    int32 download_id,
+    const content::DownloadTargetCallback& callback,
+    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.
+  // TODO(asanka): This logic is a hack. DownloadFilePicker should give us a
+  //               directory to persist. Or perhaps, if the GData path
+  //               substitution logic is moved here, then we would have a
+  //               persistable path after the DownloadFilePicker is done.

[Randy Smith (Not in Mondays), 2012/07/12 17:01:22]

This is only for GData, right?  It should go away after we do the
DownloadDetermination refactor.

[asanka, 2012/07/18 21:50:56]

Yeah. I didn't want to do it here since such a fix would become redundant once
the GData path substitution is refactored.

+  if (disposition == DownloadItem::TARGET_DISPOSITION_PROMPT &&
+      !download->IsTemporary())
+    last_download_path_ = target_path.DirName();
+
+  FilePath intermediate_path = GetIntermediatePath(target_path, danger_type);
+  callback.Run(target_path, disposition, danger_type, intermediate_path);
 }
 
 void ChromeDownloadManagerDelegate::OnItemAddedToPersistentStore(
     int32 download_id, int64 db_handle) {
   // It's not immediately obvious, but HistoryBackend::CreateDownload() can
   // call this function with an invalid |db_handle|. For instance, this can
   // happen when the history database is offline. We cannot have multiple
   // DownloadItems with the same invalid db_handle, so we need to assign a
   // unique |db_handle| here.
   if (db_handle == DownloadItem::kUninitializedHandle)
     db_handle = download_history_->GetNextFakeDbHandle();
   download_manager_->OnItemAddedToPersistentStore(download_id, db_handle);
 }