Use ByteStream in downloads system to decouple source and sink.

content/browser/download/download_manager_impl_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <set>
 #include <string>
 
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
 #include "base/stl_util.h"
 #include "base/string16.h"
 #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"
 #include "content/browser/download/download_manager_impl.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/mock_download_file.h"
 #include "content/browser/power_save_blocker.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager_delegate.h"
@@ skipping 29 matching lines @@
 using content::BrowserThread;
 using content::DownloadFile;
 using content::DownloadId;
 using content::DownloadItem;
 using content::DownloadManager;
 using content::WebContents;
 using ::testing::NiceMock;
 using ::testing::ReturnRef;
 using ::testing::Return;
 
+namespace content {
+class ByteStreamOutput;
+}
+
 namespace {
 
 FilePath GetTempDownloadPath(const FilePath& suggested_path) {
   return FilePath(suggested_path.value() + FILE_PATH_LITERAL(".temp"));
 }
 
 class MockDownloadFileFactory
     : public DownloadFileManager::DownloadFileFactory {
  public:
   MockDownloadFileFactory() {}
 
   virtual DownloadFile* CreateFile(
       DownloadCreateInfo* info,
+      scoped_ptr<content::ByteStreamOutput> pipe,
       const DownloadRequestHandle& request_handle,
       DownloadManager* download_manager,
       bool calculate_hash,
       const net::BoundNetLog& bound_net_log) OVERRIDE;
 };
 
 DownloadFile* MockDownloadFileFactory::CreateFile(
     DownloadCreateInfo* info,
+    scoped_ptr<content::ByteStreamOutput> pipe,
     const DownloadRequestHandle& request_handle,
     DownloadManager* download_manager,
     bool calculate_hash,
     const net::BoundNetLog& bound_net_log) {
   NOTREACHED();
   return NULL;
 }
 
 DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain";
 
@@ skipping 119 matching lines @@
  public:
   static const char* kTestData;
   static const size_t kTestDataLen;
 
   DownloadManagerTest()
       : browser_context(new TestBrowserContext()),
         download_manager_delegate_(new TestDownloadManagerDelegate()),
         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_);
   }
 
   ~DownloadManagerTest() {
     download_manager_->Shutdown();
     // browser_context must outlive download_manager_, so we explicitly delete
     // download_manager_ first.
     download_manager_ = NULL;
     download_manager_delegate_.reset();
@@ skipping 18 matching lines @@
   }
 
   void FileSelected(const FilePath& path, int32 download_id) {
     download_manager_->FileSelected(path, download_id);
   }
 
   void ContinueDownloadWithPath(DownloadItem* download, const FilePath& path) {
     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) {
     download_manager_->OnDownloadInterrupted(download_id, size,
                                              hash_state, reason);
   }
 
   // Get the download item with ID |id|.
   DownloadItem* GetActiveDownloadItem(int32 id) {
     return download_manager_->GetActiveDownload(id);
   }
 
  protected:
   scoped_ptr<TestBrowserContext> browser_context;
   scoped_ptr<TestDownloadManagerDelegate> download_manager_delegate_;
   scoped_refptr<DownloadManagerImpl> download_manager_;
   scoped_refptr<DownloadFileManager> file_manager_;
   MessageLoopForUI message_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
