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

content/browser/download/download_item_impl_unittest.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 "content/browser/download/download_item_impl.h"
 
 #include <stdint.h>
 
 #include <iterator>
 #include <queue>
@@ skipping 10 matching lines @@
 #include "content/browser/download/download_file_factory.h"
 #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/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_bundle.h"
+#include "content/public/test/web_contents_tester.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::DoAll;
 using ::testing::NiceMock;
 using ::testing::Property;
 using ::testing::Return;
 using ::testing::SaveArg;
 using ::testing::StrictMock;
 using ::testing::WithArg;
@@ skipping 26 matching lines @@
   MOCK_METHOD1(CheckForFileRemoval, void(DownloadItemImpl*));
 
   void ResumeInterruptedDownload(scoped_ptr<DownloadUrlParameters> params,
                                  uint32_t id) override {
     MockResumeInterruptedDownload(params.get(), id);
   }
   MOCK_METHOD2(MockResumeInterruptedDownload,
                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*));
 
   void VerifyAndClearExpectations() {
     ::testing::Mock::VerifyAndClearExpectations(this);
     SetDefaultExpectations();
   }
 
  private:
@@ skipping 41 matching lines @@
   int resume_count() const { return resume_count_; }
 
   bool CheckAndResetDownloadUpdated() {
     bool was_updated = updated_;
     updated_ = false;
     return was_updated;
   }
 
  private:
   void OnDownloadRemoved(DownloadItem* download) override {
-    DVLOG(20) << " " << __FUNCTION__
-              << " download = " << download->DebugString(false);
+    SCOPED_TRACE(::testing::Message() << " " << __FUNCTION__ << " download = "
+                                      << download->DebugString(false));
     removed_ = true;
   }
 
   void OnDownloadUpdated(DownloadItem* download) override {
     DVLOG(20) << " " << __FUNCTION__
               << " download = " << download->DebugString(false);
     updated_ = true;
     DownloadItem::DownloadState new_state = download->GetState();
     if (last_state_ == DownloadItem::IN_PROGRESS &&
         new_state == DownloadItem::INTERRUPTED) {
@@ skipping 53 matching lines @@
 }
 
 ACTION_P(ScheduleClosure, closure) {
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, closure);
 }
 
 }  // namespace
 
 class DownloadItemTest : public testing::Test {
  public:
-  DownloadItemTest() {
+  DownloadItemTest()
+      : delegate_(), next_download_id_(DownloadItem::kInvalidId + 1) {
     base::FeatureList::ClearInstanceForTesting();
     scoped_ptr<base::FeatureList> feature_list(new base::FeatureList);
     feature_list->InitializeFromCommandLine(features::kDownloadResumption.name,
                                             std::string());
     base::FeatureList::SetInstance(std::move(feature_list));
+
+    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("http://example.com/download"));
+    create_info_->etag = "SomethingToSatisfyResumption";
   }
 
   ~DownloadItemTest() {
     RunAllPendingInMessageLoops();
     STLDeleteElements(&allocated_downloads_);
   }
 
   DownloadItemImpl* CreateDownloadItemWithCreateInfo(
       scoped_ptr<DownloadCreateInfo> info) {
     DownloadItemImpl* download = new DownloadItemImpl(
         &delegate_, next_download_id_++, *(info.get()), net::BoundNetLog());
     allocated_downloads_.insert(download);
     return download;
   }
 
   // This class keeps ownership of the created download item; it will
   // 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("http://example.com/download"));
-    info->etag = "SomethingToSatisfyResumption";
-
-    return CreateDownloadItemWithCreateInfo(std::move(info));
+    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;
   }
 
-  // Add DownloadFile to DownloadItem
+  // Add DownloadFile to DownloadItem. Set |callback| to nullptr if a download
+  // target determination is not expected.
   MockDownloadFile* CallDownloadItemStart(
       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, _))
           .WillOnce(SaveArg<1>(callback));
     } else {
-      // Drop it on the floor.
-      EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _));
+      EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _)).Times(0);
     }
 
-    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(ScheduleCallbackWithParam(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));
+    item->Start(std::move(download_file), std::move(request_handle),
+                *create_info_);
     RunAllPendingInMessageLoops();
 
     // So that we don't have a function writing to a stack variable
     // lying around if the above failed.
     mock_delegate()->VerifyAndClearExpectations();
     EXPECT_CALL(*mock_delegate(), AssertStateConsistent(_))
         .WillRepeatedly(Return());
     EXPECT_CALL(*mock_delegate(), ShouldOpenFileBasedOnExtension(_))
         .WillRepeatedly(Return(false));
     EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(_, _))
