Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(191)

Unified Diff: content/browser/download/download_item_impl_unittest.cc

Issue 11571025: Initial CL for Downloads resumption. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Incorporated Pawel's comment. Created 7 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 0301b8f780957346bfa668d39f2ecfe212f4eecd..0146b1b140908fd60043a5c6d8e27abaf44fdf86 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "base/command_line.h"
#include "base/message_loop.h"
#include "base/stl_util.h"
#include "base/threading/thread.h"
@@ -15,12 +16,15 @@
#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/common/content_switches.h"
#include "content/public/test/mock_download_item.h"
#include "content/public/test/test_browser_thread.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
using ::testing::_;
+using ::testing::NiceMock;
using ::testing::Property;
using ::testing::Return;
using ::testing::SaveArg;
@@ -43,6 +47,10 @@ DownloadId::Domain kValidDownloadItemIdDomain = "valid DownloadId::Domain";
class MockDelegate : public DownloadItemImplDelegate {
public:
+ MockDelegate() : DownloadItemImplDelegate() {
+ SetDefaultExpectations();
+ }
+
MOCK_METHOD2(DetermineDownloadTarget, void(
DownloadItemImpl*, const DownloadTargetCallback&));
MOCK_METHOD2(ShouldCompleteDownload,
@@ -51,12 +59,35 @@ 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(DownloadOpened, void(DownloadItemImpl*));
MOCK_METHOD1(DownloadRemoved, void(DownloadItemImpl*));
MOCK_METHOD1(ShowDownloadInBrowser, void(DownloadItemImpl*));
MOCK_CONST_METHOD1(AssertStateConsistent, void(DownloadItemImpl*));
+
+ void VerifyAndClearExpectations() {
+ ::testing::Mock::VerifyAndClearExpectations(this);
+ SetDefaultExpectations();
+ }
+
+ private:
+ void SetDefaultExpectations() {
+ EXPECT_CALL(*this, AssertStateConsistent(_))
+ .WillRepeatedly(Return());
+ EXPECT_CALL(*this, ShouldOpenFileBasedOnExtension(_))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*this, ShouldOpenDownload(_, _))
+ .WillRepeatedly(Return(true));
+ }
};
class MockRequestHandle : public DownloadRequestHandleInterface {
@@ -89,9 +120,12 @@ class DownloadItemTest : public testing::Test {
public:
explicit MockObserver(DownloadItem* item)
: item_(item),
+ last_state_(item->GetState()),
removed_(false),
destroyed_(false),
- updated_(false) {
+ updated_(false),
+ interrupt_count_(0),
+ resume_count_(0) {
item_->AddObserver(this);
}
@@ -100,17 +134,35 @@ 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;
+ DownloadItem::DownloadState new_state = download->GetState();
+ if (last_state_ == DownloadItem::IN_PROGRESS &&
+ new_state == DownloadItem::INTERRUPTED) {
+ interrupt_count_++;
+ }
+ if (last_state_ == DownloadItem::INTERRUPTED &&
+ new_state == DownloadItem::IN_PROGRESS) {
+ resume_count_++;
+ }
+ last_state_ = new_state;
}
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;
@@ -130,11 +182,22 @@ class DownloadItemTest : public testing::Test {
return was_updated;
}
+ int GetInterruptCount() {
+ return interrupt_count_;
+ }
+
+ int GetResumeCount() {
+ return resume_count_;
+ }
+
private:
DownloadItem* item_;
+ DownloadItem::DownloadState last_state_;
bool removed_;
bool destroyed_;
bool updated_;
+ int interrupt_count_;
+ int resume_count_;
};
DownloadItemTest()
@@ -171,11 +234,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;
}
@@ -196,12 +256,20 @@ 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
// lying around if the above failed.
- ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
+ mock_delegate()->VerifyAndClearExpectations();
+ EXPECT_CALL(*mock_delegate(), AssertStateConsistent(_))
+ .WillRepeatedly(Return());
+ EXPECT_CALL(*mock_delegate(), ShouldOpenFileBasedOnExtension(_))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(_, _))
+ .WillRepeatedly(Return(true));
return mock_download_file;
}
@@ -220,6 +288,7 @@ class DownloadItemTest : public testing::Test {
EXPECT_CALL(*download_file, RenameAndUniquify(intermediate_path, _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
intermediate_path));
+ EXPECT_CALL(*mock_delegate(), ShowDownloadInBrowser(_));
callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
RunAllPendingInMessageLoops();
@@ -227,13 +296,17 @@ 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());
-
- EXPECT_CALL(*download_file, Cancel());
- item->Cancel(true);
- loop_.RunUntilIdle();
+ // 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());
+
+ if (expected_state == DownloadItem::IN_PROGRESS) {
+ EXPECT_CALL(*download_file, Cancel());
+ item->Cancel(true);
+ loop_.RunUntilIdle();
+ }
}
// Destroy a previously created download item.
@@ -254,7 +327,7 @@ class DownloadItemTest : public testing::Test {
MessageLoopForUI loop_;
TestBrowserThread ui_thread_; // UI thread
TestBrowserThread file_thread_; // FILE thread
- testing::NiceMock<MockDelegate> delegate_;
+ StrictMock<MockDelegate> delegate_;
std::set<DownloadItem*> allocated_downloads_;
};
@@ -319,8 +392,11 @@ 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);
+ DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
ASSERT_TRUE(observer.CheckUpdated());
}
@@ -328,6 +404,7 @@ TEST_F(DownloadItemTest, NotificationAfterDelete) {
DownloadItemImpl* item = CreateDownloadItem();
MockDownloadFile* download_file = AddDownloadFileToDownloadItem(item, NULL);
EXPECT_CALL(*download_file, Cancel());
+ EXPECT_CALL(*mock_delegate(), DownloadRemoved(_));
MockObserver observer(item);
item->Delete(DownloadItem::DELETE_DUE_TO_BROWSER_SHUTDOWN);
@@ -342,10 +419,97 @@ TEST_F(DownloadItemTest, NotificationAfterDestroyed) {
ASSERT_TRUE(observer.CheckDestroyed());
}
+TEST_F(DownloadItemTest, ContinueAfterInterrupted) {
+ DownloadItemImpl* item = CreateDownloadItem();
+ MockObserver observer(item);
+ DownloadItemImplDelegate::DownloadTargetCallback callback;
+ MockDownloadFile* download_file = DoIntermediateRename(item);
+
+ // Interrupt the download, using a continuable interrupt.
+ EXPECT_CALL(*download_file, Detach());
+ 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.GetInterruptCount());
+ ASSERT_EQ(0, observer.GetResumeCount());
+ RunAllPendingInMessageLoops();
+
+ CleanupItem(item, download_file, DownloadItem::INTERRUPTED);
+}
+
+// Same as above, but with a non-continuable interrupt.
+TEST_F(DownloadItemTest, RestartAfterInterrupted) {
+ DownloadItemImpl* item = CreateDownloadItem();
+ MockObserver observer(item);
+ DownloadItemImplDelegate::DownloadTargetCallback callback;
+ MockDownloadFile* download_file = DoIntermediateRename(item);
+
+ // Interrupt the download, using a restartable interrupt.
+ EXPECT_CALL(*download_file, Cancel());
+ item->DestinationObserverAsWeakPtr()->DestinationError(
+ DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
+ ASSERT_TRUE(observer.CheckUpdated());
+ // Should not try to auto-resume.
+ ASSERT_EQ(1, observer.GetInterruptCount());
+ ASSERT_EQ(0, observer.GetResumeCount());
+ RunAllPendingInMessageLoops();
+
+ CleanupItem(item, download_file, DownloadItem::INTERRUPTED);
+}
+
+TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
+ CommandLine::ForCurrentProcess()->AppendSwitch(
+ switches::kEnableDownloadResumption);
+
+ DownloadItemImpl* item = CreateDownloadItem();
+ base::WeakPtr<DownloadDestinationObserver> as_observer(
+ item->DestinationObserverAsWeakPtr());
+ MockObserver observer(item);
+ MockDownloadFile* mock_download_file(NULL);
+ scoped_ptr<DownloadFile> download_file;
+ MockRequestHandle* mock_request_handle(NULL);
+ scoped_ptr<DownloadRequestHandleInterface> request_handle;
+
+ EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _))
+ .WillRepeatedly(Return());
+ for (int i = 0; i < (DownloadItemImpl::kMaxAutoResumeAttempts + 1); ++i) {
+ DVLOG(20) << "Loop 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);
+
+ // 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.
+ if (i < (DownloadItemImpl::kMaxAutoResumeAttempts - 1)) {
+ EXPECT_CALL(*mock_request_handle, GetWebContents())
+ .WillOnce(Return(static_cast<WebContents*>(NULL)));
+ }
+
+ item->Start(download_file.Pass(), request_handle.Pass());
+
+ ASSERT_EQ(i, observer.GetResumeCount());
+
+ // Use a continuable interrupt.
+ item->DestinationObserverAsWeakPtr()->DestinationError(
+ DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
+
+ ASSERT_EQ(i + 1, observer.GetInterruptCount());
+ }
+
+ CleanupItem(item, mock_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(_));
MockObserver observer(item);
item->Remove();
@@ -406,6 +570,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnDownloadTargetDetermined) {
EXPECT_CALL(*download_file, RenameAndUniquify(intermediate_path, _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
new_intermediate_path));
+ EXPECT_CALL(*mock_delegate(), ShowDownloadInBrowser(_));
// Currently, a notification would be generated if the danger type is anything
// other than NOT_DANGEROUS.
@@ -416,12 +581,20 @@ 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();
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>);
+
+ EXPECT_CALL(*mock_download_file, Initialize(_));
+ EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(_, _));
+ item->Start(download_file.Pass(), request_handle.Pass());
item->Pause();
ASSERT_TRUE(observer.CheckUpdated());
@@ -430,6 +603,10 @@ TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
item->Resume();
ASSERT_TRUE(observer.CheckUpdated());
+
+ RunAllPendingInMessageLoops();
+
+ CleanupItem(item, mock_download_file, DownloadItem::IN_PROGRESS);
}
TEST_F(DownloadItemTest, DisplayName) {
@@ -446,13 +623,14 @@ TEST_F(DownloadItemTest, DisplayName) {
intermediate_path));
callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+ EXPECT_CALL(*mock_delegate(), ShowDownloadInBrowser(_));
RunAllPendingInMessageLoops();
EXPECT_EQ(FILE_PATH_LITERAL("foo.bar"),
item->GetFileNameToReportUser().value());
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.
@@ -461,9 +639,12 @@ TEST_F(DownloadItemTest, Start) {
scoped_ptr<DownloadFile> download_file(mock_download_file);
DownloadItemImpl* item = CreateDownloadItem();
EXPECT_CALL(*mock_download_file, Initialize(_));
- item->Start(download_file.Pass());
+ scoped_ptr<DownloadRequestHandleInterface> request_handle(
+ new NiceMock<MockRequestHandle>);
+ EXPECT_CALL(*mock_delegate(), DetermineDownloadTarget(item, _));
+ 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.
@@ -486,20 +667,18 @@ TEST_F(DownloadItemTest, CallbackAfterRename) {
RunAllPendingInMessageLoops();
// All the callbacks should have happened by now.
::testing::Mock::VerifyAndClearExpectations(download_file);
- ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
+ mock_delegate()->VerifyAndClearExpectations();
EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _))
.WillOnce(Return(true));
EXPECT_CALL(*download_file, RenameAndAnnotate(final_path, _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
final_path));
- EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
- .WillOnce(Return(true));
EXPECT_CALL(*download_file, Detach());
item->DestinationObserverAsWeakPtr()->DestinationCompleted("");
RunAllPendingInMessageLoops();
::testing::Mock::VerifyAndClearExpectations(download_file);
- ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
+ mock_delegate()->VerifyAndClearExpectations();
}
// Test that the delegate is invoked after the download file is renamed and the
@@ -525,7 +704,7 @@ TEST_F(DownloadItemTest, CallbackAfterInterruptedRename) {
RunAllPendingInMessageLoops();
// All the callbacks should have happened by now.
::testing::Mock::VerifyAndClearExpectations(download_file);
- ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
+ mock_delegate()->VerifyAndClearExpectations();
}
TEST_F(DownloadItemTest, Interrupted) {
@@ -542,9 +721,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());
}
@@ -609,7 +788,7 @@ TEST_F(DownloadItemTest, DestinationError) {
EXPECT_CALL(*download_file, Cancel());
as_observer->DestinationError(
DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
- ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
+ mock_delegate()->VerifyAndClearExpectations();
EXPECT_TRUE(observer.CheckUpdated());
EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED,
@@ -637,7 +816,7 @@ TEST_F(DownloadItemTest, DestinationCompleted) {
EXPECT_FALSE(item->AllDataSaved());
as_observer->DestinationCompleted("livebeef");
- ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
+ mock_delegate()->VerifyAndClearExpectations();
EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
EXPECT_TRUE(observer.CheckUpdated());
EXPECT_EQ("livebeef", item->GetHash());
@@ -661,8 +840,6 @@ TEST_F(DownloadItemTest, EnabledActionsForNormalDownload) {
FilePath(kDummyPath)));
EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _))
.WillOnce(Return(true));
- EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
- .WillOnce(Return(true));
EXPECT_CALL(*download_file, Detach());
item->DestinationObserverAsWeakPtr()->DestinationCompleted("");
RunAllPendingInMessageLoops();
@@ -690,8 +867,6 @@ TEST_F(DownloadItemTest, EnabledActionsForTemporaryDownload) {
EXPECT_CALL(*download_file, RenameAndAnnotate(_, _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
FilePath(kDummyPath)));
- EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
- .WillOnce(Return(true));
EXPECT_CALL(*download_file, Detach());
item->DestinationObserverAsWeakPtr()->DestinationCompleted("");
RunAllPendingInMessageLoops();
« no previous file with comments | « content/browser/download/download_item_impl_delegate.cc ('k') | content/browser/download/download_manager_impl.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698