-  scoped_refptr<content::DownloadBuffer> download_buffer_;
 
   DownloadFileManager* file_manager() {
     if (!file_manager_) {
       file_manager_ = new DownloadFileManager(new MockDownloadFileFactory);
       download_manager_->SetFileManagerForTesting(file_manager_);
     }
     return file_manager_;
   }
 
   DISALLOW_COPY_AND_ASSIGN(DownloadManagerTest);
 };
 
 const char* DownloadManagerTest::kTestData = "a;sdlfalsdfjalsdkfjad";
 const size_t DownloadManagerTest::kTestDataLen =
     strlen(DownloadManagerTest::kTestData);
 
 // A DownloadFile that we can inject errors into.
 class DownloadFileWithErrors : public DownloadFileImpl {
  public:
   DownloadFileWithErrors(DownloadCreateInfo* info,
+                         scoped_ptr<content::ByteStreamOutput> pipe,
                          DownloadManager* manager,
                          bool calculate_hash);
   virtual ~DownloadFileWithErrors() {}
 
   // BaseFile delegated functions.
   virtual net::Error Initialize();
   virtual net::Error AppendDataToFile(const char* data, size_t data_len);
   virtual net::Error Rename(const FilePath& full_path);
 
   void set_forced_error(net::Error error) { forced_error_ = error; }
   void clear_forced_error() { forced_error_ = net::OK; }
   net::Error forced_error() const { return forced_error_; }
 
  private:
   net::Error ReturnError(net::Error function_error) {
     if (forced_error_ != net::OK) {
       net::Error ret = forced_error_;
       clear_forced_error();
       return ret;
     }
 
     return function_error;
   }
 
   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,
                        scoped_ptr<PowerSaveBlocker>(NULL).Pass(),
                        net::BoundNetLog()),
       forced_error_(net::OK) {
 }
 
 net::Error DownloadFileWithErrors::Initialize() {
   return ReturnError(DownloadFileImpl::Initialize());
@@ skipping 173 matching lines @@
     // don't call the function that deletes it, so we do so ourselves.
     scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
     const DownloadId id = DownloadId(kValidIdDomain, static_cast<int>(i));
     info->download_id = id;
     info->prompt_user_for_save_location = kStartDownloadCases[i].save_as;
     info->url_chain.push_back(GURL(kStartDownloadCases[i].url));
     info->mime_type = kStartDownloadCases[i].mime_type;
     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()));
     AddDownloadToFileManager(info->download_id.local(), download_file);
     download_file->Initialize();
     download_manager_->StartDownload(info->download_id.local());
     message_loop_.RunAllPending();
 
     // SelectFileObserver will have recorded any attempt to open the
     // select file dialog.
@@ skipping 370 matching lines @@
       message_loop_.RunAllPending();
       OnResponseCompleted(i, 1024, std::string("fake_hash"));
     }
     // Validating the download item, so it will complete.
     if (state.danger == content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE)
       download->DangerousDownloadValidated();
     message_loop_.RunAllPending();
   }
 }
 
+void WriteCopyToPipe(content::ByteStreamInput* pipe,

[benjhayden, 2012/05/26 20:42:02]

inputs before outputs

[Randy Smith (Not in Mondays), 2012/05/28 02:51:37]

Done, but I don't consider pipe an output in that sense (it's having stuff done
to it, but the function isn't creating a new 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;
   using ::testing::Invoke;
   using ::testing::Return;
 
   // Normally, the download system takes ownership of info, and is
   // responsible for deleting it.  In these unit tests, however, we
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
   info->download_id = DownloadId(kValidIdDomain, 0);
   info->prompt_user_for_save_location = false;
   info->url_chain.push_back(GURL());
   info->total_bytes = static_cast<int64>(kTestDataLen);
   const FilePath new_path(FILE_PATH_LITERAL("foo.zip"));
   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);
 
   EXPECT_CALL(*download_file, Rename(cr_path))
       .Times(1)
       .WillOnce(Return(net::OK));
 
   download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
 
   DownloadItem* download = GetActiveDownloadItem(0);
   ASSERT_TRUE(download != NULL);
   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);
 
   int64 error_size = 3;
   OnDownloadInterrupted(0, error_size, "",
                         content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
   message_loop_.RunAllPending();
 
   EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
@@ skipping 39 matching lines @@
   // responsible for deleting it.  In these unit tests, however, we
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
   static const int32 local_id = 0;
   info->download_id = DownloadId(kValidIdDomain, local_id);
   info->prompt_user_for_save_location = false;
   info->url_chain.push_back(GURL());
   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);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
   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);
   message_loop_.RunAllPending();
   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);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_TRUE(observer->hit_state(DownloadItem::INTERRUPTED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::COMPLETE));
   EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::REMOVING));
   EXPECT_TRUE(observer->was_updated());
   EXPECT_FALSE(observer->was_opened());
