[downloads] Move platform specific code out of DownloadTargetDeterminer.

content/browser/download/download_item_impl_unittest.cc

Index: content/browser/download/download_item_impl_unittest.cc
diff --git a/content/browser/download/download_item_impl_unittest.cc b/content/browser/download/download_item_impl_unittest.cc
index a81c744202bee5a725b1b71e0e48153cc862d7d0..9e0ceae69fdd0404d77c1a387dfd6de51fafcfb6 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -6,6 +6,7 @@
 
 #include <stdint.h>
 
+#include <deque>
 #include <iterator>
 #include <map>
 #include <memory>
@@ -13,6 +14,7 @@
 #include <utility>
 
 #include "base/callback.h"
+#include "base/callback_helpers.h"
 #include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
@@ -38,6 +40,8 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::DoAll;
+using ::testing::Invoke;
+using ::testing::InvokeWithoutArgs;
 using ::testing::NiceMock;
 using ::testing::Property;
 using ::testing::Return;
@@ -285,19 +289,14 @@ class DownloadItemTest : public testing::Test {
     return download;
   }
 
-  // Add DownloadFile to DownloadItem. Set |callback| to nullptr if a download
-  // target determination is not expected.
+  // Add DownloadFile to DownloadItem.
   MockDownloadFile* CallDownloadItemStart(
       DownloadItemImpl* item,
       DownloadItemImplDelegate::DownloadTargetCallback* callback) {
     MockDownloadFile* mock_download_file = nullptr;
     std::unique_ptr<DownloadFile> download_file;
-    if (callback) {
-      EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
-          .WillOnce(SaveArg<1>(callback));
-    } else {
-      EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _)).Times(0);
-    }
+    EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
+        .WillOnce(SaveArg<1>(callback));
 
     // Only create a DownloadFile if the request was successful.
     if (create_info_->result == DOWNLOAD_INTERRUPT_REASON_NONE) {
@@ -309,8 +308,8 @@ class DownloadItemTest : public testing::Test {
           .WillRepeatedly(ReturnRefOfCopy(base::FilePath()));
     }
 
-    std::unique_ptr<MockRequestHandle> request_handle(
-        new NiceMock<MockRequestHandle>);
+    std::unique_ptr<MockRequestHandle> request_handle =
+        base::MakeUnique<NiceMock<MockRequestHandle>>();
     item->Start(std::move(download_file), std::move(request_handle),
                 *create_info_);
     RunAllPendingInMessageLoops();
@@ -343,7 +342,8 @@ class DownloadItemTest : public testing::Test {
         .WillOnce(ScheduleRenameAndUniquifyCallback(
             DOWNLOAD_INTERRUPT_REASON_NONE, intermediate_path));
     callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-                 danger_type, intermediate_path);
+                 danger_type, intermediate_path,
+                 DOWNLOAD_INTERRUPT_REASON_NONE);
     RunAllPendingInMessageLoops();
     return download_file;
   }
@@ -476,8 +476,7 @@ TEST_F(DownloadItemTest, NotificationAfterInterrupted) {
   EXPECT_CALL(*download_file, Cancel());
   TestDownloadItemObserver observer(item);
 
-  EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_,_))
-      .Times(0);
+  EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_, _)).Times(0);
 
   item->DestinationObserverAsWeakPtr()->DestinationError(
       DOWNLOAD_INTERRUPT_REASON_FILE_FAILED, 0,
@@ -577,7 +576,8 @@ TEST_F(DownloadItemTest, NotificationAfterOnDownloadTargetDetermined) {
   // Currently, a notification would be generated if the danger type is anything
   // other than NOT_DANGEROUS.
   callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
+               DOWNLOAD_INTERRUPT_REASON_NONE);
   EXPECT_FALSE(observer.CheckAndResetDownloadUpdated());
   RunAllPendingInMessageLoops();
   EXPECT_TRUE(observer.CheckAndResetDownloadUpdated());
@@ -613,19 +613,32 @@ TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
 }
 
 // Test that a download is resumed automatcially after a continuable interrupt.
