[Downloads] Always call DM::StartDownload() for explicit downloads.

chrome/browser/download/download_target_determiner.cc

 // Copyright 2013 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/download_target_determiner.h"
 
 #include "base/location.h"
 #include "base/rand_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/stringprintf.h"
@@ skipping 222 matching lines @@
   } else {
     virtual_path_ = download_->GetForcedFilePath();
     // If this is a resumed download which was previously interrupted due to an
     // issue with the forced path, the user is still not prompted. If the path
     // supplied to a programmatic download is invalid, then the caller needs to
     // intervene.
   }
   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;
 }
 
 DownloadTargetDeterminer::Result
     DownloadTargetDeterminer::DoNotifyExtensions() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!virtual_path_.empty());
 
   next_state_ = STATE_RESERVE_VIRTUAL_PATH;
 
-  if (!should_notify_extensions_)
+  if (!should_notify_extensions_ ||
+      download_->GetState() != DownloadItem::IN_PROGRESS)

[Randy Smith (Not in Mondays), 2016/02/12 21:57:19]

With apologies, but: Is this logic still relevant?  Will target determination
work reasonably in the absence of being able to detect
INTERRUPTED_TARGET_PENDING?

[asanka, 2016/02/12 23:34:47]

It will work, but the resulting target path reservation will be discarded when
the download is marked as interrupted at the end of the path determination
process.

[Randy Smith (Not in Mondays), 2016/02/13 00:16:31]

Acknowledged.

     return CONTINUE;
 
   delegate_->NotifyExtensions(download_, virtual_path_,
       base::Bind(&DownloadTargetDeterminer::NotifyExtensionsDone,
                  weak_ptr_factory_.GetWeakPtr()));
   return QUIT_DOLOOP;
 }
 
 void DownloadTargetDeterminer::NotifyExtensionsDone(
     const base::FilePath& suggested_path,
@@ skipping 27 matching lines @@
 
   DoLoop();
 }
 
 DownloadTargetDeterminer::Result
     DownloadTargetDeterminer::DoReserveVirtualPath() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   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_,
       base::Bind(&DownloadTargetDeterminer::ReserveVirtualPathDone,
                  weak_ptr_factory_.GetWeakPtr()));
   return QUIT_DOLOOP;
 }
 
 void DownloadTargetDeterminer::ReserveVirtualPathDone(
     const base::FilePath& path, bool verified) {
@@ skipping 16 matching lines @@
   DoLoop();
 }
 
 DownloadTargetDeterminer::Result
     DownloadTargetDeterminer::DoPromptUserForDownloadPath() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!virtual_path_.empty());
 
   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_,
         base::Bind(&DownloadTargetDeterminer::PromptUserForDownloadPathDone,
                    weak_ptr_factory_.GetWeakPtr()));
     return QUIT_DOLOOP;
   }
   return CONTINUE;
 }
 
@@ skipping 217 matching lines @@
   g_is_adobe_reader_up_to_date_ = adobe_reader_up_to_date;
   DoLoop();
 }
 #endif
 
 DownloadTargetDeterminer::Result
     DownloadTargetDeterminer::DoCheckDownloadUrl() {
   DCHECK_CURRENTLY_ON(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_,
       base::Bind(&DownloadTargetDeterminer::CheckDownloadUrlDone,
                  weak_ptr_factory_.GetWeakPtr()));
   return QUIT_DOLOOP;
 }
 
 void DownloadTargetDeterminer::CheckDownloadUrlDone(
     content::DownloadDangerType danger_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DVLOG(20) << "URL Check Result:" << danger_type;
   DCHECK_EQ(STATE_CHECK_VISITED_REFERRER_BEFORE, next_state_);
   danger_type_ = danger_type;
   DoLoop();
 }
 
 DownloadTargetDeterminer::Result
     DownloadTargetDeterminer::DoCheckVisitedReferrerBefore() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   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.
   if (danger_type_ != content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS &&
       danger_type_ != content::DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT)
     return CONTINUE;
 
   // First determine the danger level assuming that the user doesn't have any
   // prior visits to the referrer recoreded in history. The resulting danger
   // level would be ALLOW_ON_USER_GESTURE if the level depends on the visit
   // history. In the latter case, we can query the history DB to determine if
@@ skipping 305 matching lines @@
     const base::FilePath& suggested_path) {
   return base::FilePath(suggested_path.value() + kCrdownloadSuffix);
 }
 
 #if defined(OS_WIN)
 // static
 bool DownloadTargetDeterminer::IsAdobeReaderUpToDate() {
   return g_is_adobe_reader_up_to_date_;
 }
 #endif