@@ skipping 16 matching lines @@
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
   DownloadId id = DownloadId(kValidIdDomain, 0);
   info->download_id = id;
   info->prompt_user_for_save_location = false;
   info->url_chain.push_back(GURL());
   const FilePath new_path(FILE_PATH_LITERAL("foo.zip"));
   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.
   EXPECT_CALL(*download_file, Rename(cr_path))
       .Times(1)
       .WillOnce(Return(net::OK));
 
   download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
 
   DownloadItem* download = GetActiveDownloadItem(0);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
   scoped_ptr<ItemObserver> observer(new ItemObserver(download));
 
   ContinueDownloadWithPath(download, new_path);
   message_loop_.RunAllPending();
   EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
 
-  download_file->AppendDataToFile(kTestData, kTestDataLen);
-
   download->Cancel(false);
   message_loop_.RunAllPending();
 
   EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_TRUE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::COMPLETE));
   EXPECT_FALSE(observer->hit_state(DownloadItem::REMOVING));
   EXPECT_TRUE(observer->was_updated());
@@ skipping 37 matching lines @@
   unique_new_path = unique_new_path.InsertBeforeExtensionASCII(
             StringPrintf(" (%d)", uniquifier));
 
   // Normally, the download system takes ownership of info, and is
   // responsible for deleting it.  In these unit tests, however, we
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
   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());
 
   DownloadItem* download = GetActiveDownloadItem(0);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
   scoped_ptr<ItemObserver> observer(new ItemObserver(download));
 
   // Create and initialize the download file.  We're bypassing the first part
   // of the download process and skipping to the part after the final file
   // 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()));
   download_file->Rename(cr_path);
   // This creates the .temp version of the file.
   download_file->Initialize();
   // |download_file| is owned by DownloadFileManager.
   AddDownloadToFileManager(info->download_id.local(), download_file);
 
   ContinueDownloadWithPath(download, new_path);
   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, "");
   message_loop_.RunAllPending();
 
   // Download is complete.
   EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
@@ skipping 27 matching lines @@
   const FilePath cr_path(GetTempDownloadPath(new_path));
   EXPECT_FALSE(file_util::PathExists(new_path));
 
   // Normally, the download system takes ownership of info, and is
   // responsible for deleting it.  In these unit tests, however, we
   // don't call the function that deletes it, so we do so ourselves.
   scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
   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());
 
   DownloadItem* download = GetActiveDownloadItem(0);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->GetState());
   scoped_ptr<ItemObserver> observer(new ItemObserver(download));
 
   // Create and initialize the download file.  We're bypassing the first part
   // of the download process and skipping to the part after the final file
   // 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()));
   download_file->Rename(cr_path);
   // This creates the .temp version of the file.
   download_file->Initialize();
   // |download_file| is owned by DownloadFileManager.
   AddDownloadToFileManager(info->download_id.local(), download_file);
 
   ContinueDownloadWithPath(download, new_path);
   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, "");
   message_loop_.RunAllPending();
 
   // Download is complete.
   EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
@@ skipping 18 matching lines @@
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
   EXPECT_TRUE(observer->hit_state(DownloadItem::COMPLETE));
   EXPECT_FALSE(observer->hit_state(DownloadItem::REMOVING));
   EXPECT_TRUE(observer->was_updated());
   EXPECT_FALSE(observer->was_opened());
   EXPECT_TRUE(download->GetFileExternallyRemoved());
   EXPECT_EQ(DownloadItem::COMPLETE, download->GetState());
 
   EXPECT_FALSE(file_util::PathExists(new_path));
 }