Use ByteStream in downloads system to decouple source and sink.

content/browser/download/download_file_unittest.cc

Index: content/browser/download/download_file_unittest.cc
diff --git a/content/browser/download/download_file_unittest.cc b/content/browser/download/download_file_unittest.cc
index 2e8df324f0ebb164fae83acc2af352d829d123cc..3a907875a84318ffed3e87406ffc57117b1af222 100644
--- a/content/browser/download/download_file_unittest.cc
+++ b/content/browser/download/download_file_unittest.cc
@@ -1,3 +1,4 @@
+

[benjhayden, 2012/05/21 14:36:45]

Unnecessary blank line

[Randy Smith (Not in Mondays), 2012/05/23 03:33:15]

Done.

 // 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.
@@ -6,14 +7,17 @@
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
 #include "content/browser/browser_thread_impl.h"
+#include "content/browser/download/byte_stream.h"
 #include "content/browser/download/download_create_info.h"
 #include "content/browser/download/download_file_impl.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/power_save_blocker.h"
+#include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_manager.h"
 #include "content/test/mock_download_manager.h"
 #include "net/base/file_stream.h"
 #include "net/base/net_errors.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::BrowserThread;
@@ -21,9 +25,28 @@ using content::BrowserThreadImpl;
 using content::DownloadFile;
 using content::DownloadId;
 using content::DownloadManager;
-using testing::_;
-using testing::AnyNumber;
-using testing::StrictMock;
+using ::testing::_;
+using ::testing::AnyNumber;
+using ::testing::DoAll;
+using ::testing::Return;
+using ::testing::SetArgPointee;
+using ::testing::StrictMock;
+
+namespace {
+
+class MockByteStreamOutput : public content::ByteStreamOutput {
+ public:
+  MockByteStreamOutput() {}
+  ~MockByteStreamOutput() {}
+
+  // ByteStream functions
+  MOCK_METHOD2(Read, content::ByteStreamOutput::StreamState(
+      scoped_refptr<net::IOBuffer>*, size_t*));
+  MOCK_CONST_METHOD0(GetStatus, content::DownloadInterruptReason());
+  MOCK_METHOD1(RegisterCallback, void(const base::Closure&));
+};
+
+}  // namespace
 
 DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain";
 
@@ -77,45 +100,125 @@ class DownloadFileTest : public testing::Test {
     ui_thread_.message_loop()->RunAllPending();
   }
 
