Download resumption - Preliminary

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 c29c99a83582303efa9bece6285b893b2481f31c..a33c4247528cf3d27a11c7c19055ae212af725d2 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -15,6 +15,7 @@
 #include "content/public/browser/download_id.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/test/mock_download_item.h"
 #include "content/public/test/test_browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -22,6 +23,7 @@
 
 using ::testing::_;
 using ::testing::AllOf;
+using ::testing::NiceMock;
 using ::testing::Property;
 using ::testing::Return;
 using ::testing::SaveArg;
@@ -39,6 +41,14 @@ class MockDelegate : public DownloadItemImplDelegate {
                bool(DownloadItemImpl*, const ShouldOpenDownloadCallback&));
   MOCK_METHOD1(ShouldOpenFileBasedOnExtension, bool(const FilePath&));
   MOCK_METHOD1(CheckForFileRemoval, void(DownloadItemImpl*));
+
+  virtual void ResumeInterruptedDownload(
+      scoped_ptr<DownloadUrlParameters> params, DownloadId id) OVERRIDE {
+    MockResumeInterruptedDownload(params.get(), id);
+  }
+  MOCK_METHOD2(MockResumeInterruptedDownload,
+               void(DownloadUrlParameters* params, DownloadId id));
+
   MOCK_CONST_METHOD0(GetBrowserContext, BrowserContext*());
   MOCK_METHOD1(UpdatePersistence, void(DownloadItemImpl*));
   MOCK_METHOD1(DownloadStopped, void(DownloadItemImpl*));
@@ -56,6 +66,8 @@ class MockRequestHandle : public DownloadRequestHandleInterface {
   MOCK_CONST_METHOD0(PauseRequest, void());
   MOCK_CONST_METHOD0(ResumeRequest, void());
   MOCK_CONST_METHOD0(CancelRequest, void());
+  MOCK_METHOD1(SetRequestId, void(int new_request_id));
+  MOCK_CONST_METHOD0(RequestId, int());
   MOCK_CONST_METHOD0(DebugString, std::string());
 };
 
@@ -80,7 +92,9 @@ class DownloadItemTest : public testing::Test {
       : item_(item),
         removed_(false),
         destroyed_(false),
-        updated_(false) {
+        updated_(false),
+        interrupt_count_(0),
+        resume_count_(0) {
       item_->AddObserver(this);
     }
 
@@ -89,22 +103,42 @@ class DownloadItemTest : public testing::Test {
     }
 
     virtual void OnDownloadRemoved(DownloadItem* download) {
+      DVLOG(20) << " " << __FUNCTION__
+                << " download = " << download->DebugString(false);
       removed_ = true;
     }
 
     virtual void OnDownloadUpdated(DownloadItem* download) {
+      DVLOG(20) << " " << __FUNCTION__
+                << " download = " << download->DebugString(false);
       updated_ = true;
     }
 
     virtual void OnDownloadOpened(DownloadItem* download) {
+      DVLOG(20) << " " << __FUNCTION__
+                << " download = " << download->DebugString(false);
     }
 
     virtual void OnDownloadDestroyed(DownloadItem* download) {
+      DVLOG(20) << " " << __FUNCTION__
+                << " download = " << download->DebugString(false);
       destroyed_ = true;
       item_->RemoveObserver(this);
       item_ = NULL;
     }
 
+    virtual void OnDownloadInterrupted(DownloadItem* download) {
+      DVLOG(20) << " " << __FUNCTION__
+                << " download = " << download->DebugString(false);
+      interrupt_count_++;
+    }
+
+    virtual void OnDownloadResumed(DownloadItem* download) {
+      DVLOG(20) << " " << __FUNCTION__
+                << " download = " << download->DebugString(false);
+      resume_count_++;
+    }
+
     bool CheckRemoved() {
       return removed_;
     }
@@ -119,11 +153,21 @@ class DownloadItemTest : public testing::Test {
       return was_updated;
     }
 
+    int CheckInterrupted() {
+      return interrupt_count_;
+    }
+
+    int CheckResumed() {
+      return resume_count_;
+    }
+
    private:
     DownloadItem* item_;
     bool removed_;
     bool destroyed_;
     bool updated_;
+    int interrupt_count_;
+    int resume_count_;
   };
 
   DownloadItemTest()
@@ -160,11 +204,8 @@ class DownloadItemTest : public testing::Test {
     info_->save_info->prompt_for_save_location = false;
     info_->url_chain.push_back(GURL());
 
-    scoped_ptr<DownloadRequestHandleInterface> request_handle(
-        new testing::NiceMock<MockRequestHandle>);
     DownloadItemImpl* download =
-        new DownloadItemImpl(&delegate_, *(info_.get()),
-                             request_handle.Pass(), net::BoundNetLog());
+        new DownloadItemImpl(&delegate_, *(info_.get()), net::BoundNetLog());
     allocated_downloads_.insert(download);
     return download;
   }
@@ -185,7 +226,9 @@ class DownloadItemTest : public testing::Test {
       EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _));
     }
 
