[Downloads] Use final URL for resumption.

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 "base/callback.h"
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
 #include "base/threading/thread.h"
 #include "content/browser/byte_stream.h"
@@ skipping 185 matching lines @@
 
   virtual void TearDown() {
     ui_thread_.DeprecatedGetThreadObject()->message_loop()->RunUntilIdle();
     STLDeleteElements(&allocated_downloads_);
   }
 
   // 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() {
-    // Normally, the download system takes ownership of info, and is
-    // responsible for deleting it.  In these unit tests, however, we
-    // don't call the function that deletes it, so we do so ourselves.
-    scoped_ptr<DownloadCreateInfo> info_;
+    scoped_ptr<DownloadCreateInfo> info;
 
-    info_.reset(new DownloadCreateInfo());
+    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(info.Pass());
+  }
+
+  DownloadItemImpl* CreateDownloadItemWithCreateInfo(
+      scoped_ptr<DownloadCreateInfo> info) {
     static uint32 next_id = DownloadItem::kInvalidId + 1;

[svaldez, 2015/12/15 15:11:50]

nit: Possibly move this out of the CreateDownload and make it part of the
fixture? Otherwise its probably fine here.

[asanka, 2015/12/15 17:32:27]

Done.

-    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";
-
-    DownloadItemImpl* download =
-        new DownloadItemImpl(
-            &delegate_, next_id++, *(info_.get()), net::BoundNetLog());
+    DownloadItemImpl* download = new DownloadItemImpl(
+        &delegate_, next_id++, *(info.get()), net::BoundNetLog());
     allocated_downloads_.insert(download);
     return download;
   }
 
   // Add DownloadFile to DownloadItem
   MockDownloadFile* AddDownloadFileToDownloadItem(
       DownloadItemImpl* item,
       DownloadItemImplDelegate::DownloadTargetCallback *callback) {
     MockDownloadFile* mock_download_file(new StrictMock<MockDownloadFile>);
     scoped_ptr<DownloadFile> download_file(mock_download_file);
@@ skipping 166 matching lines @@
   ASSERT_TRUE(observer.download_destroyed());
 }
 
 TEST_F(DownloadItemTest, ContinueAfterInterrupted) {
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableDownloadResumption);
 
   TestBrowserContext test_browser_context;
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
-  DownloadItemImplDelegate::DownloadTargetCallback callback;
   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));
   EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_, _)).Times(1);
@@ skipping 12 matching lines @@
   CleanupItem(item, download_file, DownloadItem::INTERRUPTED);
 }
 
 // Same as above, but with a non-continuable interrupt.
 TEST_F(DownloadItemTest, RestartAfterInterrupted) {
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableDownloadResumption);
 
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
-  DownloadItemImplDelegate::DownloadTargetCallback callback;
   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);
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
   // Should not try to auto-resume.
   ASSERT_EQ(1, observer.interrupt_count());
   ASSERT_EQ(0, observer.resume_count());
   RunAllPendingInMessageLoops();
 
   CleanupItem(item, download_file, DownloadItem::INTERRUPTED);
 }
 
 // Check we do correct cleanup for RESUME_MODE_INVALID interrupts.
 TEST_F(DownloadItemTest, UnresumableInterrupt) {
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kEnableDownloadResumption);
 
   DownloadItemImpl* item = CreateDownloadItem();
   TestDownloadItemObserver observer(item);
-  DownloadItemImplDelegate::DownloadTargetCallback callback;
   MockDownloadFile* download_file =
       DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
 
   // Fail final rename with unresumable reason.
   EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _))
       .WillOnce(Return(true));
   EXPECT_CALL(*download_file, RenameAndAnnotate(base::FilePath(kDummyPath), _))
       .WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_FILE_BLOCKED,
                                        base::FilePath(kDummyPath)));
   EXPECT_CALL(*download_file, Cancel());
@@ skipping 64 matching lines @@
     item->DestinationObserverAsWeakPtr()->DestinationError(
         DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
 
     ASSERT_EQ(i + 1, observer.interrupt_count());
     ::testing::Mock::VerifyAndClearExpectations(mock_download_file);
   }
 
   CleanupItem(item, mock_download_file, DownloadItem::INTERRUPTED);
 }
 
+// Test that resumption uses the final URL in a URL chain when resuming.
+TEST_F(DownloadItemTest, ResumeUsingFinalURL) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableDownloadResumption);
+
+  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(create_info.Pass());
+  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));
+  EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(
+                                    Property(&DownloadUrlParameters::url,
+                                             GURL("http://example.com/c")),
+                                    _))
+      .Times(1);
+  item->DestinationObserverAsWeakPtr()->DestinationError(
+      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
+  ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
+  ASSERT_EQ(1, 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 resumption was triggered.
+  RunAllPendingInMessageLoops();
+
+  CleanupItem(item, download_file, DownloadItem::INTERRUPTED);
+}
+
 TEST_F(DownloadItemTest, NotificationAfterRemove) {
   DownloadItemImpl* item = CreateDownloadItem();
   MockDownloadFile* download_file = AddDownloadFileToDownloadItem(item, NULL);
   EXPECT_CALL(*download_file, Cancel());
   EXPECT_CALL(*mock_delegate(), DownloadRemoved(_));
   TestDownloadItemObserver observer(item);
 
   item->Remove();
   ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
   ASSERT_TRUE(observer.download_removed());
@@ skipping 726 matching lines @@
                  base::Unretained(&returned_path)));
   RunAllPendingInMessageLoops();
   EXPECT_TRUE(returned_path.empty());
 }
 
 TEST(MockDownloadItem, Compiles) {
   MockDownloadItem mock_item;
 }
 
 }  // namespace content