content/browser/download/download_manager_impl_unittest.cc - Issue 10392111: Use ByteStream in downloads system to decouple source and sink.

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

Issue 10392111: Use ByteStream in downloads system to decouple source and sink. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Fixed lack of virtual keyword on destructor. Created 8 years, 7 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_file_unittest.cc ('k') | content/browser/download/download_net_log_parameters.h » ('j') | content/browser/download/download_stats.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: content/browser/download/download_manager_impl_unittest.cc

diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc

index 2f3bbe8e7be9937ede2b42ea0d67175503088a61..b3887a511323c18bdf8f11d11317b7f2dd997b46 100644

--- a/content/browser/download/download_manager_impl_unittest.cc

+++ b/content/browser/download/download_manager_impl_unittest.cc

@@ -16,7 +16,6 @@

#include "base/string_util.h"

#include "base/utf_string_conversions.h"

#include "build/build_config.h"

-#include "content/browser/download/download_buffer.h"

#include "content/browser/download/download_create_info.h"

#include "content/browser/download/download_file_impl.h"

#include "content/browser/download/download_file_manager.h"

@@ -66,6 +65,10 @@ using ::testing::NiceMock;

using ::testing::ReturnRef;

using ::testing::Return;

+namespace content {

+class ByteStreamReader;

namespace {

FilePath GetTempDownloadPath(const FilePath& suggested_path) {

@@ -79,6 +82,7 @@ class MockDownloadFileFactory

virtual DownloadFile* CreateFile(

DownloadCreateInfo* info,

+ scoped_ptr<content::ByteStreamReader> stream,

const DownloadRequestHandle& request_handle,

DownloadManager* download_manager,

bool calculate_hash,

@@ -87,6 +91,7 @@ class MockDownloadFileFactory

DownloadFile* MockDownloadFileFactory::CreateFile(

DownloadCreateInfo* info,

+ scoped_ptr<content::ByteStreamReader> stream,

const DownloadRequestHandle& request_handle,

DownloadManager* download_manager,

bool calculate_hash,

@@ -226,8 +231,7 @@ class DownloadManagerTest : public testing::Test {

download_manager_(new DownloadManagerImpl(

download_manager_delegate_.get(), NULL)),

ui_thread_(BrowserThread::UI, &message_loop_),

- file_thread_(BrowserThread::FILE, &message_loop_),

- download_buffer_(new content::DownloadBuffer) {

+ file_thread_(BrowserThread::FILE, &message_loop_) {

download_manager_->Init(browser_context.get());

download_manager_delegate_->set_download_manager(download_manager_);

}

@@ -266,23 +270,6 @@ class DownloadManagerTest : public testing::Test {

download_manager_->ContinueDownloadWithPath(download, path);

}

- void UpdateData(int32 id, const char* data, size_t length) {

- // We are passing ownership of this buffer to the download file manager.

- net::IOBuffer* io_buffer = new net::IOBuffer(length);

- io_buffer->AddRef();

- memcpy(io_buffer->data(), data, length);

- download_buffer_->AddData(io_buffer, length);

- BrowserThread::PostTask(

- BrowserThread::FILE, FROM_HERE,

- base::Bind(&DownloadFileManager::UpdateDownload, file_manager_.get(),

- DownloadId(kValidIdDomain, id), download_buffer_));

- message_loop_.RunAllPending();

- }

void OnDownloadInterrupted(int32 download_id, int64 size,

const std::string& hash_state,

content::DownloadInterruptReason reason) {

@@ -303,7 +290,6 @@ class DownloadManagerTest : public testing::Test {

MessageLoopForUI message_loop_;

content::TestBrowserThread ui_thread_;

content::TestBrowserThread file_thread_;

- scoped_refptr<content::DownloadBuffer> download_buffer_;

DownloadFileManager* file_manager() {

if (!file_manager_) {

@@ -324,6 +310,7 @@ const size_t DownloadManagerTest::kTestDataLen =

class DownloadFileWithErrors : public DownloadFileImpl {

public:

DownloadFileWithErrors(DownloadCreateInfo* info,

+ scoped_ptr<content::ByteStreamReader> stream,

DownloadManager* manager,

bool calculate_hash);

virtual ~DownloadFileWithErrors() {}

@@ -351,10 +338,13 @@ class DownloadFileWithErrors : public DownloadFileImpl {

net::Error forced_error_;

};

-DownloadFileWithErrors::DownloadFileWithErrors(DownloadCreateInfo* info,

- DownloadManager* manager,

- bool calculate_hash)

+DownloadFileWithErrors::DownloadFileWithErrors(

+ DownloadCreateInfo* info,

+ scoped_ptr<content::ByteStreamReader> stream,

+ DownloadManager* manager,

+ bool calculate_hash)

: DownloadFileImpl(info,

+ stream.Pass(),

new DownloadRequestHandle(),

manager,

calculate_hash,

@@ -548,7 +538,9 @@ TEST_F(DownloadManagerTest, MAYBE_StartDownload) {

download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());

DownloadFile* download_file(

- new DownloadFileImpl(info.get(), new DownloadRequestHandle(),

+ new DownloadFileImpl(info.get(),

+ scoped_ptr<content::ByteStreamReader>(),

+ new DownloadRequestHandle(),

download_manager_, false,

scoped_ptr<PowerSaveBlocker>(NULL).Pass(),

net::BoundNetLog()));

@@ -939,6 +931,13 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) {

}

+void WriteCopyToStream(const char *source,

+ size_t len, content::ByteStreamWriter* stream) {

+ scoped_refptr<net::IOBuffer> copy(new net::IOBuffer(len));

+ memcpy(copy.get()->data(), source, len);

+ stream->Write(copy, len);

TEST_F(DownloadManagerTest, DownloadInterruptTest) {

using ::testing::_;

using ::testing::CreateFunctor;

@@ -957,8 +956,6 @@ TEST_F(DownloadManagerTest, DownloadInterruptTest) {

const FilePath cr_path(GetTempDownloadPath(new_path));

MockDownloadFile* download_file(new NiceMock<MockDownloadFile>());

- ON_CALL(*download_file, AppendDataToFile(_, _))

- .WillByDefault(Return(net::OK));

// |download_file| is owned by DownloadFileManager.

AddMockDownloadToFileManager(info->download_id.local(), download_file);

@@ -974,8 +971,6 @@ TEST_F(DownloadManagerTest, DownloadInterruptTest) {

EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());

scoped_ptr<ItemObserver> observer(new ItemObserver(download));

- download_file->AppendDataToFile(kTestData, kTestDataLen);

ContinueDownloadWithPath(download, new_path);

message_loop_.RunAllPending();

EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);

@@ -1035,14 +1030,17 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadFileErrorTest) {

info->total_bytes = static_cast<int64>(kTestDataLen * 3);

info->save_info.file_path = path;

info->save_info.file_stream.reset(stream);

+ scoped_ptr<content::ByteStreamWriter> stream_input;

+ scoped_ptr<content::ByteStreamReader> stream_output;

+ content::CreateByteStream(message_loop_.message_loop_proxy(),

+ message_loop_.message_loop_proxy(),

+ kTestDataLen, &stream_input, &stream_output);

// Create a download file that we can insert errors into.

- DownloadFileWithErrors* download_file(new DownloadFileWithErrors(

- info.get(), download_manager_, false));

+ scoped_ptr<DownloadFileWithErrors> download_file(new DownloadFileWithErrors(

+ info.get(), stream_output.Pass(), download_manager_, false));

download_file->Initialize();

- AddDownloadToFileManager(local_id, download_file);

- // |download_file| is owned by DownloadFileManager.

download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());

DownloadItem* download = GetActiveDownloadItem(0);

@@ -1052,7 +1050,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadFileErrorTest) {

scoped_ptr<ItemObserver> observer(new ItemObserver(download));

// Add some data before finalizing the file name.

- UpdateData(local_id, kTestData, kTestDataLen);

+ WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());

// Finalize the file name.

ContinueDownloadWithPath(download, path);

@@ -1060,11 +1058,11 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadFileErrorTest) {

EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);

// Add more data.

- UpdateData(local_id, kTestData, kTestDataLen);

+ WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());

// Add more data, but an error occurs.

download_file->set_forced_error(net::ERR_FAILED);

- UpdateData(local_id, kTestData, kTestDataLen);

+ WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());

// Check the state. The download should have been interrupted.

EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);

@@ -1101,8 +1099,6 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) {

const FilePath cr_path(GetTempDownloadPath(new_path));

MockDownloadFile* download_file(new NiceMock<MockDownloadFile>());

- ON_CALL(*download_file, AppendDataToFile(_, _))

- .WillByDefault(Return(net::OK));

AddMockDownloadToFileManager(info->download_id.local(), download_file);

// |download_file| is owned by DownloadFileManager.

@@ -1122,8 +1118,6 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) {

message_loop_.RunAllPending();

EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);

- download_file->AppendDataToFile(kTestData, kTestDataLen);

download->Cancel(false);

message_loop_.RunAllPending();

@@ -1181,6 +1175,11 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadOverwriteTest) {

info->download_id = DownloadId(kValidIdDomain, 0);

info->prompt_user_for_save_location = true;

info->url_chain.push_back(GURL());

+ scoped_ptr<content::ByteStreamWriter> stream_input;

+ scoped_ptr<content::ByteStreamReader> stream_output;

+ content::CreateByteStream(message_loop_.message_loop_proxy(),

+ message_loop_.message_loop_proxy(),

+ kTestDataLen, &stream_input, &stream_output);

download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());

@@ -1195,7 +1194,8 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadOverwriteTest) {

// name has been chosen, so we need to initialize the download file

// properly.

DownloadFile* download_file(

- new DownloadFileImpl(info.get(), new DownloadRequestHandle(),

+ new DownloadFileImpl(info.get(), stream_output.Pass(),

+ new DownloadRequestHandle(),

download_manager_, false,

scoped_ptr<PowerSaveBlocker>(NULL).Pass(),

net::BoundNetLog()));

@@ -1209,7 +1209,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadOverwriteTest) {

message_loop_.RunAllPending();

EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);

- download_file->AppendDataToFile(kTestData, kTestDataLen);

+ WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());

// Finish the download.

OnResponseCompleted(0, kTestDataLen, "");

@@ -1257,6 +1257,11 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadRemoveTest) {

info->download_id = DownloadId(kValidIdDomain, 0);

info->prompt_user_for_save_location = true;

info->url_chain.push_back(GURL());

+ scoped_ptr<content::ByteStreamWriter> stream_input;

+ scoped_ptr<content::ByteStreamReader> stream_output;

+ content::CreateByteStream(message_loop_.message_loop_proxy(),

+ message_loop_.message_loop_proxy(),

+ kTestDataLen, &stream_input, &stream_output);

download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());

@@ -1271,7 +1276,8 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadRemoveTest) {

// name has been chosen, so we need to initialize the download file

// properly.

DownloadFile* download_file(

- new DownloadFileImpl(info.get(), new DownloadRequestHandle(),

+ new DownloadFileImpl(info.get(), stream_output.Pass(),

+ new DownloadRequestHandle(),

download_manager_, false,

scoped_ptr<PowerSaveBlocker>(NULL).Pass(),

net::BoundNetLog()));

@@ -1285,7 +1291,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadRemoveTest) {

message_loop_.RunAllPending();

EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);

- download_file->AppendDataToFile(kTestData, kTestDataLen);

+ WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());

// Finish the download.

OnResponseCompleted(0, kTestDataLen, "");