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

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 0afad1bfca248fea496ee1801387afca60fc2572..b3088866d223f28d45deb07968192ebb37d5e275 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "base/feature_list.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/stl_util.h"
 #include "base/threading/thread.h"
 #include "content/browser/byte_stream.h"
@@ -18,13 +19,20 @@
 #include "content/browser/download/download_item_impl_delegate.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/mock_download_file.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/download_destination_observer.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_url_parameters.h"
+#include "content/public/browser/site_instance.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/content_features.h"
 #include "content/public/test/mock_download_item.h"
+#include "content/public/test/mock_download_item.h"
+#include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/web_contents_tester.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -66,7 +74,6 @@ class MockDelegate : public DownloadItemImplDelegate {
                void(DownloadUrlParameters* params, uint32_t id));
 
   MOCK_CONST_METHOD0(GetBrowserContext, BrowserContext*());
-  MOCK_METHOD1(UpdatePersistence, void(DownloadItemImpl*));
   MOCK_METHOD1(DownloadOpened, void(DownloadItemImpl*));
   MOCK_METHOD1(DownloadRemoved, void(DownloadItemImpl*));
   MOCK_CONST_METHOD1(AssertStateConsistent, void(DownloadItemImpl*));
@@ -97,6 +104,27 @@ class MockRequestHandle : public DownloadRequestHandleInterface {
   MOCK_CONST_METHOD0(DebugString, std::string());
 };
 