@@ skipping 63 matching lines @@
 
   MockDelegate* mock_delegate() {
     return &delegate_;
   }
 
   void OnDownloadFileAcquired(base::FilePath* return_path,
                               const base::FilePath& path) {
     *return_path = path;
   }
 
+  DownloadCreateInfo* create_info() { return create_info_.get(); }
+
+  BrowserContext* browser_context() { return &browser_context_; }
+
  private:
-  int next_download_id_ = DownloadItem::kInvalidId + 1;
   StrictMock<MockDelegate> delegate_;
   std::set<DownloadItem*> allocated_downloads_;
+  scoped_ptr<DownloadCreateInfo> create_info_;
+  uint32_t next_download_id_ = DownloadItem::kInvalidId + 1;
+  TestBrowserContext browser_context_;
   TestBrowserThreadBundle thread_bundle_;
 };
 
 // Tests to ensure calls that change a DownloadItem generate an update to
 // observers.
 // State changing functions not tested:
 //  void OpenDownload();
 //  void ShowDownloadInShell();
 //  void CompleteDelayedDownload();
 //  set_* mutators
 
 TEST_F(DownloadItemTest, NotificationAfterUpdate) {
   DownloadItemImpl* item = CreateDownloadItem();
   MockDownloadFile* file =
       DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
   ASSERT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
   TestDownloadItemObserver observer(item);
 
   item->DestinationUpdate(kDownloadChunkSize, kDownloadSpeed, std::string());
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
   EXPECT_EQ(kDownloadSpeed, item->CurrentSpeed());
   CleanupItem(item, file, DownloadItem::IN_PROGRESS);
 }
 
 TEST_F(DownloadItemTest, NotificationAfterCancel) {
   DownloadItemImpl* user_cancel = CreateDownloadItem();
-  MockDownloadFile* download_file = CallDownloadItemStart(user_cancel, nullptr);
+  DownloadItemImplDelegate::DownloadTargetCallback target_callback;
+  MockDownloadFile* download_file =
+      CallDownloadItemStart(user_cancel, &target_callback);
   EXPECT_CALL(*download_file, Cancel());
+
   TestDownloadItemObserver observer1(user_cancel);
-
   user_cancel->Cancel(true);
   ASSERT_TRUE(observer1.CheckAndResetDownloadUpdated());
 
   DownloadItemImpl* system_cancel = CreateDownloadItem();
-  download_file = CallDownloadItemStart(system_cancel, nullptr);
+  download_file = CallDownloadItemStart(system_cancel, &target_callback);
   EXPECT_CALL(*download_file, Cancel());
+
   TestDownloadItemObserver observer2(system_cancel);
-
   system_cancel->Cancel(false);
   ASSERT_TRUE(observer2.CheckAndResetDownloadUpdated());
 }
 
 TEST_F(DownloadItemTest, NotificationAfterComplete) {
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
   MockDownloadFile* download_file =
       DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
@@ skipping 25 matching lines @@
 }
 
 TEST_F(DownloadItemTest, NotificationAfterDestroyed) {
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
 
   DestroyDownloadItem(item);
   ASSERT_TRUE(observer.download_destroyed());
 }
 
+// Test that a download is resumed automatcially after a continuable interrupt.
 TEST_F(DownloadItemTest, ContinueAfterInterrupted) {
-  TestBrowserContext test_browser_context;
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
   MockDownloadFile* download_file =
       DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
 
   // Interrupt the download, using a continuable interrupt.
   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(browser_context()));
   EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_, _)).Times(1);
   item->DestinationObserverAsWeakPtr()->DestinationError(
       DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
   // Since the download is resumed automatically, the interrupt count doesn't
   // increase.
   ASSERT_EQ(0, observer.interrupt_count());
 
   // Test expectations verify that ResumeInterruptedDownload() is called (by way
   // of MockResumeInterruptedDownload) after the download is interrupted. But
   // the mock doesn't follow through with the resumption.
   // ResumeInterruptedDownload() being called is sufficient for verifying that
   // the automatic resumption was triggered.
   RunAllPendingInMessageLoops();
 
   // The download item is currently in RESUMING_INTERNAL state, which maps to
   // IN_PROGRESS.
   CleanupItem(item, nullptr, DownloadItem::IN_PROGRESS);
 }
 
-// Same as above, but with a non-continuable interrupt.
+// Test that automatic resumption doesn't happen after a non-continuable
+// interrupt.
 TEST_F(DownloadItemTest, RestartAfterInterrupted) {
   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);