-TEST_F(DownloadItemTest, ContinueAfterInterrupted) {
+TEST_F(DownloadItemTest, AutomaticResumption_Continue) {
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
   MockDownloadFile* download_file =
       DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
 
-  // Interrupt the download, using a continuable interrupt.
+  // Interrupt the download using a continuable interrupt after writing a single
+  // byte. An intermediate file with data shouldn't be discarding after a
+  // continuable interrupt.
+
+  // The DownloadFile should be detached without discarding.
   EXPECT_CALL(*download_file, FullPath())
       .WillOnce(ReturnRefOfCopy(base::FilePath()));
   EXPECT_CALL(*download_file, Detach());
-  EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_, _)).Times(1);
+
+  // Resumption attempt should pass the intermediate file along.
+  EXPECT_CALL(*mock_delegate(),
+              MockResumeInterruptedDownload(
+                  AllOf(Property(&DownloadUrlParameters::file_path,
+                                 Property(&base::FilePath::value,
+                                          kDummyIntermediatePath)),
+                        Property(&DownloadUrlParameters::offset, 1)),
+                  _));
+
   item->DestinationObserverAsWeakPtr()->DestinationError(
-      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 0,
+      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 1,
       std::unique_ptr<crypto::SecureHash>());
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
   // Since the download is resumed automatically, the interrupt count doesn't
@@ -644,9 +657,9 @@ TEST_F(DownloadItemTest, ContinueAfterInterrupted) {
   CleanupItem(item, nullptr, DownloadItem::IN_PROGRESS);
 }
 