+// |new_path| on the UI thread. Should only be used as the action for
+// MockDownloadFile::Rename as follows:
+//
+//   EXPECT_CALL(download_file, Rename*(_,_))
+//       .WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
+//                                        new_path));
+ACTION_P2(ScheduleRenameCallback, interrupt_reason, new_path) {
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(arg1, interrupt_reason, new_path));
+}
+
+// Schedules a task to invoke DownloadFile::InitializedCallback with
+// |interrupt_reason| on the UI thread. Should be used as the action for
+// MockDownloadFile::Initialize as follows:
+//   EXPECT_CALL(download_file, Initialize(_))
+//       .WillOnce(ScheduleInitializeCallback(DOWNLOAD_INTERRUPT_REASON_NONE));
+ACTION_P(ScheduleInitializeCallback, interrupt_reason) {
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(arg0, interrupt_reason));
+}
+
 class TestDownloadItemObserver : public DownloadItem::Observer {
  public:
   explicit TestDownloadItemObserver(DownloadItem* item)
@@ -128,8 +156,8 @@ class TestDownloadItemObserver : public DownloadItem::Observer {
 
  private:
   void OnDownloadRemoved(DownloadItem* download) override {
-    DVLOG(20) << " " << __FUNCTION__
-              << " download = " << download->DebugString(false);
+    SCOPED_TRACE(::testing::Message() << " " << __FUNCTION__ << " download = "
+                                      << download->DebugString(false));
     removed_ = true;
   }
 
@@ -171,25 +199,18 @@ class TestDownloadItemObserver : public DownloadItem::Observer {
   int resume_count_;
 };
 
-// Schedules a task to invoke the RenameCompletionCallback with |new_path| on
-// the UI thread. Should only be used as the action for
-// MockDownloadFile::Rename as follows:
-//   EXPECT_CALL(download_file, Rename*(_,_))
-//       .WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
-//                                        new_path));
-ACTION_P2(ScheduleRenameCallback, interrupt_reason, new_path) {
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                          base::Bind(arg1, interrupt_reason, new_path));
-}
-
 }  // namespace
 
 class DownloadItemTest : public testing::Test {
  public:
   DownloadItemTest()
-      : ui_thread_(BrowserThread::UI, &loop_),
-        file_thread_(BrowserThread::FILE, &loop_),
-        delegate_() {
+      : delegate_(), next_download_id_(DownloadItem::kInvalidId + 1) {
+    create_info_.reset(new DownloadCreateInfo());
+    create_info_->save_info =
+        scoped_ptr<DownloadSaveInfo>(new DownloadSaveInfo());
+    create_info_->save_info->prompt_for_save_location = false;
+    create_info_->url_chain.push_back(GURL());
+    create_info_->etag = "SomethingToSatisfyResumption";
   }
 
   ~DownloadItemTest() {
@@ -204,7 +225,8 @@ class DownloadItemTest : public testing::Test {
   }
 
   virtual void TearDown() {
-    ui_thread_.DeprecatedGetThreadObject()->message_loop()->RunUntilIdle();
+    base::RunLoop run_loop;
+    run_loop.RunUntilIdle();
     STLDeleteElements(&allocated_downloads_);
   }
 
@@ -212,21 +234,10 @@ class DownloadItemTest : public testing::Test {
   // be torn down at the end of the test unless DestroyDownloadItem is
   // called.
   DownloadItemImpl* CreateDownloadItem() {
-    scoped_ptr<DownloadCreateInfo> info;
-
-    info.reset(new DownloadCreateInfo());
-    info->save_info = scoped_ptr<DownloadSaveInfo>(new DownloadSaveInfo());
-    info->save_info->prompt_for_save_location = false;
-    info->url_chain.push_back(GURL());
-    info->etag = "SomethingToSatisfyResumption";
-
-    return CreateDownloadItemWithCreateInfo(std::move(info));
-  }
-
-  DownloadItemImpl* CreateDownloadItemWithCreateInfo(
-      scoped_ptr<DownloadCreateInfo> info) {
-    DownloadItemImpl* download = new DownloadItemImpl(
-        &delegate_, next_download_id_++, *(info.get()), net::BoundNetLog());
+    create_info_->download_id = ++next_download_id_;
+    DownloadItemImpl* download =
+        new DownloadItemImpl(&delegate_, create_info_->download_id,
+                             *create_info_, net::BoundNetLog());
     allocated_downloads_.insert(download);
     return download;
   }
@@ -235,9 +246,8 @@ class DownloadItemTest : public testing::Test {
   MockDownloadFile* AddDownloadFileToDownloadItem(
       DownloadItemImpl* item,
       DownloadItemImplDelegate::DownloadTargetCallback *callback) {
-    MockDownloadFile* mock_download_file(new StrictMock<MockDownloadFile>);
-    scoped_ptr<DownloadFile> download_file(mock_download_file);
-    EXPECT_CALL(*mock_download_file, Initialize(_));
+    MockDownloadFile* mock_download_file = nullptr;
+    scoped_ptr<DownloadFile> download_file;
     if (callback) {
       // Save the callback.
       EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
@@ -247,10 +257,23 @@ class DownloadItemTest : public testing::Test {
       EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _));
     }
 
-    scoped_ptr<DownloadRequestHandleInterface> request_handle(
+    // Only create a DownloadFile if the request was successful.
+    if (create_info_->result == DOWNLOAD_INTERRUPT_REASON_NONE) {
+      mock_download_file = new StrictMock<MockDownloadFile>;
+      download_file.reset(mock_download_file);
+      EXPECT_CALL(*mock_download_file, Initialize(_))
+          .WillOnce(ScheduleInitializeCallback(DOWNLOAD_INTERRUPT_REASON_NONE));
+      EXPECT_CALL(*mock_download_file, FullPath())
+          .WillRepeatedly(Return(base::FilePath()));
+    }
+
+    scoped_ptr<MockRequestHandle> request_handle(
         new NiceMock<MockRequestHandle>);
-    item->Start(std::move(download_file), std::move(request_handle));
-    loop_.RunUntilIdle();
+    EXPECT_CALL(*request_handle, GetWebContents())
+        .WillRepeatedly(Return(GetWebContents()));
+    item->Start(std::move(download_file), std::move(request_handle),
+                *create_info_);
+    base::RunLoop().RunUntilIdle();
 
     // So that we don't have a function writing to a stack variable
     // lying around if the above failed.
@@ -298,7 +321,7 @@ class DownloadItemTest : public testing::Test {
       if (download_file)
         EXPECT_CALL(*download_file, Cancel());
       item->Cancel(true);
-      loop_.RunUntilIdle();
+      base::RunLoop().RunUntilIdle();
     }
   }
 
@@ -308,9 +331,7 @@ class DownloadItemTest : public testing::Test {
     delete item;
   }
 
-  void RunAllPendingInMessageLoops() {
-    loop_.RunUntilIdle();
-  }
+  void RunAllPendingInMessageLoops() { base::RunLoop().RunUntilIdle(); }
 
   MockDelegate* mock_delegate() {
     return &delegate_;
@@ -321,15 +342,57 @@ class DownloadItemTest : public testing::Test {
     *return_path = path;
   }
 
+  DownloadCreateInfo* create_info() { return create_info_.get(); }
+
+  virtual WebContents* GetWebContents() { return nullptr; }
+
  private:
-  int next_download_id_ = DownloadItem::kInvalidId + 1;
-  base::MessageLoopForUI loop_;
-  TestBrowserThread ui_thread_;    // UI thread
-  TestBrowserThread file_thread_;  // FILE thread
   StrictMock<MockDelegate> delegate_;
   std::set<DownloadItem*> allocated_downloads_;
+  scoped_ptr<DownloadCreateInfo> create_info_;
+  uint32_t next_download_id_;
+  TestBrowserThreadBundle thread_bundle_;
+};
+
+// Test fixture for resumption tests. The fixture takes a GTest parameter of
+// type 'bool'. If the parameter is true, then the DownloadItem has a valid
+// WebContents. It would have a null WebContents otherwise. Whether or not a
+// WebContents exists has a bearing on the code path used for resumption. Hence
+// it's important to exercise both possibilities for these tests.
+class DownloadItemTestWithResumption
+    : public DownloadItemTest,
+      public ::testing::WithParamInterface<bool> {
+ public:
+  void SetUp() override {
+    browser_context_.reset(new TestBrowserContext);
+    if (GetParam()) {
+      scoped_refptr<SiteInstance> site_instance(
+          SiteInstance::Create(browser_context_.get()));
+      web_contents_.reset(WebContentsTester::CreateTestWebContents(
+          browser_context_.get(), site_instance.get()));
+    }
+    DownloadItemTest::SetUp();
+  }
+
+  void TearDown() override {
+    DownloadItemTest::TearDown();
+    web_contents_.reset();
+    browser_context_.reset();
+  }
+
+  WebContents* GetWebContents() override { return web_contents_.get(); }
+
+  BrowserContext* GetBrowserContext() { return browser_context_.get(); }
+
+ private:
+  scoped_ptr<TestBrowserContext> browser_context_;
+  scoped_ptr<WebContents> web_contents_;
 };
 
+INSTANTIATE_TEST_CASE_P(WithAndWithoutWebContents,
+                        DownloadItemTestWithResumption,
+                        ::testing::Bool());
+
 // Tests to ensure calls that change a DownloadItem generate an update to
 // observers.
 // State changing functions not tested:
@@ -408,8 +471,8 @@ TEST_F(DownloadItemTest, NotificationAfterDestroyed) {
   ASSERT_TRUE(observer.download_destroyed());
 }
 
-TEST_F(DownloadItemTest, ContinueAfterInterrupted) {
-  TestBrowserContext test_browser_context;
+// Test that a download is resumed automatcially after a continuable interrupt.
+TEST_P(DownloadItemTestWithResumption, ContinueAfterInterrupted) {
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
   MockDownloadFile* download_file =
@@ -420,7 +483,7 @@ TEST_F(DownloadItemTest, ContinueAfterInterrupted) {
       .WillOnce(Return(base::FilePath()));
   EXPECT_CALL(*download_file, Detach());
   EXPECT_CALL(*mock_delegate(), GetBrowserContext())
-      .WillRepeatedly(Return(&test_browser_context));
+      .WillRepeatedly(Return(GetBrowserContext()));
   EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_, _)).Times(1);
   item->DestinationObserverAsWeakPtr()->DestinationError(
       DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
@@ -441,8 +504,9 @@ TEST_F(DownloadItemTest, ContinueAfterInterrupted) {
   CleanupItem(item, nullptr, DownloadItem::IN_PROGRESS);
 }
 
-// Same as above, but with a non-continuable interrupt.
-TEST_F(DownloadItemTest, RestartAfterInterrupted) {
+// Test that automatic resumption doesn't happen after a non-continuable
+// interrupt.
+TEST_P(DownloadItemTestWithResumption, RestartAfterInterrupted) {
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
   MockDownloadFile* download_file =
@@ -488,8 +552,7 @@ TEST_F(DownloadItemTest, UnresumableInterrupt) {
   CleanupItem(item, nullptr, DownloadItem::INTERRUPTED);
 }
 
-TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
-  TestBrowserContext test_browser_context;
+TEST_P(DownloadItemTestWithResumption, LimitRestartsAfterInterrupted) {
   DownloadItemImpl* item = CreateDownloadItem();
   base::WeakPtr<DownloadDestinationObserver> as_observer(
       item->DestinationObserverAsWeakPtr());
@@ -503,11 +566,11 @@ TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
       .WillRepeatedly(SaveArg<1>(&callback));
   EXPECT_CALL(*mock_delegate(), GetBrowserContext())
-      .WillRepeatedly(Return(&test_browser_context));
+      .WillRepeatedly(Return(GetBrowserContext()));
   EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_, _))
       .Times(DownloadItemImpl::kMaxAutoResumeAttempts);
   for (int i = 0; i < (DownloadItemImpl::kMaxAutoResumeAttempts + 1); ++i) {
-    DVLOG(20) << "Loop iteration " << i;
+    SCOPED_TRACE(::testing::Message() << "Iteration " << i);
 
     mock_download_file = new NiceMock<MockDownloadFile>;
     download_file.reset(mock_download_file);
@@ -519,7 +582,8 @@ TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
 
     // Copied key parts of DoIntermediateRename & AddDownloadFileToDownloadItem
     // to allow for holding onto the request handle.
-    item->Start(std::move(download_file), std::move(request_handle));
+    item->Start(std::move(download_file), std::move(request_handle),
+                *create_info());
     RunAllPendingInMessageLoops();
     if (i == 0) {
       // Target determination is only done the first time through.
@@ -538,6 +602,7 @@ TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
     item->DestinationObserverAsWeakPtr()->DestinationError(
         DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
 
+    RunAllPendingInMessageLoops();
     ::testing::Mock::VerifyAndClearExpectations(mock_download_file);
   }
 
@@ -545,19 +610,73 @@ TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
   CleanupItem(item, nullptr, DownloadItem::INTERRUPTED);
 }
 
+// If the download attempts to resume and the resumption request fails, the
+// subsequent Start() call shouldn't update the origin state (URL redirect
+// chains, Content-Disposition, download URL, etc..)
+TEST_P(DownloadItemTestWithResumption,
+       FailedResumptionDoesntUpdateOriginState) {
+  const char kContentDisposition[] = "attachment; filename=foo";
+  const char kFirstETag[] = "ABC";
+  const char kFirstLastModified[] = "Yesterday";
+  const char kFirstURL[] = "http://www.example.com/download";
+  create_info()->content_disposition = kContentDisposition;
+  create_info()->etag = kFirstETag;
+  create_info()->last_modified = kFirstLastModified;
+  create_info()->url_chain.push_back(GURL(kFirstURL));
+
+  DownloadItemImpl* item = CreateDownloadItem();
+  MockDownloadFile* download_file =
+      DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
+  EXPECT_EQ(kContentDisposition, item->GetContentDisposition());
+  EXPECT_EQ(kFirstETag, item->GetETag());
+  EXPECT_EQ(kFirstLastModified, item->GetLastModifiedTime());
+  EXPECT_EQ(kFirstURL, item->GetURL().spec());
+
+  EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_, _));
+  EXPECT_CALL(*mock_delegate(), GetBrowserContext())
+      .WillRepeatedly(Return(GetBrowserContext()));
+  EXPECT_CALL(*download_file, Detach());
+  item->DestinationObserverAsWeakPtr()->DestinationError(
+      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
+  RunAllPendingInMessageLoops();
+  EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
+
+  // Now change the create info. The changes should not cause the DownloadItem
+  // to be updated.
+  const char kSecondContentDisposition[] = "attachment; filename=bar";
+  const char kSecondETag[] = "123";
+  const char kSecondLastModified[] = "Today";
+  const char kSecondURL[] = "http://example.com/another-download";
+  create_info()->content_disposition = kSecondContentDisposition;
+  create_info()->etag = kSecondETag;
+  create_info()->last_modified = kSecondLastModified;
+  create_info()->url_chain.clear();
+  create_info()->url_chain.push_back(GURL(kSecondURL));
+  create_info()->result = DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED;
+
+  // The following ends up calling DownloadItem::Start(), but shouldn't result
+  // in an origin update.
+  DownloadItemImplDelegate::DownloadTargetCallback download_target_callback;
+  download_file =
+      AddDownloadFileToDownloadItem(item, &download_target_callback);
+
+  EXPECT_EQ(kContentDisposition, item->GetContentDisposition());
+  EXPECT_EQ(kFirstETag, item->GetETag());
+  EXPECT_EQ(kFirstLastModified, item->GetLastModifiedTime());
+  EXPECT_EQ(kFirstURL, item->GetURL().spec());
+  EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
+  EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, item->GetLastReason());
+}
+
 // Test that resumption uses the final URL in a URL chain when resuming.
-TEST_F(DownloadItemTest, ResumeUsingFinalURL) {
-  TestBrowserContext test_browser_context;
-  scoped_ptr<DownloadCreateInfo> create_info(new DownloadCreateInfo);
-  create_info->save_info = scoped_ptr<DownloadSaveInfo>(new DownloadSaveInfo());
-  create_info->save_info->prompt_for_save_location = false;
-  create_info->etag = "SomethingToSatisfyResumption";
-  create_info->url_chain.push_back(GURL("http://example.com/a"));
-  create_info->url_chain.push_back(GURL("http://example.com/b"));
-  create_info->url_chain.push_back(GURL("http://example.com/c"));
-
-  DownloadItemImpl* item =
-      CreateDownloadItemWithCreateInfo(std::move(create_info));
+TEST_P(DownloadItemTestWithResumption, ResumeUsingFinalURL) {
+  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"));
+  create_info()->url_chain.push_back(GURL("http://example.com/b"));
+  create_info()->url_chain.push_back(GURL("http://example.com/c"));
+
+  DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
   MockDownloadFile* download_file =
       DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
@@ -566,7 +685,7 @@ TEST_F(DownloadItemTest, ResumeUsingFinalURL) {
   EXPECT_CALL(*download_file, FullPath()).WillOnce(Return(base::FilePath()));
   EXPECT_CALL(*download_file, Detach());
   EXPECT_CALL(*mock_delegate(), GetBrowserContext())
-      .WillRepeatedly(Return(&test_browser_context));
+      .WillRepeatedly(Return(GetBrowserContext()));
   EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(
                                     Property(&DownloadUrlParameters::url,
                                              GURL("http://example.com/c")),
@@ -609,8 +728,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
   EXPECT_TRUE(safe_observer.CheckAndResetDownloadUpdated());
 
   // Setting to unsafe url or unsafe file should trigger a notification.
-  DownloadItemImpl* unsafeurl_item =
-      CreateDownloadItem();
+  DownloadItemImpl* unsafeurl_item = CreateDownloadItem();
   TestDownloadItemObserver unsafeurl_observer(unsafeurl_item);
 
   unsafeurl_item->OnAllDataSaved(std::string());
@@ -621,8 +739,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
   unsafeurl_item->ValidateDangerousDownload();
   EXPECT_TRUE(unsafeurl_observer.CheckAndResetDownloadUpdated());
 
-  DownloadItemImpl* unsafefile_item =
-      CreateDownloadItem();
+  DownloadItemImpl* unsafefile_item = CreateDownloadItem();
   TestDownloadItemObserver unsafefile_observer(unsafefile_item);
 
   unsafefile_item->OnAllDataSaved(std::string());
@@ -675,7 +792,8 @@ TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
 
   EXPECT_CALL(*mock_download_file, Initialize(_));
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(_, _));
-  item->Start(std::move(download_file), std::move(request_handle));
+  item->Start(std::move(download_file), std::move(request_handle),
+              *create_info());
 
   item->Pause();
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
@@ -722,12 +840,45 @@ TEST_F(DownloadItemTest, Start) {
   scoped_ptr<DownloadRequestHandleInterface> request_handle(
       new NiceMock<MockRequestHandle>);
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _));
-  item->Start(std::move(download_file), std::move(request_handle));
+  item->Start(std::move(download_file), std::move(request_handle),
+              *create_info());
   RunAllPendingInMessageLoops();
 
   CleanupItem(item, mock_download_file, DownloadItem::IN_PROGRESS);
 }
 
+// Handling of downloads initiated via a failed request. In this case, Start()
+// will get called with a DownloadCreateInfo with a non-zero interrupt_reason.
+TEST_F(DownloadItemTest, StartFailedDownload) {
+  create_info()->result = DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED;
+  DownloadItemImpl* item = CreateDownloadItem();
+  EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
+
+  // DownloadFile and DownloadRequestHandleInterface objects aren't created for
+  // failed downloads.
+  scoped_ptr<DownloadFile> null_download_file;
+  scoped_ptr<DownloadRequestHandleInterface> null_request_handle;
+  DownloadItemImplDelegate::DownloadTargetCallback download_target_callback;
+  EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
+      .WillOnce(SaveArg<1>(&download_target_callback));
+  item->Start(std::move(null_download_file), std::move(null_request_handle),
+              *create_info());
+  RunAllPendingInMessageLoops();
+
+  // The DownloadItemImpl should attempt to determine a target path even if the
+  // download was interrupted.
+  ASSERT_FALSE(download_target_callback.is_null());
+  ASSERT_EQ(DownloadItem::INTERRUPTED, item->GetState());
+  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);
+  RunAllPendingInMessageLoops();
+
+  EXPECT_EQ(target_path, item->GetTargetFilePath());
+  CleanupItem(item, NULL, DownloadItem::INTERRUPTED);
+}
+
 // Test that the delegate is invoked after the download file is renamed.
 TEST_F(DownloadItemTest, CallbackAfterRename) {
   DownloadItemImpl* item = CreateDownloadItem();
@@ -844,7 +995,8 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Restart) {
 // As above. But if the download can be resumed by continuing, then the
 // intermediate path should be retained when the download is interrupted after
 // the intermediate rename succeeds.
-TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Continue) {
+TEST_P(DownloadItemTestWithResumption,
+       InterruptedBeforeIntermediateRename_Continue) {
   DownloadItemImpl* item = CreateDownloadItem();
   DownloadItemImplDelegate::DownloadTargetCallback callback;
   MockDownloadFile* download_file =
@@ -877,7 +1029,8 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Continue) {
 
 // As above. If the intermediate rename fails, then the interrupt reason should
 // be set to the destination error and the intermediate path should be empty.
-TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Failed) {
+TEST_P(DownloadItemTestWithResumption,
+       InterruptedBeforeIntermediateRename_Failed) {
   DownloadItemImpl* item = CreateDownloadItem();
   DownloadItemImplDelegate::DownloadTargetCallback callback;
   MockDownloadFile* download_file =
@@ -1275,7 +1428,7 @@ TEST_F(DownloadItemTest, StealDangerousDownload) {
   EXPECT_EQ(full_path, returned_path);
 }
 
-TEST_F(DownloadItemTest, StealInterruptedDangerousDownload) {
+TEST_P(DownloadItemTestWithResumption, StealInterruptedDangerousDownload) {
   base::FilePath returned_path;
   DownloadItemImpl* item = CreateDownloadItem();
   MockDownloadFile* download_file =
@@ -1299,7 +1452,8 @@ TEST_F(DownloadItemTest, StealInterruptedDangerousDownload) {
   EXPECT_EQ(full_path, returned_path);
 }
 
-TEST_F(DownloadItemTest, StealInterruptedNonResumableDangerousDownload) {
+TEST_P(DownloadItemTestWithResumption,
+       StealInterruptedNonResumableDangerousDownload) {
   base::FilePath returned_path;
   DownloadItemImpl* item = CreateDownloadItem();
   MockDownloadFile* download_file =