@@ skipping 28 matching lines @@
 
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
   // Should not try to auto-resume.
   ASSERT_EQ(1, observer.interrupt_count());
   ASSERT_EQ(0, observer.resume_count());
 
   CleanupItem(item, nullptr, DownloadItem::INTERRUPTED);
 }
 
 TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
-  TestBrowserContext test_browser_context;
   DownloadItemImpl* item = CreateDownloadItem();
   base::WeakPtr<DownloadDestinationObserver> as_observer(
       item->DestinationObserverAsWeakPtr());
   TestDownloadItemObserver observer(item);
   MockDownloadFile* mock_download_file(nullptr);
   scoped_ptr<DownloadFile> download_file;
   MockRequestHandle* mock_request_handle(nullptr);
   scoped_ptr<DownloadRequestHandleInterface> request_handle;
   DownloadItemImplDelegate::DownloadTargetCallback callback;
 
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
       .WillRepeatedly(SaveArg<1>(&callback));
   EXPECT_CALL(*mock_delegate(), GetBrowserContext())
-      .WillRepeatedly(Return(&test_browser_context));
+      .WillRepeatedly(Return(browser_context()));
   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);
     mock_request_handle = new NiceMock<MockRequestHandle>;
     request_handle.reset(mock_request_handle);
 
     ON_CALL(*mock_download_file, FullPath())
         .WillByDefault(Return(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(download_file), std::move(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(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
                                            intermediate_path));
     }
     callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                  DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
     RunAllPendingInMessageLoops();
 
     // Use a continuable interrupt.
     item->DestinationObserverAsWeakPtr()->DestinationError(
         DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
 
+    RunAllPendingInMessageLoops();
     ::testing::Mock::VerifyAndClearExpectations(mock_download_file);
   }
 
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   EXPECT_EQ(1, observer.interrupt_count());
   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_F(DownloadItemTest, 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(browser_context()));
+  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.
+  download_file = CallDownloadItemStart(item, nullptr);
+
+  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"));
+  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 =
-      CreateDownloadItemWithCreateInfo(std::move(create_info));
+  DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
   MockDownloadFile* download_file =
       DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
 
   // Interrupt the download, using a continuable interrupt.
   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(browser_context()));
   EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(
                                     Property(&DownloadUrlParameters::url,
                                              GURL("http://example.com/c")),
                                     _))
       .Times(1);
   item->DestinationObserverAsWeakPtr()->DestinationError(
       DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
 
   // Test expectations verify that ResumeInterruptedDownload() is called (by way
   // of MockResumeInterruptedDownload) after the download is interrupted. But
   // the mock doesn't follow through with the resumption.
   // ResumeInterruptedDownload() being called is sufficient for verifying that
   // the resumption was triggered.
   RunAllPendingInMessageLoops();
 
   // The download is currently in RESUMING_INTERNAL, which maps to IN_PROGRESS.
   CleanupItem(item, nullptr, DownloadItem::IN_PROGRESS);
 }
 
-TEST_F(DownloadItemTest, NotificationAfterRemove) {

[Randy Smith (Not in Mondays), 2016/02/12 22:09:23]

Why removed?

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

Whoops. Restored.

-  DownloadItemImpl* item = CreateDownloadItem();
-  MockDownloadFile* download_file = CallDownloadItemStart(item, nullptr);
-  EXPECT_CALL(*download_file, Cancel());
-  EXPECT_CALL(*mock_delegate(), DownloadRemoved(_));
-  TestDownloadItemObserver observer(item);
-
-  item->Remove();
-  ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
-  ASSERT_TRUE(observer.download_removed());
-}
-
 TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
   // Setting to NOT_DANGEROUS does not trigger a notification.
   DownloadItemImpl* safe_item = CreateDownloadItem();
   MockDownloadFile* download_file =
       DoIntermediateRename(safe_item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
   TestDownloadItemObserver safe_observer(safe_item);
 
   safe_item->OnAllDataSaved(std::string());
   EXPECT_TRUE(safe_observer.CheckAndResetDownloadUpdated());
   safe_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
@@ skipping 71 matching lines @@
 TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
   MockDownloadFile* mock_download_file(new MockDownloadFile);
   scoped_ptr<DownloadFile> download_file(mock_download_file);
   scoped_ptr<DownloadRequestHandleInterface> request_handle(
       new NiceMock<MockRequestHandle>);
 
   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());
 
   ASSERT_TRUE(item->IsPaused());
 
   item->Resume();
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
 
   RunAllPendingInMessageLoops();
