Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2205)

Unified Diff: chrome/browser/download/download_target_determiner.cc

Issue 148133007: [Downloads] Always call DM::StartDownload() for explicit downloads. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/download/download_target_determiner.cc
diff --git a/chrome/browser/download/download_target_determiner.cc b/chrome/browser/download/download_target_determiner.cc
index 0ddb3a077c00370e4db7cd8216372274aaf9db4c..bc19c67f449a19299a2811fc1147b7ca408687a6 100644
--- a/chrome/browser/download/download_target_determiner.cc
+++ b/chrome/browser/download/download_target_determiner.cc
@@ -207,15 +207,6 @@ DownloadTargetDeterminer::Result
DCHECK(virtual_path_.IsAbsolute());
DVLOG(20) << "Generated virtual path: " << virtual_path_.AsUTF8Unsafe();
- // If the download is DOA, don't bother going any further. This would be the
- // case for a download that failed to initialize (e.g. the initial temporary
- // file couldn't be created because both the downloads directory and the
- // temporary directory are unwriteable).
- //
- // A virtual path is determined for DOA downloads for display purposes. This
- // is why this check is performed here instead of at the start.
- if (download_->GetState() != DownloadItem::IN_PROGRESS)
- return COMPLETE;
return CONTINUE;
}
@@ -226,7 +217,8 @@ DownloadTargetDeterminer::Result
next_state_ = STATE_RESERVE_VIRTUAL_PATH;
- if (!should_notify_extensions_)
+ if (!should_notify_extensions_ ||
+ download_->GetState() != DownloadItem::IN_PROGRESS)
return CONTINUE;
delegate_->NotifyExtensions(download_, virtual_path_,
@@ -269,6 +261,8 @@ DownloadTargetDeterminer::Result
DCHECK(!virtual_path_.empty());
next_state_ = STATE_PROMPT_USER_FOR_DOWNLOAD_PATH;
+ if (download_->GetState() != DownloadItem::IN_PROGRESS)
+ return CONTINUE;
delegate_->ReserveVirtualPath(
download_, virtual_path_, create_target_directory_, conflict_action_,
@@ -294,7 +288,9 @@ DownloadTargetDeterminer::Result
next_state_ = STATE_DETERMINE_LOCAL_PATH;
- if (should_prompt_) {
+ // Avoid prompting for a download if it isn't in-progress. The user will be
+ // prompted once the download is resumed and headers are available.
+ if (should_prompt_ && download_->GetState() == DownloadItem::IN_PROGRESS) {
delegate_->PromptUserForDownloadPath(
download_,
virtual_path_,
@@ -485,6 +481,17 @@ DownloadTargetDeterminer::Result
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK(!virtual_path_.empty());
next_state_ = STATE_CHECK_VISITED_REFERRER_BEFORE;
+
+ // Downloads that are already interrupted aren't checked for malicious URLs.
+ // Such downloads were unsuccessful at initiation and there should be no data
+ // saved at this point for the user to discard. If the download is
+ // subsequently resumed, DownloadTargetDeterminer would be invoked again and
+ // the URL check would be performed if the resumption was successful.
+ if (download_->GetState() != DownloadItem::IN_PROGRESS) {
+ DCHECK(download_->GetFullPath().empty());
+ return CONTINUE;
+ }
+
delegate_->CheckDownloadUrl(
download_,
virtual_path_,
@@ -507,6 +514,15 @@ DownloadTargetDeterminer::Result
next_state_ = STATE_DETERMINE_INTERMEDIATE_PATH;
+ // If the download is already interrupted, there should be no download file
+ // on-disk. So skip the danger type determination since there's no file for
+ // the user to discard. Also the absence of a on-disk file prevents a filename
+ // based attack (e.g. creating a foo.exe.local file). If the download is
+ // successfully resumed, then DownloadTargetDeterminer would be invoked again
+ // at which point the danger level would be determined again.
+ if (download_->GetState() != DownloadItem::IN_PROGRESS)
+ return CONTINUE;
+
// Checking if there are prior visits to the referrer is only necessary if the
// danger level of the download depends on the file type. This excludes cases
// where the download has already been deemed dangerous, or where the user is

Powered by Google App Engine
This is Rietveld 408576698