-// Test that automatic resumption doesn't happen after a non-continuable
-// interrupt.
-TEST_F(DownloadItemTest, RestartAfterInterrupted) {
+// Automatic resumption should restart and discard the intermediate file if the
+// interrupt reason requires it.
+TEST_F(DownloadItemTest, AutomaticResumption_Restart) {
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
   MockDownloadFile* download_file =
@@ -654,8 +667,40 @@ TEST_F(DownloadItemTest, RestartAfterInterrupted) {
 
   // Interrupt the download, using a restartable interrupt.
   EXPECT_CALL(*download_file, Cancel());
+  EXPECT_EQ(kDummyIntermediatePath, item->GetFullPath().value());
+
+  // Resumption attempt should have discarded intermediate file.
+  EXPECT_CALL(*mock_delegate(),
+              MockResumeInterruptedDownload(
+                  Property(&DownloadUrlParameters::file_path,
+                           Property(&base::FilePath::empty, true)),
+                  _));
+
   item->DestinationObserverAsWeakPtr()->DestinationError(
-      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED, 0,
+      DOWNLOAD_INTERRUPT_REASON_SERVER_NO_RANGE, 1,
+      std::unique_ptr<crypto::SecureHash>());
+  ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
+
+  // Since the download is resumed automatically, the interrupt count doesn't
+  // increase.
+  ASSERT_EQ(0, observer.interrupt_count());
+  RunAllPendingInMessageLoops();
+
+  CleanupItem(item, nullptr, DownloadItem::IN_PROGRESS);
+}
+
+// Test that automatic resumption doesn't happen after an interrupt that
+// requires user action to resolve.
+TEST_F(DownloadItemTest, AutomaticResumption_NeedsUserAction) {
+  DownloadItemImpl* item = CreateDownloadItem();
+  TestDownloadItemObserver observer(item);
+  MockDownloadFile* download_file =
+      DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
+
+  // Interrupt the download, using a restartable interrupt.
+  EXPECT_CALL(*download_file, Cancel());
+  item->DestinationObserverAsWeakPtr()->DestinationError(
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED, 1,
       std::unique_ptr<crypto::SecureHash>());
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
   // Should not try to auto-resume.
@@ -695,59 +740,72 @@ TEST_F(DownloadItemTest, UnresumableInterrupt) {
   CleanupItem(item, nullptr, DownloadItem::INTERRUPTED);
 }
 
-TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
+TEST_F(DownloadItemTest, AutomaticResumption_AttemptLimit) {
   DownloadItemImpl* item = CreateDownloadItem();
   base::WeakPtr<DownloadDestinationObserver> as_observer(
       item->DestinationObserverAsWeakPtr());
   TestDownloadItemObserver observer(item);
-  MockDownloadFile* mock_download_file(nullptr);
-  std::unique_ptr<DownloadFile> download_file;
-  MockRequestHandle* mock_request_handle(nullptr);
-  std::unique_ptr<DownloadRequestHandleInterface> request_handle;
+  MockDownloadFile* mock_download_file_ref = nullptr;
+  std::unique_ptr<MockDownloadFile> mock_download_file;
+  std::unique_ptr<MockRequestHandle> mock_request_handle;
   DownloadItemImplDelegate::DownloadTargetCallback callback;
 
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
       .WillRepeatedly(SaveArg<1>(&callback));
-  EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_, _))
+
+  // All attempts at resumption should pass along the intermediate file.
+  EXPECT_CALL(*mock_delegate(),
+              MockResumeInterruptedDownload(
+                  AllOf(Property(&DownloadUrlParameters::file_path,
+                                 Property(&base::FilePath::value,
+                                          kDummyIntermediatePath)),
+                        Property(&DownloadUrlParameters::offset, 1)),
+                  _))
       .Times(DownloadItemImpl::kMaxAutoResumeAttempts);
   for (int i = 0; i < (DownloadItemImpl::kMaxAutoResumeAttempts + 1); ++i) {
     SCOPED_TRACE(::testing::Message() << "Iteration " << i);
 
-    mock_download_file = new NiceMock<MockDownloadFile>;
-    download_file.reset(mock_download_file);
-    mock_request_handle = new NiceMock<MockRequestHandle>;
-    request_handle.reset(mock_request_handle);
+    mock_download_file = base::MakeUnique<NiceMock<MockDownloadFile>>();
+    mock_download_file_ref = mock_download_file.get();
+    mock_request_handle = base::MakeUnique<NiceMock<MockRequestHandle>>();
 
-    ON_CALL(*mock_download_file, FullPath())
+    ON_CALL(*mock_download_file_ref, FullPath())
         .WillByDefault(ReturnRefOfCopy(base::FilePath()));
 
     // Copied key parts of DoIntermediateRename & CallDownloadItemStart
     // to allow for holding onto the request handle.
-    item->Start(std::move(download_file), std::move(request_handle),
+    item->Start(std::move(mock_download_file), std::move(mock_request_handle),
                 *create_info());
     RunAllPendingInMessageLoops();
 
     base::FilePath target_path(kDummyTargetPath);
     base::FilePath intermediate_path(kDummyIntermediatePath);
-    if (i == 0) {
-      // RenameAndUniquify is only called the first time. In all the subsequent
-      // iterations, the intermediate file already has the correct name, hence
-      // no rename is necessary.
-      EXPECT_CALL(*mock_download_file, RenameAndUniquify(intermediate_path, _))
-          .WillOnce(ScheduleRenameAndUniquifyCallback(
-              DOWNLOAD_INTERRUPT_REASON_NONE, intermediate_path));
-    }
-    callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-                 DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+
+    // Target of RenameAndUniquify is always the intermediate path.
+    ON_CALL(*mock_download_file_ref, RenameAndUniquify(_, _))
+        .WillByDefault(ScheduleRenameAndUniquifyCallback(
+            DOWNLOAD_INTERRUPT_REASON_NONE, intermediate_path));
+
+    // RenameAndUniquify is only called the first time. In all the subsequent
+    // iterations, the intermediate file already has the correct name, hence no
+    // rename is necessary.
+    EXPECT_CALL(*mock_download_file_ref, RenameAndUniquify(_, _)).Times(i == 0);
+
+    ASSERT_FALSE(callback.is_null());
+    base::ResetAndReturn(&callback).Run(
+        target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+        DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
+        DOWNLOAD_INTERRUPT_REASON_NONE);
     RunAllPendingInMessageLoops();
 
     // Use a continuable interrupt.
+    EXPECT_CALL(*mock_download_file_ref, Cancel()).Times(0);
     item->DestinationObserverAsWeakPtr()->DestinationError(
-        DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 0,
+        DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 1,
         std::unique_ptr<crypto::SecureHash>());
 
     RunAllPendingInMessageLoops();
-    ::testing::Mock::VerifyAndClearExpectations(mock_download_file);
+    ::testing::Mock::VerifyAndClearExpectations(mock_download_file_ref);
   }
 
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
@@ -779,10 +837,16 @@ TEST_F(DownloadItemTest, FailedResumptionDoesntUpdateOriginState) {
   EXPECT_EQ(kFirstURL, item->GetURL().spec());
   EXPECT_EQ(kMimeType, item->GetMimeType());
 
-  EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_, _));
+  EXPECT_CALL(*mock_delegate(),
+              MockResumeInterruptedDownload(
+                  AllOf(Property(&DownloadUrlParameters::file_path,
+                                 Property(&base::FilePath::value,
+                                          kDummyIntermediatePath)),
+                        Property(&DownloadUrlParameters::offset, 1)),
+                  _));
   EXPECT_CALL(*download_file, Detach());
   item->DestinationObserverAsWeakPtr()->DestinationError(
-      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 0,
+      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 1,
       std::unique_ptr<crypto::SecureHash>());
   RunAllPendingInMessageLoops();
   EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
@@ -801,10 +865,20 @@ TEST_F(DownloadItemTest, FailedResumptionDoesntUpdateOriginState) {
   create_info()->url_chain.push_back(GURL(kSecondURL));
   create_info()->mime_type = kSecondMimeType;
   create_info()->result = DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED;
+  create_info()->save_info->file_path = base::FilePath(kDummyIntermediatePath);
+  create_info()->save_info->offset = 1;
 
-  // The following ends up calling DownloadItem::Start(), but shouldn't result
-  // in an origin update.
-  download_file = CallDownloadItemStart(item, nullptr);
+  // Calling Start() with a response indicating failure shouldn't cause a target
+  // update, nor should it result in discarding the intermediate file.
+  DownloadItemImplDelegate::DownloadTargetCallback target_callback;
+  download_file = CallDownloadItemStart(item, &target_callback);
+  ASSERT_FALSE(target_callback.is_null());
+  target_callback.Run(base::FilePath(kDummyTargetPath),
+                      DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                      DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+                      base::FilePath(kDummyIntermediatePath),
+                      DOWNLOAD_INTERRUPT_REASON_NONE);
+  RunAllPendingInMessageLoops();
 
   EXPECT_EQ(kContentDisposition, item->GetContentDisposition());
   EXPECT_EQ(kFirstETag, item->GetETag());
@@ -813,6 +887,8 @@ TEST_F(DownloadItemTest, FailedResumptionDoesntUpdateOriginState) {
   EXPECT_EQ(kMimeType, item->GetMimeType());
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, item->GetLastReason());
+  EXPECT_EQ(kDummyIntermediatePath, item->GetFullPath().value());
+  EXPECT_EQ(1, item->GetReceivedBytes());
 }
 
 // If the download resumption request succeeds, the origin state should be
@@ -867,7 +943,7 @@ TEST_F(DownloadItemTest, SucceededResumptionUpdatesOriginState) {
 }
 
 // Test that resumption uses the final URL in a URL chain when resuming.
-TEST_F(DownloadItemTest, ResumeUsingFinalURL) {
+TEST_F(DownloadItemTest, ResumeUsesFinalURL) {
   create_info()->save_info->prompt_for_save_location = false;
   create_info()->url_chain.clear();
   create_info()->url_chain.push_back(GURL("http://example.com/a"));
@@ -889,7 +965,7 @@ TEST_F(DownloadItemTest, ResumeUsingFinalURL) {
                                     _))
       .Times(1);
   item->DestinationObserverAsWeakPtr()->DestinationError(
-      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 0,
+      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 1,
       std::unique_ptr<crypto::SecureHash>());
 
   // Test expectations verify that ResumeInterruptedDownload() is called (by way
@@ -916,7 +992,8 @@ TEST_F(DownloadItemTest, DisplayName) {
       .WillOnce(ScheduleRenameAndUniquifyCallback(
           DOWNLOAD_INTERRUPT_REASON_NONE, intermediate_path));
   callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
+               DOWNLOAD_INTERRUPT_REASON_NONE);
   RunAllPendingInMessageLoops();
   EXPECT_EQ(FILE_PATH_LITERAL("foo.bar"),
             item->GetFileNameToReportUser().value());
@@ -946,8 +1023,10 @@ TEST_F(DownloadItemTest, Start) {
 // file initialization failing.
 TEST_F(DownloadItemTest, InitDownloadFileFails) {
   DownloadItemImpl* item = CreateDownloadItem();
-  std::unique_ptr<MockDownloadFile> file(new MockDownloadFile());
-  std::unique_ptr<MockRequestHandle> request_handle(new MockRequestHandle());
+  std::unique_ptr<MockDownloadFile> file = base::MakeUnique<MockDownloadFile>();
+  std::unique_ptr<MockRequestHandle> request_handle =
+      base::MakeUnique<MockRequestHandle>();
+
   EXPECT_CALL(*file, Cancel());
   EXPECT_CALL(*request_handle, CancelRequest());
   EXPECT_CALL(*file, Initialize(_))
@@ -966,12 +1045,15 @@ TEST_F(DownloadItemTest, InitDownloadFileFails) {
   download_target_callback.Run(base::FilePath(kDummyTargetPath),
                                DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                                DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
-                               base::FilePath(kDummyIntermediatePath));
+                               base::FilePath(kDummyIntermediatePath),
+                               DOWNLOAD_INTERRUPT_REASON_NONE);
   RunAllPendingInMessageLoops();
 
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED,
             item->GetLastReason());
+  EXPECT_FALSE(item->GetTargetFilePath().empty());
+  EXPECT_TRUE(item->GetFullPath().empty());
 }
 
 // Handling of downloads initiated via a failed request. In this case, Start()
@@ -999,7 +1081,8 @@ TEST_F(DownloadItemTest, StartFailedDownload) {
   base::FilePath target_path(FILE_PATH_LITERAL("foo"));
   download_target_callback.Run(target_path,
                                DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-                               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, target_path);
+                               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, target_path,
+                               DOWNLOAD_INTERRUPT_REASON_NONE);
   RunAllPendingInMessageLoops();
 
   EXPECT_EQ(target_path, item->GetTargetFilePath());
@@ -1021,7 +1104,8 @@ TEST_F(DownloadItemTest, CallbackAfterRename) {
           DOWNLOAD_INTERRUPT_REASON_NONE, new_intermediate_path));
 
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
+               DOWNLOAD_INTERRUPT_REASON_NONE);
   RunAllPendingInMessageLoops();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
@@ -1060,7 +1144,8 @@ TEST_F(DownloadItemTest, CallbackAfterInterruptedRename) {
       .Times(1);
 
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
+               DOWNLOAD_INTERRUPT_REASON_NONE);
   RunAllPendingInMessageLoops();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
@@ -1112,7 +1197,8 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Restart) {
       .Times(1);
 
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
+               DOWNLOAD_INTERRUPT_REASON_NONE);
   RunAllPendingInMessageLoops();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