@@ skipping 26 matching lines @@
 
 // Test to make sure that Start method calls DF initialize properly.
 TEST_F(DownloadItemTest, Start) {
   MockDownloadFile* mock_download_file(new MockDownloadFile);
   scoped_ptr<DownloadFile> download_file(mock_download_file);
   DownloadItemImpl* item = CreateDownloadItem();
   EXPECT_CALL(*mock_download_file, Initialize(_));
   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);
 }
 
 // Download file and the request should be cancelled as a result of download
 // file initialization failing.
 TEST_F(DownloadItemTest, InitDownloadFileFails) {
+  DownloadItemImpl* item = CreateDownloadItem();
   scoped_ptr<MockDownloadFile> file(new MockDownloadFile());
   scoped_ptr<MockRequestHandle> request_handle(new MockRequestHandle());
   EXPECT_CALL(*file, Cancel());
   EXPECT_CALL(*request_handle, CancelRequest());
   EXPECT_CALL(*file, Initialize(_))
       .WillOnce(ScheduleCallbackWithParam(
           DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED));
 
-  DownloadItemImpl* item = CreateDownloadItem();
-  item->Start(std::move(file), std::move(request_handle));
+  base::RunLoop start_download_loop;
+  DownloadItemImplDelegate::DownloadTargetCallback download_target_callback;
+  EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
+      .WillOnce(DoAll(SaveArg<1>(&download_target_callback),
+                      ScheduleClosure(start_download_loop.QuitClosure())));
+
+  item->Start(std::move(file), std::move(request_handle), *create_info());
+  start_download_loop.Run();
+
+  download_target_callback.Run(base::FilePath(kDummyTargetPath),
+                               DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                               DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+                               base::FilePath(kDummyIntermediatePath));
   RunAllPendingInMessageLoops();
 
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED,
             item->GetLastReason());
 }
 
+// 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();
+
+  // 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());
+  EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
+  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::IN_PROGRESS, 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();
   DownloadItemImplDelegate::DownloadTargetCallback callback;
   MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
   base::FilePath final_path(
       base::FilePath(kDummyTargetPath).AppendASCII("foo.bar"));
   base::FilePath intermediate_path(final_path.InsertBeforeExtensionASCII("x"));
   base::FilePath new_intermediate_path(
       final_path.InsertBeforeExtensionASCII("y"));
@@ skipping 162 matching lines @@
   ::testing::Mock::VerifyAndClearExpectations(download_file);
   mock_delegate()->VerifyAndClearExpectations();
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, item->GetLastReason());
   EXPECT_TRUE(item->GetFullPath().empty());
   EXPECT_EQ(final_path, item->GetTargetFilePath());
 }
 
 TEST_F(DownloadItemTest, Canceled) {
   DownloadItemImpl* item = CreateDownloadItem();
-  MockDownloadFile* download_file = CallDownloadItemStart(item, nullptr);
+  DownloadItemImplDelegate::DownloadTargetCallback target_callback;
+  MockDownloadFile* download_file =
+      CallDownloadItemStart(item, &target_callback);
 
   // Confirm cancel sets state properly.
   EXPECT_CALL(*download_file, Cancel());
   item->Cancel(true);
   EXPECT_EQ(DownloadItem::CANCELLED, item->GetState());
 }
 
 TEST_F(DownloadItemTest, FileRemoved) {
   DownloadItemImpl* item = CreateDownloadItem();
 
@@ skipping 52 matching lines @@
       DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
   mock_delegate()->VerifyAndClearExpectations();
   EXPECT_TRUE(observer.CheckAndResetDownloadUpdated());
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED,
             item->GetLastReason());
 }
 
 TEST_F(DownloadItemTest, DestinationCompleted) {
   DownloadItemImpl* item = CreateDownloadItem();
-  MockDownloadFile* download_file = CallDownloadItemStart(item, nullptr);
+  MockDownloadFile* download_file =
+      DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
   base::WeakPtr<DownloadDestinationObserver> as_observer(
       item->DestinationObserverAsWeakPtr());
   TestDownloadItemObserver observer(item);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
   EXPECT_EQ("", item->GetHash());
   EXPECT_EQ("", item->GetHashState());
   EXPECT_FALSE(item->AllDataSaved());
   EXPECT_FALSE(observer.CheckAndResetDownloadUpdated());
 
   as_observer->DestinationUpdate(10, 20, "deadbeef");
   EXPECT_TRUE(observer.CheckAndResetDownloadUpdated());
   EXPECT_FALSE(observer.CheckAndResetDownloadUpdated());  // Confirm reset.
   EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
   EXPECT_EQ("", item->GetHash());
   EXPECT_EQ("deadbeef", item->GetHashState());
   EXPECT_FALSE(item->AllDataSaved());
 
+  EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(_, _));
   as_observer->DestinationCompleted("livebeef");
   mock_delegate()->VerifyAndClearExpectations();
   EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
   EXPECT_TRUE(observer.CheckAndResetDownloadUpdated());
   EXPECT_EQ("livebeef", item->GetHash());
   EXPECT_EQ("", item->GetHashState());
   EXPECT_TRUE(item->AllDataSaved());
 
   // Even though the DownloadItem receives a DestinationCompleted() event,
   // target determination hasn't completed, hence the download item is stuck in
