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

Issues Search
    My Issues | Starred     Open | Closed | All

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: Added some histogram statistics. 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
« content/browser/download/download_file_unittest.cc ('K') | « content/browser/download/download_file_unittest.cc ('k') | content/browser/download/download_net_log_parameters.h » ('j') | content/browser/download/download_resource_handler.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments 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 221692e4d9e20aef492e75d21b3e5fa19bd9d882..8739664af93a2acba8ef67cb6d2f530352206f63 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 ByteStreamOutput;
+}
+
namespace {
FilePath GetTempDownloadPath(const FilePath& suggested_path) {
@@ -79,6 +82,7 @@ class MockDownloadFileFactory
virtual DownloadFile* CreateFile(
DownloadCreateInfo* info,
+ scoped_ptr<content::ByteStreamOutput> pipe,
const DownloadRequestHandle& request_handle,
DownloadManager* download_manager,
bool calculate_hash,
@@ -87,6 +91,7 @@ class MockDownloadFileFactory
DownloadFile* MockDownloadFileFactory::CreateFile(
DownloadCreateInfo* info,
+ scoped_ptr<content::ByteStreamOutput> pipe,
const DownloadRequestHandle& request_handle,
DownloadManager* download_manager,
bool calculate_hash,
@@ -219,8 +224,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_);
}
@@ -259,23 +263,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);
- // We need |AddRef()| because we do a |Release()| in |UpdateDownload()|.
- 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) {
@@ -296,7 +283,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_) {
@@ -317,6 +303,7 @@ const size_t DownloadManagerTest::kTestDataLen =
class DownloadFileWithErrors : public DownloadFileImpl {
public:
DownloadFileWithErrors(DownloadCreateInfo* info,
+ scoped_ptr<content::ByteStreamOutput> pipe,
DownloadManager* manager,
bool calculate_hash);
virtual ~DownloadFileWithErrors() {}
@@ -344,10 +331,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::ByteStreamOutput> pipe,
+ DownloadManager* manager,
+ bool calculate_hash)
: DownloadFileImpl(info,
+ pipe.Pass(),
new DownloadRequestHandle(),
manager,
calculate_hash,
@@ -541,7 +531,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::ByteStreamOutput>(),
+ new DownloadRequestHandle(),
download_manager_, false,
scoped_ptr<PowerSaveBlocker>(NULL).Pass(),
net::BoundNetLog()));
@@ -932,6 +924,14 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) {
}
}
+void WriteCopyToPipe(content::ByteStreamInput* pipe,
+ const char *source,
+ size_t len) {
+ scoped_refptr<net::IOBuffer> copy(new net::IOBuffer(len));
+ memcpy(copy.get()->data(), source, len);
+ pipe->Write(copy, len);
+}
+
TEST_F(DownloadManagerTest, DownloadInterruptTest) {
using ::testing::_;
using ::testing::CreateFunctor;
@@ -950,8 +950,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);
@@ -967,8 +965,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);
@@ -1028,14 +1024,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::ByteStreamInput> pipe_input;
+ scoped_ptr<content::ByteStreamOutput> pipe_output;
+ content::CreateByteStream(message_loop_.message_loop_proxy(),
+ message_loop_.message_loop_proxy(),
+ kTestDataLen, &pipe_input, &pipe_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(), pipe_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);
@@ -1045,7 +1044,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);
+ WriteCopyToPipe(pipe_input.get(), kTestData, kTestDataLen);
// Finalize the file name.
ContinueDownloadWithPath(download, path);
@@ -1053,11 +1052,11 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadFileErrorTest) {
EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
// Add more data.
- UpdateData(local_id, kTestData, kTestDataLen);
+ WriteCopyToPipe(pipe_input.get(), kTestData, kTestDataLen);
// Add more data, but an error occurs.
download_file->set_forced_error(net::ERR_FAILED);
- UpdateData(local_id, kTestData, kTestDataLen);
+ WriteCopyToPipe(pipe_input.get(), kTestData, kTestDataLen);
// Check the state. The download should have been interrupted.
EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
@@ -1094,8 +1093,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.
@@ -1115,8 +1112,6 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) {
message_loop_.RunAllPending();
EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
- download_file->AppendDataToFile(kTestData, kTestDataLen);
-
download->Cancel(false);
message_loop_.RunAllPending();
@@ -1174,6 +1169,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::ByteStreamInput> pipe_input;
+ scoped_ptr<content::ByteStreamOutput> pipe_output;
+ content::CreateByteStream(message_loop_.message_loop_proxy(),
+ message_loop_.message_loop_proxy(),
+ kTestDataLen, &pipe_input, &pipe_output);
download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
@@ -1188,7 +1188,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(), pipe_output.Pass(),
+ new DownloadRequestHandle(),
download_manager_, false,
scoped_ptr<PowerSaveBlocker>(NULL).Pass(),
net::BoundNetLog()));
@@ -1202,7 +1203,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadOverwriteTest) {
message_loop_.RunAllPending();
EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
- download_file->AppendDataToFile(kTestData, kTestDataLen);
+ WriteCopyToPipe(pipe_input.get(), kTestData, kTestDataLen);
// Finish the download.
OnResponseCompleted(0, kTestDataLen, "");
@@ -1250,6 +1251,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::ByteStreamInput> pipe_input;
+ scoped_ptr<content::ByteStreamOutput> pipe_output;
+ content::CreateByteStream(message_loop_.message_loop_proxy(),
+ message_loop_.message_loop_proxy(),
+ kTestDataLen, &pipe_input, &pipe_output);
download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
@@ -1264,7 +1270,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(), pipe_output.Pass(),
+ new DownloadRequestHandle(),
download_manager_, false,
scoped_ptr<PowerSaveBlocker>(NULL).Pass(),
net::BoundNetLog()));
@@ -1278,7 +1285,7 @@ TEST_F(DownloadManagerTest, MAYBE_DownloadRemoveTest) {
message_loop_.RunAllPending();
EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
- download_file->AppendDataToFile(kTestData, kTestDataLen);
+ WriteCopyToPipe(pipe_input.get(), kTestData, kTestDataLen);
// Finish the download.
OnResponseCompleted(0, kTestDataLen, "");
« content/browser/download/download_file_unittest.cc ('K') | « content/browser/download/download_file_unittest.cc ('k') | content/browser/download/download_net_log_parameters.h » ('j') | content/browser/download/download_resource_handler.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698