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

Issues Search
    My Issues | Starred     Open | Closed | All

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

Issue 1691543002: [Downloads] Enforce state transition integrity and state invariants. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address comments Created 4 years, 10 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
« no previous file with comments | « content/browser/download/download_item_impl.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..4af5406c18e77cc9f0b3a8b449580032a38d5213 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -5,11 +5,15 @@
#include "content/browser/download/download_item_impl.h"
#include <stdint.h>
+
+#include <iterator>
+#include <queue>
#include <utility>
#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,26 +22,32 @@
#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/test_browser_context.h"
-#include "content/public/test/test_browser_thread.h"
+#include "content/public/test/test_browser_thread_bundle.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
-using ::testing::_;
+using ::testing::DoAll;
using ::testing::NiceMock;
using ::testing::Property;
using ::testing::Return;
using ::testing::SaveArg;
using ::testing::StrictMock;
+using ::testing::WithArg;
+using ::testing::_;
const int kDownloadChunkSize = 1000;
const int kDownloadSpeed = 1000;
-const base::FilePath::CharType kDummyPath[] = FILE_PATH_LITERAL("/testpath");
+const base::FilePath::CharType kDummyTargetPath[] =
+ FILE_PATH_LITERAL("/testpath");
+const base::FilePath::CharType kDummyIntermediatePath[] =
+ FILE_PATH_LITERAL("/testpathx");
namespace content {
@@ -159,7 +169,7 @@ class TestDownloadItemObserver : public DownloadItem::Observer {
<< " download = " << download->DebugString(false);
destroyed_ = true;
item_->RemoveObserver(this);
- item_ = NULL;
+ item_ = nullptr;
}
DownloadItem* item_;
@@ -182,20 +192,28 @@ ACTION_P2(ScheduleRenameCallback, interrupt_reason, new_path) {
base::Bind(arg1, interrupt_reason, new_path));
}
+// Schedules a task to invoke a callback that's bound to the specified
+// parameter.
+// E.g.:
+//
+// EXPECT_CALL(foo, Bar(1, _))
+// .WithArg<1>(ScheduleCallbackWithParam(0));
+//
+// .. will invoke the second argument to Bar with 0 as the parameter.
+ACTION_P(ScheduleCallbackWithParam, param) {
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+ base::Bind(arg0, param));
+}
+
+ACTION_P(ScheduleClosure, closure) {
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, closure);
+}
+
} // namespace
class DownloadItemTest : public testing::Test {
public:
- DownloadItemTest()
- : ui_thread_(BrowserThread::UI, &loop_),
- file_thread_(BrowserThread::FILE, &loop_),
- delegate_() {
- }
-
- ~DownloadItemTest() {
- }
-
- virtual void SetUp() {
+ DownloadItemTest() {
base::FeatureList::ClearInstanceForTesting();
scoped_ptr<base::FeatureList> feature_list(new base::FeatureList);
feature_list->InitializeFromCommandLine(features::kDownloadResumption.name,
@@ -203,11 +221,19 @@ class DownloadItemTest : public testing::Test {
base::FeatureList::SetInstance(std::move(feature_list));
}
- virtual void TearDown() {
- ui_thread_.DeprecatedGetThreadObject()->message_loop()->RunUntilIdle();
+ ~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.
@@ -217,24 +243,16 @@ class DownloadItemTest : public testing::Test {
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->url_chain.push_back(GURL("http://example.com/download"));
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());
- allocated_downloads_.insert(download);
- return download;
- }
-
// Add DownloadFile to DownloadItem
- MockDownloadFile* AddDownloadFileToDownloadItem(
+ MockDownloadFile* CallDownloadItemStart(
DownloadItemImpl* item,
- DownloadItemImplDelegate::DownloadTargetCallback *callback) {
+ DownloadItemImplDelegate::DownloadTargetCallback* callback) {
MockDownloadFile* mock_download_file(new StrictMock<MockDownloadFile>);
scoped_ptr<DownloadFile> download_file(mock_download_file);
EXPECT_CALL(*mock_download_file, Initialize(_));
@@ -250,7 +268,7 @@ class DownloadItemTest : public testing::Test {
scoped_ptr<DownloadRequestHandleInterface> request_handle(
new NiceMock<MockRequestHandle>);
item->Start(std::move(download_file), std::move(request_handle));
- loop_.RunUntilIdle();
+ RunAllPendingInMessageLoops();
// So that we don't have a function writing to a stack variable
// lying around if the above failed.
@@ -266,18 +284,16 @@ class DownloadItemTest : public testing::Test {
}
// Perform the intermediate rename for |item|. The target path for the
- // download will be set to kDummyPath. Returns the MockDownloadFile* that was
- // added to the DownloadItem.
+ // download will be set to kDummyTargetPath. Returns the MockDownloadFile*
+ // that was added to the DownloadItem.
MockDownloadFile* DoIntermediateRename(DownloadItemImpl* item,
DownloadDangerType danger_type) {
EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
EXPECT_TRUE(item->GetTargetFilePath().empty());
DownloadItemImplDelegate::DownloadTargetCallback callback;
- MockDownloadFile* download_file =
- AddDownloadFileToDownloadItem(item, &callback);
- base::FilePath target_path(kDummyPath);
- base::FilePath intermediate_path(
- target_path.InsertBeforeExtensionASCII("x"));
+ MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
+ base::FilePath target_path(kDummyTargetPath);
+ base::FilePath intermediate_path(kDummyIntermediatePath);
EXPECT_CALL(*download_file, RenameAndUniquify(intermediate_path, _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
intermediate_path));
@@ -287,6 +303,22 @@ class DownloadItemTest : public testing::Test {
return download_file;
}
+ void DoDestinationComplete(DownloadItemImpl* item,
+ MockDownloadFile* download_file) {
+ EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(_, _))
+ .WillOnce(Return(true));
+ base::FilePath final_path(kDummyTargetPath);
+ EXPECT_CALL(*download_file, RenameAndAnnotate(_, _))
+ .WillOnce(
+ ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE, final_path));
+ EXPECT_CALL(*download_file, FullPath())
+ .WillRepeatedly(Return(base::FilePath(kDummyTargetPath)));
+ EXPECT_CALL(*download_file, Detach());
+
+ item->DestinationObserverAsWeakPtr()->DestinationCompleted("");
+ RunAllPendingInMessageLoops();
+ }
+
// Cleanup a download item (specifically get rid of the DownloadFile on it).
// The item must be in the expected state.
void CleanupItem(DownloadItemImpl* item,
@@ -298,7 +330,7 @@ class DownloadItemTest : public testing::Test {
if (download_file)
EXPECT_CALL(*download_file, Cancel());
item->Cancel(true);
- loop_.RunUntilIdle();
+ RunAllPendingInMessageLoops();
}
}
@@ -308,9 +340,7 @@ class DownloadItemTest : public testing::Test {
delete item;
}
- void RunAllPendingInMessageLoops() {
- loop_.RunUntilIdle();
- }
+ void RunAllPendingInMessageLoops() { base::RunLoop().RunUntilIdle(); }
MockDelegate* mock_delegate() {
return &delegate_;
@@ -323,11 +353,9 @@ class DownloadItemTest : public testing::Test {
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_;
+ TestBrowserThreadBundle thread_bundle_;
};
// Tests to ensure calls that change a DownloadItem generate an update to
@@ -340,17 +368,20 @@ class DownloadItemTest : public testing::Test {
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 =
- AddDownloadFileToDownloadItem(user_cancel, NULL);
+ MockDownloadFile* download_file = CallDownloadItemStart(user_cancel, nullptr);
EXPECT_CALL(*download_file, Cancel());
TestDownloadItemObserver observer1(user_cancel);
@@ -358,7 +389,7 @@ TEST_F(DownloadItemTest, NotificationAfterCancel) {
ASSERT_TRUE(observer1.CheckAndResetDownloadUpdated());
DownloadItemImpl* system_cancel = CreateDownloadItem();
- download_file = AddDownloadFileToDownloadItem(system_cancel, NULL);
+ download_file = CallDownloadItemStart(system_cancel, nullptr);
EXPECT_CALL(*download_file, Cancel());
TestDownloadItemObserver observer2(system_cancel);
@@ -369,11 +400,10 @@ TEST_F(DownloadItemTest, NotificationAfterCancel) {
TEST_F(DownloadItemTest, NotificationAfterComplete) {
DownloadItemImpl* item = CreateDownloadItem();
TestDownloadItemObserver observer(item);
-
- item->OnAllDataSaved(DownloadItem::kEmptyFileHash);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
-
- item->MarkAsComplete();
+ DoDestinationComplete(item, download_file);
ASSERT_TRUE(observer.CheckAndResetDownloadUpdated());
}
@@ -471,9 +501,10 @@ TEST_F(DownloadItemTest, UnresumableInterrupt) {
// Fail final rename with unresumable reason.
EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _))
.WillOnce(Return(true));
- EXPECT_CALL(*download_file, RenameAndAnnotate(base::FilePath(kDummyPath), _))
+ EXPECT_CALL(*download_file,
+ RenameAndAnnotate(base::FilePath(kDummyTargetPath), _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_FILE_BLOCKED,
- base::FilePath(kDummyPath)));
+ base::FilePath()));
EXPECT_CALL(*download_file, Cancel());
// Complete download to trigger final rename.
@@ -494,9 +525,9 @@ TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
base::WeakPtr<DownloadDestinationObserver> as_observer(
item->DestinationObserverAsWeakPtr());
TestDownloadItemObserver observer(item);
- MockDownloadFile* mock_download_file(NULL);
+ MockDownloadFile* mock_download_file(nullptr);
scoped_ptr<DownloadFile> download_file;
- MockRequestHandle* mock_request_handle(NULL);
+ MockRequestHandle* mock_request_handle(nullptr);
scoped_ptr<DownloadRequestHandleInterface> request_handle;
DownloadItemImplDelegate::DownloadTargetCallback callback;
@@ -517,22 +548,24 @@ TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
ON_CALL(*mock_download_file, FullPath())
.WillByDefault(Return(base::FilePath()));
- // Copied key parts of DoIntermediateRename & AddDownloadFileToDownloadItem
+ // Copied key parts of DoIntermediateRename & CallDownloadItemStart
// to allow for holding onto the request handle.
item->Start(std::move(download_file), std::move(request_handle));
RunAllPendingInMessageLoops();
+
+ base::FilePath target_path(kDummyTargetPath);
+ base::FilePath intermediate_path(kDummyIntermediatePath);
if (i == 0) {
- // Target determination is only done the first time through.
- base::FilePath target_path(kDummyPath);
- base::FilePath intermediate_path(
- target_path.InsertBeforeExtensionASCII("x"));
+ // 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();
}
+ callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+ DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+ RunAllPendingInMessageLoops();
// Use a continuable interrupt.
item->DestinationObserverAsWeakPtr()->DestinationError(
@@ -541,6 +574,7 @@ TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
::testing::Mock::VerifyAndClearExpectations(mock_download_file);
}
+ EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
EXPECT_EQ(1, observer.interrupt_count());
CleanupItem(item, nullptr, DownloadItem::INTERRUPTED);
}
@@ -588,7 +622,7 @@ TEST_F(DownloadItemTest, ResumeUsingFinalURL) {
TEST_F(DownloadItemTest, NotificationAfterRemove) {
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = AddDownloadFileToDownloadItem(item, NULL);
+ MockDownloadFile* download_file = CallDownloadItemStart(item, nullptr);
EXPECT_CALL(*download_file, Cancel());
EXPECT_CALL(*mock_delegate(), DownloadRemoved(_));
TestDownloadItemObserver observer(item);
@@ -601,16 +635,21 @@ TEST_F(DownloadItemTest, NotificationAfterRemove) {
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);
EXPECT_TRUE(safe_observer.CheckAndResetDownloadUpdated());
+ CleanupItem(safe_item, download_file, DownloadItem::IN_PROGRESS);
// Setting to unsafe url or unsafe file should trigger a notification.
DownloadItemImpl* unsafeurl_item =
CreateDownloadItem();
+ download_file =
+ DoIntermediateRename(unsafeurl_item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
TestDownloadItemObserver unsafeurl_observer(unsafeurl_item);
unsafeurl_item->OnAllDataSaved(std::string());
@@ -618,11 +657,17 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
unsafeurl_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_DANGEROUS_URL);
EXPECT_TRUE(unsafeurl_observer.CheckAndResetDownloadUpdated());
+ EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(_, _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*download_file, RenameAndAnnotate(_, _));
unsafeurl_item->ValidateDangerousDownload();
EXPECT_TRUE(unsafeurl_observer.CheckAndResetDownloadUpdated());
+ CleanupItem(unsafeurl_item, download_file, DownloadItem::IN_PROGRESS);
DownloadItemImpl* unsafefile_item =
CreateDownloadItem();
+ download_file =
+ DoIntermediateRename(unsafefile_item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
TestDownloadItemObserver unsafefile_observer(unsafefile_item);
unsafefile_item->OnAllDataSaved(std::string());
@@ -630,8 +675,12 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
unsafefile_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE);
EXPECT_TRUE(unsafefile_observer.CheckAndResetDownloadUpdated());
+ EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(_, _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*download_file, RenameAndAnnotate(_, _));
unsafefile_item->ValidateDangerousDownload();
EXPECT_TRUE(unsafefile_observer.CheckAndResetDownloadUpdated());
+ CleanupItem(unsafefile_item, download_file, DownloadItem::IN_PROGRESS);
}
// DownloadItemImpl::OnDownloadTargetDetermined will schedule a task to run
@@ -642,10 +691,9 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
TEST_F(DownloadItemTest, NotificationAfterOnDownloadTargetDetermined) {
DownloadItemImpl* item = CreateDownloadItem();
DownloadItemImplDelegate::DownloadTargetCallback callback;
- MockDownloadFile* download_file =
- AddDownloadFileToDownloadItem(item, &callback);
+ MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
TestDownloadItemObserver observer(item);
- base::FilePath target_path(kDummyPath);
+ base::FilePath target_path(kDummyTargetPath);
base::FilePath intermediate_path(target_path.InsertBeforeExtensionASCII("x"));
base::FilePath new_intermediate_path(
target_path.InsertBeforeExtensionASCII("y"));
@@ -693,9 +741,9 @@ TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
TEST_F(DownloadItemTest, DisplayName) {
DownloadItemImpl* item = CreateDownloadItem();
DownloadItemImplDelegate::DownloadTargetCallback callback;
- MockDownloadFile* download_file =
- AddDownloadFileToDownloadItem(item, &callback);
- base::FilePath target_path(base::FilePath(kDummyPath).AppendASCII("foo.bar"));
+ MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
+ base::FilePath target_path(
+ base::FilePath(kDummyTargetPath).AppendASCII("foo.bar"));
base::FilePath intermediate_path(target_path.InsertBeforeExtensionASCII("x"));
EXPECT_EQ(FILE_PATH_LITERAL(""),
item->GetFileNameToReportUser().value());
@@ -728,13 +776,33 @@ TEST_F(DownloadItemTest, Start) {
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) {
+ 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));
+ RunAllPendingInMessageLoops();
+
+ EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
+ EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED,
+ item->GetLastReason());
+}
+
// 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 =
- AddDownloadFileToDownloadItem(item, &callback);
- base::FilePath final_path(base::FilePath(kDummyPath).AppendASCII("foo.bar"));
+ 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"));
@@ -768,9 +836,9 @@ TEST_F(DownloadItemTest, CallbackAfterRename) {
TEST_F(DownloadItemTest, CallbackAfterInterruptedRename) {
DownloadItemImpl* item = CreateDownloadItem();
DownloadItemImplDelegate::DownloadTargetCallback callback;
- MockDownloadFile* download_file =
- AddDownloadFileToDownloadItem(item, &callback);
- base::FilePath final_path(base::FilePath(kDummyPath).AppendASCII("foo.bar"));
+ 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"));
@@ -814,13 +882,13 @@ TEST_F(DownloadItemTest, Interrupted) {
TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Restart) {
DownloadItemImpl* item = CreateDownloadItem();
DownloadItemImplDelegate::DownloadTargetCallback callback;
- MockDownloadFile* download_file =
- AddDownloadFileToDownloadItem(item, &callback);
+ MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
item->DestinationObserverAsWeakPtr()->DestinationError(
DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
ASSERT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
- base::FilePath final_path(base::FilePath(kDummyPath).AppendASCII("foo.bar"));
+ 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"));
@@ -847,13 +915,13 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Restart) {
TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Continue) {
DownloadItemImpl* item = CreateDownloadItem();
DownloadItemImplDelegate::DownloadTargetCallback callback;
- MockDownloadFile* download_file =
- AddDownloadFileToDownloadItem(item, &callback);
+ MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
item->DestinationObserverAsWeakPtr()->DestinationError(
DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED);
ASSERT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
- base::FilePath final_path(base::FilePath(kDummyPath).AppendASCII("foo.bar"));
+ 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"));
@@ -880,13 +948,13 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Continue) {
TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Failed) {
DownloadItemImpl* item = CreateDownloadItem();
DownloadItemImplDelegate::DownloadTargetCallback callback;
- MockDownloadFile* download_file =
- AddDownloadFileToDownloadItem(item, &callback);
+ MockDownloadFile* download_file = CallDownloadItemStart(item, &callback);
item->DestinationObserverAsWeakPtr()->DestinationError(
DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED);
ASSERT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
- base::FilePath final_path(base::FilePath(kDummyPath).AppendASCII("foo.bar"));
+ 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"));
@@ -910,7 +978,7 @@ TEST_F(DownloadItemTest, InterruptedBeforeIntermediateRename_Failed) {
TEST_F(DownloadItemTest, Canceled) {
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = AddDownloadFileToDownloadItem(item, NULL);
+ MockDownloadFile* download_file = CallDownloadItemStart(item, nullptr);
// Confirm cancel sets state properly.
EXPECT_CALL(*download_file, Cancel());
@@ -928,6 +996,8 @@ TEST_F(DownloadItemTest, FileRemoved) {
TEST_F(DownloadItemTest, DestinationUpdate) {
DownloadItemImpl* item = CreateDownloadItem();
+ MockDownloadFile* file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
base::WeakPtr<DownloadDestinationObserver> as_observer(
item->DestinationObserverAsWeakPtr());
TestDownloadItemObserver observer(item);
@@ -953,6 +1023,8 @@ TEST_F(DownloadItemTest, DestinationUpdate) {
EXPECT_EQ(200l, item->GetReceivedBytes());
EXPECT_EQ(0l, item->GetTotalBytes());
EXPECT_TRUE(observer.CheckAndResetDownloadUpdated());
+
+ CleanupItem(item, file, DownloadItem::IN_PROGRESS);
}
TEST_F(DownloadItemTest, DestinationError) {
@@ -979,6 +1051,7 @@ TEST_F(DownloadItemTest, DestinationError) {
TEST_F(DownloadItemTest, DestinationCompleted) {
DownloadItemImpl* item = CreateDownloadItem();
+ MockDownloadFile* download_file = CallDownloadItemStart(item, nullptr);
base::WeakPtr<DownloadDestinationObserver> as_observer(
item->DestinationObserverAsWeakPtr());
TestDownloadItemObserver observer(item);
@@ -1004,6 +1077,11 @@ TEST_F(DownloadItemTest, DestinationCompleted) {
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
+ // TARGET_PENDING.
+ CleanupItem(item, download_file, DownloadItem::IN_PROGRESS);
}
TEST_F(DownloadItemTest, EnabledActionsForNormalDownload) {
@@ -1020,7 +1098,7 @@ TEST_F(DownloadItemTest, EnabledActionsForNormalDownload) {
// Complete
EXPECT_CALL(*download_file, RenameAndAnnotate(_, _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
- base::FilePath(kDummyPath)));
+ base::FilePath(kDummyTargetPath)));
EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _))
.WillOnce(Return(true));
EXPECT_CALL(*download_file, FullPath())
@@ -1052,7 +1130,7 @@ TEST_F(DownloadItemTest, EnabledActionsForTemporaryDownload) {
.WillOnce(Return(true));
EXPECT_CALL(*download_file, RenameAndAnnotate(_, _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
- base::FilePath(kDummyPath)));
+ base::FilePath(kDummyTargetPath)));
EXPECT_CALL(*download_file, FullPath())
.WillOnce(Return(base::FilePath()));
EXPECT_CALL(*download_file, Detach());
@@ -1077,7 +1155,7 @@ TEST_F(DownloadItemTest, EnabledActionsForInterruptedDownload) {
ASSERT_EQ(DownloadItem::INTERRUPTED, item->GetState());
ASSERT_FALSE(item->GetTargetFilePath().empty());
EXPECT_FALSE(item->CanShowInFolder());
- EXPECT_FALSE(item->CanOpenDownload());
+ EXPECT_TRUE(item->CanOpenDownload());
}
TEST_F(DownloadItemTest, EnabledActionsForCancelledDownload) {
@@ -1112,9 +1190,10 @@ TEST_F(DownloadItemTest, CompleteDelegate_ReturnTrue) {
EXPECT_FALSE(item->IsDangerous());
// Make sure the download can complete.
- EXPECT_CALL(*download_file, RenameAndAnnotate(base::FilePath(kDummyPath), _))
+ EXPECT_CALL(*download_file,
+ RenameAndAnnotate(base::FilePath(kDummyTargetPath), _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
- base::FilePath(kDummyPath)));
+ base::FilePath(kDummyTargetPath)));
EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
.WillOnce(Return(true));
EXPECT_CALL(*download_file, FullPath())
@@ -1150,9 +1229,10 @@ TEST_F(DownloadItemTest, CompleteDelegate_BlockOnce) {
EXPECT_FALSE(item->IsDangerous());
// Make sure the download can complete.
- EXPECT_CALL(*download_file, RenameAndAnnotate(base::FilePath(kDummyPath), _))
+ EXPECT_CALL(*download_file,
+ RenameAndAnnotate(base::FilePath(kDummyTargetPath), _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
- base::FilePath(kDummyPath)));
+ base::FilePath(kDummyTargetPath)));
EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
.WillOnce(Return(true));
EXPECT_CALL(*download_file, FullPath())
@@ -1191,9 +1271,10 @@ TEST_F(DownloadItemTest, CompleteDelegate_SetDanger) {
EXPECT_TRUE(item->IsDangerous());
// Make sure the download doesn't complete until we've validated it.
- EXPECT_CALL(*download_file, RenameAndAnnotate(base::FilePath(kDummyPath), _))
+ EXPECT_CALL(*download_file,
+ RenameAndAnnotate(base::FilePath(kDummyTargetPath), _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
- base::FilePath(kDummyPath)));
+ base::FilePath(kDummyTargetPath)));
EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
.WillOnce(Return(true));
EXPECT_CALL(*download_file, FullPath())
@@ -1242,9 +1323,10 @@ TEST_F(DownloadItemTest, CompleteDelegate_BlockTwice) {
EXPECT_FALSE(item->IsDangerous());
// Make sure the download can complete.
- EXPECT_CALL(*download_file, RenameAndAnnotate(base::FilePath(kDummyPath), _))
+ EXPECT_CALL(*download_file,
+ RenameAndAnnotate(base::FilePath(kDummyTargetPath), _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
- base::FilePath(kDummyPath)));
+ base::FilePath(kDummyTargetPath)));
EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
.WillOnce(Return(true));
EXPECT_CALL(*download_file, FullPath())
@@ -1319,6 +1401,430 @@ TEST_F(DownloadItemTest, StealInterruptedNonResumableDangerousDownload) {
EXPECT_TRUE(returned_path.empty());
}
+namespace {
+
+// The DownloadItemDestinationUpdateRaceTest fixture (defined below) is used to
+// test for race conditions between download destination events received via the
+// DownloadDestinationObserver interface, and the target determination logic.
+//
+// The general control flow for DownloadItemImpl looks like this:
+//
+// * Start() called, which in turn calls DownloadFile::Initialize().
+//
+// Even though OnDownloadFileInitialized hasn't been called, there could now
+// be destination observer calls queued prior to the task that calls
+// OnDownloadFileInitialized. Let's call this point in the workflow "A".
+//
+// * DownloadItemImpl::OnDownloadFileInitialized() called.
+//
+// * Assuming the result is successful, DII now invokes the delegate's
+// DetermineDownloadTarget method.
+//
+// At this point DonwnloadFile acts as the source of
+// DownloadDestinationObserver events, and may invoke callbacks. Let's call
+// this point in the workflow "B".
+//
+// * DII::OnDownloadTargetDetermined() invoked after delegate is done with
+// target determination.
+//
+// * DII attempts to rename the DownloadFile to its intermediate name.
+//
+// More DownloadDestinationObserver events can happen here. Let's call this
+// point in the workflow "C".
+//
+// * DII::OnDownloadRenamedToIntermediateName() invoked. Assuming all went well,
+// DII is now in IN_PROGRESS state.
+//
+// More DownloadDestinationObserver events can happen here. Let's call this
+// point in the workflow "D".
+//
+// The DownloadItemDestinationUpdateRaceTest works by generating various
+// combinations of DownloadDestinationObserver events that might occur at the
+// points "A", "B", "C", and "D" above. Each test in this suite cranks a
+// DownloadItemImpl through the states listed above and invokes the events
+// assigned to each position.
+
+// This type of callback represents a call to a DownloadDestinationObserver
+// method that's missing the DownloadDestinationObserver object. Currying this
+// way allows us to bind a call prior to constructing the object on which the
+// method would be invoked. This is necessary since we are going to construct
+// various permutations of observer calls that will then be applied to a
+// DownloadItem in a state as yet undetermined.
+using CurriedObservation =
+ base::Callback<void(base::WeakPtr<DownloadDestinationObserver>)>;
+
+// A list of observations that are to be made during some event in the
+// DownloadItemImpl control flow. Ordering of the observations is significant.
+using ObservationList = std::deque<CurriedObservation>;
+
+// An ordered list of events.
+//
+// An "event" in this context refers to some stage in the DownloadItemImpl's
+// workflow described as "A", "B", "C", or "D" above. An EventList is expected
+// to always contains kEventCount events.
+using EventList = std::deque<ObservationList>;
+
+// Number of events in an EventList. This is always 4 for now as described
+// above.
+const int kEventCount = 4;
+
+// The following functions help us with currying the calls to
+// DownloadDestinationObserver. If std::bind was allowed along with
+// std::placeholders, it is possible to avoid these functions, but currently
+// Chromium doesn't allow using std::bind for good reasons.
+void DestinationUpdateInvoker(
+ int64_t bytes_so_far,
+ int64_t bytes_per_sec,
+ const std::string& hash_state,
+ base::WeakPtr<DownloadDestinationObserver> observer) {
+ DVLOG(20) << "DestinationUpdate(bytes_so_far:" << bytes_so_far
+ << ", bytes_per_sec:" << bytes_per_sec
+ << ", hash_state:" << hash_state << ") observer:" << !!observer;
+ if (observer)
+ observer->DestinationUpdate(bytes_so_far, bytes_per_sec, hash_state);
+}
+
+void DestinationErrorInvoker(
+ DownloadInterruptReason reason,
+ base::WeakPtr<DownloadDestinationObserver> observer) {
+ DVLOG(20) << "DestinationError(reason:"
+ << DownloadInterruptReasonToString(reason)
+ << ") observer:" << !!observer;
+ if (observer)
+ observer->DestinationError(reason);
+}
+
+void DestinationCompletedInvoker(
+ const std::string& final_hash,
+ base::WeakPtr<DownloadDestinationObserver> observer) {
+ DVLOG(20) << "DestinationComplete(final_hash:" << final_hash
+ << ") observer:" << !!observer;
+ if (observer)
+ observer->DestinationCompleted(final_hash);
+}
+
+// Given a set of observations (via the range |begin|..|end|), constructs a list
+// of EventLists such that:
+//
+// * There are exactly |event_count| ObservationSets in each EventList.
+//
+// * Each ObservationList in each EventList contains a subrange (possibly empty)
+// of observations from the input range, in the same order as the input range.
+//
+// * The ordering of the ObservationList in each EventList is such that all
+// observations in one ObservationList occur earlier than all observations in
+// an ObservationList that follows it.
+//
+// * The list of EventLists together describe all the possible ways in which the
+// list of observations can be distributed into |event_count| events.
+std::vector<EventList> DistributeObservationsIntoEvents(
+ const std::vector<CurriedObservation>::iterator begin,
+ const std::vector<CurriedObservation>::iterator end,
+ int event_count) {
+ std::vector<EventList> all_event_lists;
+ for (auto partition = begin;; ++partition) {
+ ObservationList first_group_of_observations(begin, partition);
+ if (event_count > 1) {
+ std::vector<EventList> list_of_subsequent_events =
+ DistributeObservationsIntoEvents(partition, end, event_count - 1);
+ for (const auto& subsequent_events : list_of_subsequent_events) {
+ EventList event_list;
+ event_list = subsequent_events;
+ event_list.push_front(first_group_of_observations);
+ all_event_lists.push_back(event_list);
+ }
+ } else {
+ EventList event_list;
+ event_list.push_front(first_group_of_observations);
+ all_event_lists.push_back(event_list);
+ }
+ if (partition == end)
+ break;
+ }
+ return all_event_lists;
+}
+
+// For the purpose of this tests, we are only concerned with 3 events:
+//
+// 1. Immediately after the DownloadFile is initialized.
+// 2. Immediately after the DownloadTargetCallback is invoked.
+// 3. Immediately after the intermediate file is renamed.
+//
+// We are going to take a couple of sets of DownloadDestinationObserver events
+// and distribute them into the three events described above. And then we are
+// going to invoke the observations while a DownloadItemImpl is carefully
+// stepped through its stages.
+
+std::vector<EventList> GenerateSuccessfulEventLists() {
+ std::vector<CurriedObservation> all_observations;
+ all_observations.push_back(
+ base::Bind(&DestinationUpdateInvoker, 100, 100, "abc"));
+ all_observations.push_back(
+ base::Bind(&DestinationUpdateInvoker, 200, 100, "def"));
+ all_observations.push_back(
+ base::Bind(&DestinationCompletedInvoker, "final-hash-1"));
+ return DistributeObservationsIntoEvents(all_observations.begin(),
+ all_observations.end(), kEventCount);
+}
+
+std::vector<EventList> GenerateFailingEventLists() {
+ std::vector<CurriedObservation> all_observations;
+ all_observations.push_back(
+ base::Bind(&DestinationUpdateInvoker, 100, 100, "abc"));
+ all_observations.push_back(base::Bind(
+ &DestinationErrorInvoker, DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED));
+ return DistributeObservationsIntoEvents(all_observations.begin(),
+ all_observations.end(), kEventCount);
+}
+
+class DownloadItemDestinationUpdateRaceTest
+ : public DownloadItemTest,
+ public ::testing::WithParamInterface<EventList> {
+ public:
+ DownloadItemDestinationUpdateRaceTest()
+ : DownloadItemTest(),
+ item_(CreateDownloadItem()),
+ file_(new ::testing::StrictMock<MockDownloadFile>()),
+ request_handle_(new ::testing::StrictMock<MockRequestHandle>()) {
+ DCHECK_EQ(GetParam().size(), static_cast<unsigned>(kEventCount));
+ EXPECT_CALL(*request_handle_, GetWebContents())
+ .WillRepeatedly(Return(nullptr));
+ }
+
+ protected:
+ const ObservationList& PreInitializeFileObservations() {
+ return GetParam().front();
+ }
+ const ObservationList& PostInitializeFileObservations() {
+ return *(GetParam().begin() + 1);
+ }
+ const ObservationList& PostTargetDeterminationObservations() {
+ return *(GetParam().begin() + 2);
+ }
+ const ObservationList& PostIntermediateRenameObservations() {
+ return *(GetParam().begin() + 3);
+ }
+
+ // Apply all the observations in |observations| to |observer|, but do so
+ // asynchronously so that the events are applied in order behind any tasks
+ // that are already scheduled.
+ void ScheduleObservations(
+ const ObservationList& observations,
+ base::WeakPtr<DownloadDestinationObserver> observer) {
+ for (const auto action : observations)
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+ base::Bind(action, observer));
+ }
+
+ DownloadItemImpl* item_;
+ scoped_ptr<MockDownloadFile> file_;
+ scoped_ptr<MockRequestHandle> request_handle_;
+
+ std::queue<base::Closure> successful_update_events_;
+ std::queue<base::Closure> failing_update_events_;
+};
+
+INSTANTIATE_TEST_CASE_P(Success,
+ DownloadItemDestinationUpdateRaceTest,
+ ::testing::ValuesIn(GenerateSuccessfulEventLists()));
+
+INSTANTIATE_TEST_CASE_P(Failure,
+ DownloadItemDestinationUpdateRaceTest,
+ ::testing::ValuesIn(GenerateFailingEventLists()));
+
+} // namespace
+
+// Run through the DII workflow but the embedder cancels the download at target
+// determination.
+TEST_P(DownloadItemDestinationUpdateRaceTest, DownloadCancelledByUser) {
+ // 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_));
+ 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(_, _))
+ .WillOnce(
+ DoAll(SaveArg<1>(&target_callback),
+ ScheduleClosure(initialize_completion_loop.QuitClosure())));
+ ScheduleObservations(PostInitializeFileObservations(), destination_observer);
+ initialize_callback.Run(DOWNLOAD_INTERRUPT_REASON_NONE);
+ initialize_completion_loop.Run();
+
+ RunAllPendingInMessageLoops();
+
+ ASSERT_FALSE(target_callback.is_null());
+ ScheduleObservations(PostTargetDeterminationObservations(),
+ destination_observer);
+ target_callback.Run(base::FilePath(),
+ DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+ DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, base::FilePath());
+ EXPECT_EQ(DownloadItem::CANCELLED, item_->GetState());
+ RunAllPendingInMessageLoops();
+}
+
+// Run through the DII workflow, but the intermediate rename fails.
+TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameFails) {
+ // Expect that the download file and the request will be cancelled as a
+ // result.
+ EXPECT_CALL(*file_, Cancel());
+ EXPECT_CALL(*request_handle_, CancelRequest());
+
+ // Intermediate rename loop is not used immediately, but let's set up the
+ // DownloadFile expectations since we are about to transfer its ownership to
+ // the DownloadItem.
+ base::RunLoop intermediate_rename_loop;
+ DownloadFile::RenameCompletionCallback intermediate_rename_callback;
+ 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_));
+ 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(_, _))
+ .WillOnce(
+ DoAll(SaveArg<1>(&target_callback),
+ ScheduleClosure(initialize_completion_loop.QuitClosure())));
+ ScheduleObservations(PostInitializeFileObservations(), destination_observer);
+ initialize_callback.Run(DOWNLOAD_INTERRUPT_REASON_NONE);
+ initialize_completion_loop.Run();
+
+ RunAllPendingInMessageLoops();
+ ASSERT_FALSE(target_callback.is_null());
+
+ ScheduleObservations(PostTargetDeterminationObservations(),
+ destination_observer);
+ target_callback.Run(base::FilePath(kDummyTargetPath),
+ DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+ DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ base::FilePath(kDummyIntermediatePath));
+
+ intermediate_rename_loop.Run();
+ ASSERT_FALSE(intermediate_rename_callback.is_null());
+
+ ScheduleObservations(PostIntermediateRenameObservations(),
+ destination_observer);
+ intermediate_rename_callback.Run(DOWNLOAD_INTERRUPT_REASON_FILE_FAILED,
+ base::FilePath());
+ RunAllPendingInMessageLoops();
+
+ EXPECT_EQ(DownloadItem::INTERRUPTED, item_->GetState());
+}
+
+// Run through the DII workflow. Download file initialization, target
+// determination and intermediate rename all succeed.
+TEST_P(DownloadItemDestinationUpdateRaceTest, IntermediateRenameSucceeds) {
+ // We expect either that the download will fail (in which case the request and
+ // the download file will be cancelled), or it will succeed (in which case the
+ // DownloadFile will Detach()). It depends on the list of observations that
+ // are given to us.
+ EXPECT_CALL(*file_, Cancel()).Times(::testing::AnyNumber());
+ EXPECT_CALL(*request_handle_, CancelRequest()).Times(::testing::AnyNumber());
+ EXPECT_CALL(*file_, Detach()).Times(::testing::AnyNumber());
+
+ EXPECT_CALL(*file_, FullPath())
+ .WillRepeatedly(Return(base::FilePath(kDummyIntermediatePath)));
+
+ // Intermediate rename loop is not used immediately, but let's set up the
+ // DownloadFile expectations since we are about to transfer its ownership to
+ // the DownloadItem.
+ base::RunLoop intermediate_rename_loop;
+ DownloadFile::RenameCompletionCallback intermediate_rename_callback;
+ 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_));
+ 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(_, _))
+ .WillOnce(
+ DoAll(SaveArg<1>(&target_callback),
+ ScheduleClosure(initialize_completion_loop.QuitClosure())));
+ ScheduleObservations(PostInitializeFileObservations(), destination_observer);
+ initialize_callback.Run(DOWNLOAD_INTERRUPT_REASON_NONE);
+ initialize_completion_loop.Run();
+
+ RunAllPendingInMessageLoops();
+ ASSERT_FALSE(target_callback.is_null());
+
+ ScheduleObservations(PostTargetDeterminationObservations(),
+ destination_observer);
+ target_callback.Run(base::FilePath(kDummyTargetPath),
+ DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+ DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ base::FilePath(kDummyIntermediatePath));
+
+ intermediate_rename_loop.Run();
+ ASSERT_FALSE(intermediate_rename_callback.is_null());
+
+ // This may or may not be called, depending on whether there are any errors in
+ // our action list.
+ EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(_, _))
+ .Times(::testing::AnyNumber());
+
+ ScheduleObservations(PostIntermediateRenameObservations(),
+ destination_observer);
+ intermediate_rename_callback.Run(DOWNLOAD_INTERRUPT_REASON_NONE,
+ base::FilePath(kDummyIntermediatePath));
+ RunAllPendingInMessageLoops();
+
+ // The state of the download depends on the observer events that were played
+ // back to the DownloadItemImpl. Hence we can't establish a single expectation
+ // here. On Debug builds, the DCHECKs will verify that the state transitions
+ // were correct. On Release builds, tests are expected to run to completion
+ // without crashing on success.
+ EXPECT_TRUE(item_->GetState() == DownloadItem::IN_PROGRESS ||
+ item_->GetState() == DownloadItem::INTERRUPTED);
+ if (item_->GetState() == DownloadItem::INTERRUPTED)
+ EXPECT_EQ(DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED, item_->GetLastReason());
+
+ item_->Cancel(true);
+ RunAllPendingInMessageLoops();
+}
+
TEST(MockDownloadItem, Compiles) {
MockDownloadItem mock_item;
}
« no previous file with comments | « content/browser/download/download_item_impl.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698