-    item->Start(download_file.Pass());
+    scoped_ptr<DownloadRequestHandleInterface> request_handle(
+        new NiceMock<MockRequestHandle>);
+    item->Start(download_file.Pass(), request_handle.Pass());
     loop_.RunUntilIdle();
 
     // So that we don't have a function writing to a stack variable
@@ -196,9 +239,11 @@ class DownloadItemTest : public testing::Test {
   }
 
   // Cleanup a download item (specifically get rid of the DownloadFile on it).
-  // The item must be in the IN_PROGRESS state.
-  void CleanupItem(DownloadItemImpl* item, MockDownloadFile* download_file) {
-    EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
+  // The item must be in the expected state.
+  void CleanupItem(DownloadItemImpl* item,
+                   MockDownloadFile* download_file,
+                   DownloadItem::DownloadState expected_state) {
+    EXPECT_EQ(expected_state, item->GetState());
 
     EXPECT_CALL(*download_file, Cancel());
     EXPECT_CALL(delegate_, DownloadStopped(item));
@@ -299,6 +344,9 @@ TEST_F(DownloadItemTest, NotificationAfterInterrupted) {
   EXPECT_CALL(*download_file, Cancel());
   MockObserver observer(item);
 
+  EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_,_))
+      .Times(0);
+
   item->DestinationObserverAsWeakPtr()->DestinationError(
       DOWNLOAD_INTERRUPT_REASON_NONE);
   ASSERT_TRUE(observer.CheckUpdated());
@@ -322,6 +370,87 @@ TEST_F(DownloadItemTest, NotificationAfterDestroyed) {
   ASSERT_TRUE(observer.CheckDestroyed());
 }
 
+TEST_F(DownloadItemTest, RestartAfterInterrupted) {
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  MockObserver observer(item);
+  MockDownloadFile* mock_download_file(new NiceMock<MockDownloadFile>);
+  scoped_ptr<DownloadFile> download_file(mock_download_file);
+  scoped_ptr<DownloadRequestHandleInterface> request_handle(
+      new NiceMock<MockRequestHandle>);
+
+  item->Start(download_file.Pass(), request_handle.Pass());
+
+  // Use a continuable interrupt.
+  item->DestinationObserverAsWeakPtr()->DestinationError(
+      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
+  ASSERT_TRUE(observer.CheckUpdated());
+  // Should attempt to auto-resume.  Because we don't have a mock WebContents,
+  // ResumeInterruptedDownload() will abort early, with another interrupt,
+  // which will be ignored.
+  ASSERT_EQ(1, observer.CheckInterrupted());
+  ASSERT_EQ(0, observer.CheckResumed());
+
+  mock_download_file = new NiceMock<MockDownloadFile>;
+  download_file.reset(new MockDownloadFile);
+  request_handle.reset(new NiceMock<MockRequestHandle>);
+
+  item->Start(download_file.Pass(), request_handle.Pass());
+  ASSERT_EQ(1, observer.CheckResumed());
+
+  // Use a non-resumable interrupt.
+  item->DestinationObserverAsWeakPtr()->DestinationError(
+      DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
+  ASSERT_TRUE(observer.CheckUpdated());
+  // Should not try to auto-resume.
+  ASSERT_EQ(2, observer.CheckInterrupted());
+  ASSERT_EQ(1, observer.CheckResumed());
+
+  CleanupItem(item, mock_download_file, DownloadItem::INTERRUPTED);
+}
+
+TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  base::WeakPtr<DownloadDestinationObserver> as_observer(
+      item->DestinationObserverAsWeakPtr());
+  MockObserver observer(item);
+  MockDownloadFile* mock_download_file(new NiceMock<MockDownloadFile>);
+  scoped_ptr<DownloadFile> download_file(mock_download_file);
+  MockRequestHandle* mock_request_handle = new NiceMock<MockRequestHandle>;
+  scoped_ptr<DownloadRequestHandleInterface> request_handle(
+      mock_request_handle);
+
+  item->Start(download_file.Pass(), request_handle.Pass());
+
+  for (int i = 0; i < (DownloadItemImpl::kMaxAutoResumeAttempts + 2); ++i) {
+    DVLOG(20) << "Loop iteration " << i;
+
+    // Use a continuable interrupt.
+    item->DestinationObserverAsWeakPtr()->DestinationError(
+        DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
+
+    mock_download_file = new NiceMock<MockDownloadFile>;
+    download_file.reset(mock_download_file);
+    mock_request_handle = new NiceMock<MockRequestHandle>;
+    request_handle.reset(mock_request_handle);
+
+    // It's too complicated to set up a WebContents instance that would cause
+    // the MockDownloadItemDelegate's ResumeInterruptedDownload() function
+    // to be callled, so we simply verify that GetWebContents() is called.
+    EXPECT_CALL(*mock_request_handle, GetWebContents())
+        .Times((i < (DownloadItemImpl::kMaxAutoResumeAttempts - 1)) ? 1 : 0)
+        .WillRepeatedly(Return(static_cast<WebContents*>(NULL)));
+
+    // Should call delegate->ResumeInterruptedDownload() here, up to
+    // kMaxAutoResumeAttempts times.
+    item->Start(download_file.Pass(), request_handle.Pass());
+
+    ASSERT_EQ(i + 1, observer.CheckInterrupted());
+    ASSERT_EQ(i + 1, observer.CheckResumed());
+  }
+
+  CleanupItem(item, mock_download_file, DownloadItem::IN_PROGRESS);
+}
+
 TEST_F(DownloadItemTest, NotificationAfterRemove) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockDownloadFile* download_file = AddDownloadFileToDownloadItem(item, NULL);
@@ -395,18 +524,26 @@ TEST_F(DownloadItemTest, NotificationAfterOnDownloadTargetDetermined) {
   EXPECT_TRUE(observer.CheckUpdated());
   EXPECT_EQ(new_intermediate_path, item->GetFullPath());
 
-  CleanupItem(item, download_file);
+  CleanupItem(item, download_file, DownloadItem::IN_PROGRESS);
 }
 
 TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
+  MockDownloadFile* mock_download_file(new MockDownloadFile);
+  scoped_ptr<DownloadFile> download_file(mock_download_file);
+  scoped_ptr<DownloadRequestHandleInterface> request_handle(
+      new NiceMock<MockRequestHandle>);
+
+  item->Start(download_file.Pass(), request_handle.Pass());
 
   item->TogglePause();
   ASSERT_TRUE(observer.CheckUpdated());
 
   item->TogglePause();
   ASSERT_TRUE(observer.CheckUpdated());
+
+  CleanupItem(item, mock_download_file, DownloadItem::IN_PROGRESS);
 }
 
 TEST_F(DownloadItemTest, DisplayName) {
@@ -428,7 +565,7 @@ TEST_F(DownloadItemTest, DisplayName) {
   item->SetDisplayName(FilePath(FILE_PATH_LITERAL("new.name")));
   EXPECT_EQ(FILE_PATH_LITERAL("new.name"),
             item->GetFileNameToReportUser().value());
-  CleanupItem(item, download_file);
+  CleanupItem(item, download_file, DownloadItem::IN_PROGRESS);
 }
 
 // Test to make sure that Start method calls DF initialize properly.
@@ -437,9 +574,11 @@ TEST_F(DownloadItemTest, Start) {
   scoped_ptr<DownloadFile> download_file(mock_download_file);
   DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   EXPECT_CALL(*mock_download_file, Initialize(_));
-  item->Start(download_file.Pass());
+  scoped_ptr<DownloadRequestHandleInterface> request_handle(
+      new NiceMock<MockRequestHandle>);
+  item->Start(download_file.Pass(), request_handle.Pass());
 
-  CleanupItem(item, mock_download_file);
+  CleanupItem(item, mock_download_file, DownloadItem::IN_PROGRESS);
 }
 
 // Test that the delegate is invoked after the download file is renamed.
@@ -490,9 +629,9 @@ TEST_F(DownloadItemTest, Interrupted) {
   EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
   EXPECT_EQ(reason, item->GetLastReason());
 
-  // Cancel should result in no change.
+  // Cancel should kill it.
   item->Cancel(true);
-  EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
+  EXPECT_EQ(DownloadItem::CANCELLED, item->GetState());
   EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_USER_CANCELED, item->GetLastReason());
 }