@@ skipping 557 matching lines @@
   // Expect that the download file and the request will be cancelled as a
   // result.
   EXPECT_CALL(*file_, Cancel());
   EXPECT_CALL(*request_handle_, CancelRequest());
 
   base::RunLoop download_start_loop;
   DownloadFile::InitializeCallback initialize_callback;
   EXPECT_CALL(*file_, Initialize(_))
       .WillOnce(DoAll(SaveArg<0>(&initialize_callback),
                       ScheduleClosure(download_start_loop.QuitClosure())));
-  item_->Start(std::move(file_), std::move(request_handle_));
+  item_->Start(std::move(file_), std::move(request_handle_), *create_info());
   download_start_loop.Run();
 
   base::WeakPtr<DownloadDestinationObserver> destination_observer =
       item_->DestinationObserverAsWeakPtr();
 
   ScheduleObservations(PreInitializeFileObservations(), destination_observer);
   RunAllPendingInMessageLoops();
 
   base::RunLoop initialize_completion_loop;
   DownloadItemImplDelegate::DownloadTargetCallback target_callback;
@@ skipping 32 matching lines @@
   EXPECT_CALL(*file_, RenameAndUniquify(_, _))
       .WillOnce(DoAll(SaveArg<1>(&intermediate_rename_callback),
                       ScheduleClosure(intermediate_rename_loop.QuitClosure())));
 
   base::RunLoop download_start_loop;
   DownloadFile::InitializeCallback initialize_callback;
   EXPECT_CALL(*file_, Initialize(_))
       .WillOnce(DoAll(SaveArg<0>(&initialize_callback),
                       ScheduleClosure(download_start_loop.QuitClosure())));
 
-  item_->Start(std::move(file_), std::move(request_handle_));
+  item_->Start(std::move(file_), std::move(request_handle_), *create_info());
   download_start_loop.Run();
   base::WeakPtr<DownloadDestinationObserver> destination_observer =
       item_->DestinationObserverAsWeakPtr();
 
   ScheduleObservations(PreInitializeFileObservations(), destination_observer);
   RunAllPendingInMessageLoops();
 
   base::RunLoop initialize_completion_loop;
   DownloadItemImplDelegate::DownloadTargetCallback target_callback;
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(_, _))
@@ skipping 48 matching lines @@
   EXPECT_CALL(*file_, RenameAndUniquify(_, _))
       .WillOnce(DoAll(SaveArg<1>(&intermediate_rename_callback),
                       ScheduleClosure(intermediate_rename_loop.QuitClosure())));
 
   base::RunLoop download_start_loop;
   DownloadFile::InitializeCallback initialize_callback;
   EXPECT_CALL(*file_, Initialize(_))
       .WillOnce(DoAll(SaveArg<0>(&initialize_callback),
                       ScheduleClosure(download_start_loop.QuitClosure())));
 
-  item_->Start(std::move(file_), std::move(request_handle_));
+  item_->Start(std::move(file_), std::move(request_handle_), *create_info());
   download_start_loop.Run();
   base::WeakPtr<DownloadDestinationObserver> destination_observer =
       item_->DestinationObserverAsWeakPtr();
 
   ScheduleObservations(PreInitializeFileObservations(), destination_observer);
   RunAllPendingInMessageLoops();
 
   base::RunLoop initialize_completion_loop;
   DownloadItemImplDelegate::DownloadTargetCallback target_callback;
   EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(_, _))
@@ skipping 40 matching lines @@
 
   item_->Cancel(true);
   RunAllPendingInMessageLoops();
 }
 
 TEST(MockDownloadItem, Compiles) {
   MockDownloadItem mock_item;
 }
 
 }  // namespace content