@@ -1129,8 +1215,11 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Continue) {
   DownloadItemImpl* item = CreateDownloadItem();
   DownloadItemImplDelegate::DownloadTargetCallback callback;
   MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
+
+  // Write some data and interrupt with NETWORK_FAILED. The download shouldn't
+  // transition to INTERRUPTED until the destination callback has been invoked.
   item->DestinationObserverAsWeakPtr()->DestinationError(
-      DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, 0,
+      DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, 1,
       std::unique_ptr<crypto::SecureHash>());
   ASSERT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
 
@@ -1147,7 +1236,8 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Continue) {
   EXPECT_CALL(*download_file, Detach());
 
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
+               DOWNLOAD_INTERRUPT_REASON_NONE);
   RunAllPendingInMessageLoops();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
@@ -1180,7 +1270,8 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Failed) {
       .Times(1);
 
   callback.Run(final_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path,
+               DOWNLOAD_INTERRUPT_REASON_NONE);
   RunAllPendingInMessageLoops();
   // All the callbacks should have happened by now.
   ::testing::Mock::VerifyAndClearExpectations(download_file);
@@ -1203,6 +1294,32 @@ TEST_F(DownloadItemTest, Canceled) {
   EXPECT_EQ(DownloadItem::CANCELLED, item->GetState());
 }
 
+TEST_F(DownloadItemTest, DownloadTargetDetermined_Cancel) {
+  DownloadItemImpl* item = CreateDownloadItem();
+  DownloadItemImplDelegate::DownloadTargetCallback callback;
+  MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
+
+  EXPECT_CALL(*download_file, Cancel());
+  callback.Run(base::FilePath(FILE_PATH_LITERAL("foo")),
+               DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+               base::FilePath(FILE_PATH_LITERAL("bar")),
+               DOWNLOAD_INTERRUPT_REASON_USER_CANCELED);
+  EXPECT_EQ(DownloadItem::CANCELLED, item->GetState());
+}
+
+TEST_F(DownloadItemTest, DownloadTargetDetermined_CancelWithEmptyName) {
+  DownloadItemImpl* item = CreateDownloadItem();
+  DownloadItemImplDelegate::DownloadTargetCallback callback;
+  MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
+
+  EXPECT_CALL(*download_file, Cancel());
+  callback.Run(base::FilePath(), DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, base::FilePath(),
+               DOWNLOAD_INTERRUPT_REASON_NONE);
+  EXPECT_EQ(DownloadItem::CANCELLED, item->GetState());
+}
+
 TEST_F(DownloadItemTest, FileRemoved) {
   DownloadItemImpl* item = CreateDownloadItem();
 
@@ -1263,8 +1380,8 @@ TEST_F(DownloadItemTest, DestinationError_NoRestartRequired) {
   hash->Update(kTestData1, sizeof(kTestData1));
 
   EXPECT_CALL(*download_file, Detach());
-  as_observer->DestinationError(
-      DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, 0, std::move(hash));
+  as_observer->DestinationError(DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, 1,
+                                std::move(hash));
   mock_delegate()->VerifyAndClearExpectations();
   EXPECT_TRUE(observer.CheckAndResetDownloadUpdated());
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
@@ -1273,6 +1390,7 @@ TEST_F(DownloadItemTest, DestinationError_NoRestartRequired) {
       std::string(std::begin(kHashOfTestData1), std::end(kHashOfTestData1)),
       item->GetHash());
 }
+
 TEST_F(DownloadItemTest, DestinationError_RestartRequired) {
   DownloadItemImpl* item = CreateDownloadItem();
   MockDownloadFile* download_file =
@@ -1290,8 +1408,8 @@ TEST_F(DownloadItemTest, DestinationError_RestartRequired) {
   hash->Update(kTestData1, sizeof(kTestData1));
 
   EXPECT_CALL(*download_file, Cancel());
-  as_observer->DestinationError(
-      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED, 0, std::move(hash));
+  as_observer->DestinationError(DOWNLOAD_INTERRUPT_REASON_FILE_FAILED, 1,
+                                std::move(hash));
   mock_delegate()->VerifyAndClearExpectations();
   EXPECT_TRUE(observer.CheckAndResetDownloadUpdated());
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
@@ -1643,7 +1761,7 @@ TEST_F(DownloadItemTest, StealDangerousDownloadAndKeep) {
   CleanupItem(item, download_file, DownloadItem::IN_PROGRESS);
 }
 
-TEST_F(DownloadItemTest, StealInterruptedDangerousDownload) {
+TEST_F(DownloadItemTest, StealInterruptedContinuableDangerousDownload) {
   base::FilePath returned_path;
   DownloadItemImpl* item = CreateDownloadItem();
   MockDownloadFile* download_file =
@@ -1653,7 +1771,7 @@ TEST_F(DownloadItemTest, StealInterruptedDangerousDownload) {
   EXPECT_CALL(*download_file, FullPath()).WillOnce(ReturnRefOfCopy(full_path));
   EXPECT_CALL(*download_file, Detach());
   item->DestinationObserverAsWeakPtr()->DestinationError(
-      DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, 0,
+      DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, 1,
       std::unique_ptr<crypto::SecureHash>());
   ASSERT_TRUE(item->IsDangerous());
 
@@ -1668,14 +1786,14 @@ TEST_F(DownloadItemTest, StealInterruptedDangerousDownload) {
   EXPECT_EQ(full_path, returned_path);
 }
 
-TEST_F(DownloadItemTest, StealInterruptedNonResumableDangerousDownload) {
+TEST_F(DownloadItemTest, StealInterruptedNonContinuableDangerousDownload) {
   base::FilePath returned_path;
   DownloadItemImpl* item = CreateDownloadItem();
   MockDownloadFile* download_file =
       DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE);
   EXPECT_CALL(*download_file, Cancel());
   item->DestinationObserverAsWeakPtr()->DestinationError(
-      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED, 0,
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED, 1,
       std::unique_ptr<crypto::SecureHash>());
   ASSERT_TRUE(item->IsDangerous());
 
@@ -1962,7 +2080,8 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, DownloadCancelledByUser) {
                        destination_observer);
   target_callback.Run(base::FilePath(),
                       DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-                      DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, base::FilePath());
+                      DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, base::FilePath(),
+                      DOWNLOAD_INTERRUPT_REASON_NONE);
   EXPECT_EQ(DownloadItem::CANCELLED, item_->GetState());
   RunAllPendingInMessageLoops();
 }
@@ -2015,7 +2134,8 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameFails) {
   target_callback.Run(base::FilePath(kDummyTargetPath),
                       DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                       DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
-                      base::FilePath(kDummyIntermediatePath));
+                      base::FilePath(kDummyIntermediatePath),
+                      DOWNLOAD_INTERRUPT_REASON_NONE);
 
   intermediate_rename_loop.Run();
   ASSERT_FALSE(intermediate_rename_callback.is_null());
@@ -2084,7 +2204,8 @@ TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) {
   target_callback.Run(base::FilePath(kDummyTargetPath),
                       DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                       DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
-                      base::FilePath(kDummyIntermediatePath));
+                      base::FilePath(kDummyIntermediatePath),
+                      DOWNLOAD_INTERRUPT_REASON_NONE);
 
   intermediate_rename_loop.Run();
   ASSERT_FALSE(intermediate_rename_callback.is_null());