-  virtual void CreateDownloadFile(scoped_ptr<DownloadFile>* file,
-                                  int offset,
-                                  bool calculate_hash) {
+  // Mock calls to this function are forwarded here.
+  void RegisterCallback(base::Closure sink_callback) {
+    sink_callback_ = sink_callback;
+  }
+
+  virtual bool CreateDownloadFile(int offset, bool calculate_hash) {
+    // There can be only one.
+    DCHECK(!download_file_.get());
+
+    input_pipe_ = new StrictMock<MockByteStreamOutput>();
+
+    // TODO: Need to actually create a function that'll set the variables
+    // based on the inputs from the callback.
+    EXPECT_CALL(*input_pipe_, RegisterCallback(_))
+        .WillOnce(Invoke(this, &DownloadFileTest::RegisterCallback))
+        .RetiresOnSaturation();
+
     DownloadCreateInfo info;
     info.download_id = DownloadId(kValidIdDomain, kDummyDownloadId + offset);
     // info.request_handle default constructed to null.
     info.save_info.file_stream = file_stream_;
-    file->reset(
-        new DownloadFileImpl(&info, new DownloadRequestHandle(),
-                             download_manager_, calculate_hash,
-                             scoped_ptr<PowerSaveBlocker>(NULL).Pass(),
-                             net::BoundNetLog()));
+    download_file_.reset(
+        new DownloadFileImpl(
+            &info,
+            scoped_ptr<content::ByteStreamOutput>(input_pipe_).Pass(),
+            new DownloadRequestHandle(),
+            download_manager_, calculate_hash,
+            scoped_ptr<PowerSaveBlocker>(NULL).Pass(),
+            net::BoundNetLog()));
+
+    EXPECT_CALL(*input_pipe_, Read(_, _))
+        .WillOnce(Return(content::ByteStreamOutput::STREAM_EMPTY))
+        .RetiresOnSaturation();
+    net::Error result = download_file_->Initialize();
+    ::testing::Mock::VerifyAndClearExpectations(input_pipe_);
+    return result == net::OK;
   }
 
-  virtual void DestroyDownloadFile(scoped_ptr<DownloadFile>* file, int offset) {
-    EXPECT_EQ(kDummyDownloadId + offset, (*file)->Id());
-    EXPECT_EQ(download_manager_, (*file)->GetDownloadManager());
-    EXPECT_FALSE((*file)->InProgress());
+  virtual void DestroyDownloadFile(int offset) {
+    EXPECT_EQ(kDummyDownloadId + offset, download_file_->Id());
+    EXPECT_EQ(download_manager_, download_file_->GetDownloadManager());
+    EXPECT_FALSE(download_file_->InProgress());
     EXPECT_EQ(static_cast<int64>(expected_data_.size()),
-              (*file)->BytesSoFar());
+              download_file_->BytesSoFar());
 
     // Make sure the data has been properly written to disk.
     std::string disk_data;
-    EXPECT_TRUE(file_util::ReadFileToString((*file)->FullPath(),
+    EXPECT_TRUE(file_util::ReadFileToString(download_file_->FullPath(),
                                             &disk_data));
     EXPECT_EQ(expected_data_, disk_data);
 
     // Make sure the Browser and File threads outlive the DownloadFile
     // to satisfy thread checks inside it.
-    file->reset();
+    EXPECT_CALL(*input_pipe_, RegisterCallback(_))
+        .WillOnce(Invoke(this, &DownloadFileTest::RegisterCallback))
+        .RetiresOnSaturation();
+    download_file_.reset();
   }
 
-  void AppendDataToFile(scoped_ptr<DownloadFile>* file,
-                        const std::string& data) {
-    EXPECT_TRUE((*file)->InProgress());
-    (*file)->AppendDataToFile(data.data(), data.size());
-    expected_data_ += data;
-    EXPECT_EQ(static_cast<int64>(expected_data_.size()),
-              (*file)->BytesSoFar());
+  // Setup the pipe to do be a data append; don't actually trigger
+  // the callback or do verifications.
+  void SetupDataAppend(const char **data_chunks, size_t num_chunks,
+                       ::testing::Sequence s) {
+    DCHECK(input_pipe_);
+    for (size_t i = 0; i < num_chunks; i++) {
+      const char *source_data = data_chunks[i];
+      size_t length = strlen(source_data);
+      scoped_refptr<net::IOBuffer> data = new net::IOBuffer(length);
+      memcpy(data->data(), source_data, length);
+      EXPECT_CALL(*input_pipe_, Read(_, _))
+          .InSequence(s)
+          .WillOnce(DoAll(SetArgPointee<0>(data),
+                          SetArgPointee<1>(length),
+                          Return(content::ByteStreamOutput::STREAM_HAS_DATA)))
+          .RetiresOnSaturation();
+      expected_data_ += source_data;
+    }
+  }
+
+  void VerifyPipeAndSize() {
+    ::testing::Mock::VerifyAndClearExpectations(input_pipe_);
+    int64 size;
+    EXPECT_TRUE(file_util::GetFileSize(download_file_->FullPath(), &size));
+    EXPECT_EQ(expected_data_.size(), static_cast<size_t>(size));
+  }
+
+  // TODO(rdsmith): Manage full percentage issues properly.
+  void AppendDataToFile(const char **data_chunks, size_t num_chunks) {
+    ::testing::Sequence s1;
+    SetupDataAppend(data_chunks, num_chunks, s1);
+    EXPECT_CALL(*input_pipe_, Read(_, _))
+        .InSequence(s1)
+        .WillOnce(Return(content::ByteStreamOutput::STREAM_EMPTY))
+        .RetiresOnSaturation();
+    sink_callback_.Run();
+    VerifyPipeAndSize();
+  }
+
+  void SetupFinishPipe(content::DownloadInterruptReason interrupt_reason,
+                       ::testing::Sequence s) {
+    EXPECT_CALL(*input_pipe_, Read(_, _))
+        .InSequence(s)
+        .WillOnce(Return(content::ByteStreamOutput::STREAM_COMPLETE))
+        .RetiresOnSaturation();
+    EXPECT_CALL(*input_pipe_, GetStatus())
+        .InSequence(s)
+        .WillOnce(Return(interrupt_reason))
+        .RetiresOnSaturation();
+    EXPECT_CALL(*input_pipe_, RegisterCallback(_))
+        .InSequence(s)
+        .WillOnce(Invoke(this, &DownloadFileTest::RegisterCallback))
+        .RetiresOnSaturation();
+  }
+
+  void FinishPipe(content::DownloadInterruptReason interrupt_reason) {
+    ::testing::Sequence s1;
+    SetupFinishPipe(interrupt_reason, s1);
+    sink_callback_.Run();
+    VerifyPipeAndSize();
   }
 
  protected:
@@ -126,6 +229,13 @@ class DownloadFileTest : public testing::Test {
   // DownloadFile instance we are testing.
   scoped_ptr<DownloadFile> download_file_;
 
+  // Pipe for sending data into the download file.
+  // Owned by download_file_; will be alive for lifetime of download_file_.
+  StrictMock<MockByteStreamOutput>* input_pipe_;
+
+  // Sink callback data for pipe.
+  base::Closure sink_callback_;
+
   // Latest update sent to the download manager.
   int64 bytes_;
   int64 bytes_per_sec_;
@@ -157,7 +267,7 @@ const int DownloadFileTest::kDummyRequestId = 67;
 // Rename the file before any data is downloaded, after some has, after it all
 // has, and after it's closed.
 TEST_F(DownloadFileTest, RenameFileFinal) {
-  CreateDownloadFile(&download_file_, 0, true);
+  CreateDownloadFile(0, true);
   ASSERT_EQ(net::OK, download_file_->Initialize());
   FilePath initial_path(download_file_->FullPath());
   EXPECT_TRUE(file_util::PathExists(initial_path));
@@ -176,8 +286,8 @@ TEST_F(DownloadFileTest, RenameFileFinal) {
   EXPECT_TRUE(file_util::PathExists(path_1));
 
   // Download the data.
-  AppendDataToFile(&download_file_, kTestData1);
-  AppendDataToFile(&download_file_, kTestData2);
+  const char* chunks1[] = { kTestData1, kTestData2 };
+  AppendDataToFile(chunks1, 2);
 
   // Rename the file after downloading some data.
   EXPECT_EQ(net::OK, download_file_->Rename(path_2));
@@ -188,7 +298,8 @@ TEST_F(DownloadFileTest, RenameFileFinal) {
   EXPECT_FALSE(file_util::PathExists(path_1));
   EXPECT_TRUE(file_util::PathExists(path_2));
 
-  AppendDataToFile(&download_file_, kTestData3);
+  const char* chunks2[] = { kTestData3 };
+  AppendDataToFile(chunks2, 1);
 
   // Rename the file after downloading all the data.
   EXPECT_EQ(net::OK, download_file_->Rename(path_3));
@@ -202,8 +313,7 @@ TEST_F(DownloadFileTest, RenameFileFinal) {
   // Should not be able to get the hash until the file is closed.
   std::string hash;
   EXPECT_FALSE(download_file_->GetHash(&hash));
-
-  download_file_->Finish();
+  FinishPipe(content::DOWNLOAD_INTERRUPT_REASON_NONE);
 
   // Rename the file after downloading all the data and closing the file.
   EXPECT_EQ(net::OK, download_file_->Rename(path_4));
@@ -218,17 +328,94 @@ TEST_F(DownloadFileTest, RenameFileFinal) {
   EXPECT_TRUE(download_file_->GetHash(&hash));
   EXPECT_EQ(kDataHash, base::HexEncode(hash.data(), hash.size()));
 
-  DestroyDownloadFile(&download_file_, 0);
+  DestroyDownloadFile(0);
+}
+
+// Various tests of the PipeActive method.
+TEST_F(DownloadFileTest, PipeEmptySuccess) {
+  ASSERT_TRUE(CreateDownloadFile(0, true));
+  FilePath initial_path(download_file_->FullPath());
+  EXPECT_TRUE(file_util::PathExists(initial_path));
+
+  // Test that calling the sink_callback_ on an empty pipe shouldn't
+  // do anything.
+  AppendDataToFile(NULL, 0);
+  ::testing::Mock::VerifyAndClearExpectations(download_manager_.get());
+
+  // Finish the download this way and make sure we see it on the
+  // DownloadManager.
+  EXPECT_CALL(*(download_manager_.get()),
+              OnResponseCompleted(DownloadId(kValidIdDomain,
+                                             kDummyDownloadId + 0).local(),
+                                  0, _));
+  FinishPipe(content::DOWNLOAD_INTERRUPT_REASON_NONE);
+
+  DestroyDownloadFile(0);
+}
+
+TEST_F(DownloadFileTest, PipeEmptyError) {
+  ASSERT_TRUE(CreateDownloadFile(0, true));
+  FilePath initial_path(download_file_->FullPath());
+  EXPECT_TRUE(file_util::PathExists(initial_path));
+
+  // Finish the download in error and make sure we see it on the
+  // DownloadManager.
+  EXPECT_CALL(*(download_manager_.get()),
+              OnDownloadInterrupted(
+                  DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
+                  0, _,
+                  content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED));
+  FinishPipe(content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED);
+
+  DestroyDownloadFile(0);
+}
+
+TEST_F(DownloadFileTest, PipeNonEmptySuccess) {
+  ASSERT_TRUE(CreateDownloadFile(0, true));
+  FilePath initial_path(download_file_->FullPath());
+  EXPECT_TRUE(file_util::PathExists(initial_path));
+
+  const char* chunks1[] = { kTestData1, kTestData2 };
+  ::testing::Sequence s1;
+  SetupDataAppend(chunks1, 2, s1);
+  SetupFinishPipe(content::DOWNLOAD_INTERRUPT_REASON_NONE, s1);
+  EXPECT_CALL(*(download_manager_.get()),
+              OnResponseCompleted(DownloadId(kValidIdDomain,
+                                             kDummyDownloadId + 0).local(),
+                                  strlen(kTestData1) + strlen(kTestData2),
+                                  _));
+  sink_callback_.Run();
+  VerifyPipeAndSize();
+  DestroyDownloadFile(0);
+}
+
+TEST_F(DownloadFileTest, PipeNonEmptyError) {
+  ASSERT_TRUE(CreateDownloadFile(0, true));
+  FilePath initial_path(download_file_->FullPath());
+  EXPECT_TRUE(file_util::PathExists(initial_path));
+
+  const char* chunks1[] = { kTestData1, kTestData2 };
+  ::testing::Sequence s1;
+  SetupDataAppend(chunks1, 2, s1);
+  SetupFinishPipe(content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED, s1);
+  EXPECT_CALL(*(download_manager_.get()),
+              OnDownloadInterrupted(
+                  DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
+                  strlen(kTestData1) + strlen(kTestData2), _,
+                  content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED));
+  sink_callback_.Run();
+  VerifyPipeAndSize();
+  DestroyDownloadFile(0);
 }
 
 // Send some data, wait 3/4s of a second, run the message loop, and
 // confirm the values the DownloadManager received are correct.
 TEST_F(DownloadFileTest, ConfirmUpdate) {
-  CreateDownloadFile(&download_file_, 0, true);
+  CreateDownloadFile(0, true);
   ASSERT_EQ(net::OK, download_file_->Initialize());
 
-  AppendDataToFile(&download_file_, kTestData1);
-  AppendDataToFile(&download_file_, kTestData2);
+  const char* chunks1[] = { kTestData1, kTestData2 };
+  AppendDataToFile(chunks1, 2);
 
   // Run the message loops for 750ms and check for results.
   loop_.PostDelayedTask(FROM_HERE, MessageLoop::QuitClosure(),
@@ -239,6 +426,6 @@ TEST_F(DownloadFileTest, ConfirmUpdate) {
             bytes_);
   EXPECT_EQ(download_file_->GetHashState(), hash_state_);
 
-  download_file_->Finish();
-  DestroyDownloadFile(&download_file_, 0);
+  FinishPipe(content::DOWNLOAD_INTERRUPT_REASON_NONE);
+  DestroyDownloadFile(0);
 }