Created an interface for DownloadFile, for use in unit tests.

chrome/browser/download/download_manager_unittest.cc

 // Copyright (c) 2011 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/i18n/number_formatting.h"
 #include "base/i18n/rtl.h"
 #include "base/memory/scoped_ptr.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 "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/browser/download/download_buffer.h"
 #include "content/browser/download/download_create_info.h"
-#include "content/browser/download/download_file.h"
+#include "content/browser/download/download_file_impl.h"
 #include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_id_factory.h"
 #include "content/browser/download/download_item.h"
 #include "content/browser/download/download_manager.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/download_status_updater.h"
 #include "content/browser/download/interrupt_reasons.h"
 #include "content/browser/download/mock_download_manager.h"
 #include "content/test/test_browser_thread.h"
 #include "grit/generated_resources.h"
 #include "net/base/io_buffer.h"
-#include "net/base/mock_file_stream.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gmock_mutant.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/text/bytes_formatting.h"
 
 using content::BrowserThread;
 
 DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain";
 
 class DownloadManagerTest : public testing::Test {
  public:
   static const char* kTestData;
   static const size_t kTestDataLen;
 
   DownloadManagerTest()
       : profile_(new TestingProfile()),
         download_manager_delegate_(new ChromeDownloadManagerDelegate(
             profile_.get())),
         id_factory_(new DownloadIdFactory(kValidIdDomain)),
-        download_manager_(new MockDownloadManager(

[Randy Smith (Not in Mondays), 2011/11/10 21:58:43]

Why this change here?  I thought that was the other CL?

[ahendrickson, 2011/11/13 00:50:21]

Ah, yes, this is a result of splitting a CL in two, not that it changes
functionality in any way.

Reverted.

+        download_manager_(new DownloadManager(
             download_manager_delegate_,
             id_factory_,
             &download_status_updater_)),
         ui_thread_(BrowserThread::UI, &message_loop_),
         file_thread_(BrowserThread::FILE, &message_loop_),
         download_buffer_(new content::DownloadBuffer) {
     download_manager_->Init(profile_.get());
     download_manager_delegate_->SetDownloadManager(download_manager_);
   }
 
@@ skipping 85 matching lines @@
         safety_state != DownloadItem::SAFE : safety_state == DownloadItem::SAFE;
   }
 
   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.  Uses MockFileStream.
-// Note:  This can't be in an anonymous namespace because it must be declared
-// as a friend of |DownloadFile| in order to access its private members.
-class DownloadFileWithMockStream : public DownloadFile {
+// A DownloadFile that we can inject errors into.
+class DownloadFileWithErrors : public DownloadFileImpl {

[Randy Smith (Not in Mondays), 2011/11/10 21:58:43]

Just noting that in the context of non-test code I'd want to bias away from
implementation inheritance, but in test code I'm cool with it.  (In the abstract
it's a short-cut that can come back to bite you because it splits code that
works together into separate locations, but I think it's a fine shortcut for
testing.)

  public:
-  DownloadFileWithMockStream(DownloadCreateInfo* info,
-                             DownloadManager* manager,
-                             net::testing::MockFileStream* stream);
+  DownloadFileWithErrors(DownloadCreateInfo* info, DownloadManager* manager);
+  virtual ~DownloadFileWithErrors() {}
 
-  virtual ~DownloadFileWithMockStream() {}
+  // BaseFile delegated functions.
+  virtual net::Error Initialize(bool calculate_hash);
+  virtual net::Error AppendDataToFile(const char* data, size_t data_len);
+  virtual net::Error Rename(const FilePath& full_path);
 
-  void SetForcedError(int error);
+  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_; }
 
- protected:
-  // This version creates a |MockFileStream| instead of a |FileStream|.
-  virtual void CreateFileStream() OVERRIDE;
+ 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_;
 };
 
-DownloadFileWithMockStream::DownloadFileWithMockStream(
-    DownloadCreateInfo* info,
-    DownloadManager* manager,
-    net::testing::MockFileStream* stream)
-    : DownloadFile(info, new DownloadRequestHandle(), manager) {
-  DCHECK(file_stream_ == NULL);
-  file_stream_.reset(stream);
+DownloadFileWithErrors::DownloadFileWithErrors(DownloadCreateInfo* info,
+                                               DownloadManager* manager)
+    : DownloadFileImpl(info, new DownloadRequestHandle(), manager),
+      forced_error_(net::OK) {
 }
 
-void DownloadFileWithMockStream::SetForcedError(int error)
-{
-  // |file_stream_| can only be set in the constructor and in
-  // CreateFileStream(), both of which insure that it is a |MockFileStream|.
-  net::testing::MockFileStream* mock_stream =
-    static_cast<net::testing::MockFileStream *>(file_stream_.get());
-  mock_stream->set_forced_error(error);
+net::Error DownloadFileWithErrors::Initialize(bool calculate_hash) {
+  return ReturnError(DownloadFileImpl::Initialize(calculate_hash));
 }
-void DownloadFileWithMockStream::CreateFileStream() {
-  file_stream_.reset(new net::testing::MockFileStream);
+
+net::Error DownloadFileWithErrors::AppendDataToFile(const char* data,
+                                                    size_t data_len) {
+  return ReturnError(DownloadFileImpl::AppendDataToFile(data, data_len));
+}
+
+net::Error DownloadFileWithErrors::Rename(const FilePath& full_path) {
+  return ReturnError(DownloadFileImpl::Rename(full_path));
 }
 
 namespace {
 
 const struct {
   const char* url;
   const char* mime_type;
   bool save_as;
   bool prompt_for_download;
   bool expected_save_as;
@@ skipping 60 matching lines @@
   // Safe download, download finishes AFTER file name determined.
   // Needs to be renamed twice.
   { FILE_PATH_LITERAL("foo.zip"), false, false, false, 2, },
   // Dangerous download, download finishes AFTER file name determined.
   // Needs to be renamed only once.
   { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), true, false, false, 1, },
   { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), false, true, false, 1, },
   { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), true, true, false, 1, },
 };
 
-class MockDownloadFile : public DownloadFile {
+class GMockDownloadFile : public DownloadFileImpl {
  public:
-  MockDownloadFile(DownloadCreateInfo* info, DownloadManager* manager)
-      : DownloadFile(info, new DownloadRequestHandle(), manager),
+  GMockDownloadFile(DownloadCreateInfo* info, DownloadManager* manager)
+      : DownloadFileImpl(info, new DownloadRequestHandle(), manager),
         renamed_count_(0) { }
-  virtual ~MockDownloadFile() { Destructed(); }
+  virtual ~GMockDownloadFile() { Destructed(); }
   MOCK_METHOD1(Rename, net::Error(const FilePath&));
   MOCK_METHOD0(Destructed, void());
 
   net::Error TestMultipleRename(
       int expected_count, const FilePath& expected,
       const FilePath& path) {
     ++renamed_count_;
     EXPECT_EQ(expected_count, renamed_count_);
     EXPECT_EQ(expected.value(), path.value());
     return net::OK;
@@ skipping 93 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);
     info->download_id = DownloadId(kValidIdDomain, static_cast<int>(i));
     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 DownloadFile(info.get(), new DownloadRequestHandle(),
-                         download_manager_));
+        new DownloadFileImpl(info.get(), new DownloadRequestHandle(),
+                             download_manager_));
     AddDownloadToFileManager(info->download_id.local(), download_file);
     download_file->Initialize(false);
     download_manager_->StartDownload(info->download_id.local());
     message_loop_.RunAllPending();
 
     // SelectFileObserver will have recorded any attempt to open the
     // select file dialog.
     // Note that DownloadManager::FileSelectionCanceled() is never called.
     EXPECT_EQ(kStartDownloadCases[i].expected_save_as,
               observer.ShowedFileDialogForId(i));
   }
 }
 
 TEST_F(DownloadManagerTest, DownloadRenameTest) {
   using ::testing::_;
   using ::testing::CreateFunctor;
   using ::testing::Invoke;
   using ::testing::Return;
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kDownloadRenameCases); ++i) {
     // 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, static_cast<int>(i));
     info->prompt_user_for_save_location = false;
     info->url_chain.push_back(GURL());
     const FilePath new_path(kDownloadRenameCases[i].suggested_path);
 
-    MockDownloadFile* download_file(
-        new MockDownloadFile(info.get(), download_manager_));
+    GMockDownloadFile* download_file(
+        new GMockDownloadFile(info.get(), download_manager_));
     AddDownloadToFileManager(info->download_id.local(), download_file);
 
     // |download_file| is owned by DownloadFileManager.
     ::testing::Mock::AllowLeak(download_file);
     EXPECT_CALL(*download_file, Destructed()).Times(1);
 
     if (kDownloadRenameCases[i].expected_rename_count == 1) {
       EXPECT_CALL(*download_file, Rename(new_path)).WillOnce(Return(net::OK));
     } else {
       ASSERT_EQ(2, kDownloadRenameCases[i].expected_rename_count);
       FilePath crdownload(download_util::GetCrDownloadPath(new_path));
       EXPECT_CALL(*download_file, Rename(_))
           .WillOnce(testing::WithArgs<0>(Invoke(CreateFunctor(
-              download_file, &MockDownloadFile::TestMultipleRename,
+              download_file, &GMockDownloadFile::TestMultipleRename,
               1, crdownload))))
           .WillOnce(testing::WithArgs<0>(Invoke(CreateFunctor(
-              download_file, &MockDownloadFile::TestMultipleRename,
+              download_file, &GMockDownloadFile::TestMultipleRename,
               2, new_path))));
     }
     download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
     DownloadItem* download = GetActiveDownloadItem(i);
     ASSERT_TRUE(download != NULL);
     if (kDownloadRenameCases[i].is_dangerous_file)
       download->MarkFileDangerous();
     if (kDownloadRenameCases[i].is_dangerous_url)
       download->MarkUrlDangerous();
 
@@ skipping 26 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);
   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(download_util::GetCrDownloadPath(new_path));
 
-  MockDownloadFile* download_file(
-      new MockDownloadFile(info.get(), download_manager_));
+  GMockDownloadFile* download_file(
+      new GMockDownloadFile(info.get(), download_manager_));
   AddDownloadToFileManager(info->download_id.local(), download_file);
 
   // |download_file| is owned by DownloadFileManager.
   ::testing::Mock::AllowLeak(download_file);
   EXPECT_CALL(*download_file, Destructed()).Times(1);
 
   EXPECT_CALL(*download_file, Rename(cr_path)).WillOnce(Return(net::OK));
 
   download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
 
@@ skipping 52 matching lines @@
 }
 
 // Test the behavior of DownloadFileManager and DownloadManager in the event
 // of a file error while writing the download to disk.
 TEST_F(DownloadManagerTest, DownloadFileErrorTest) {
   // Create a temporary file and a mock stream.
   FilePath path;
   ASSERT_TRUE(file_util::CreateTemporaryFile(&path));
 
   // This file stream will be used, until the first rename occurs.
-  net::testing::MockFileStream* mock_stream = new net::testing::MockFileStream;
-  ASSERT_EQ(0, mock_stream->Open(
+  net::FileStream* stream = new net::FileStream;
+  ASSERT_EQ(0, stream->Open(
       path,
       base::PLATFORM_FILE_OPEN_ALWAYS | base::PLATFORM_FILE_WRITE));
 
   // 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);
   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);
 
   // Create a download file that we can insert errors into.
-  DownloadFileWithMockStream* download_file(new DownloadFileWithMockStream(
-      info.get(), download_manager_, mock_stream));
+  DownloadFileWithErrors* download_file(new DownloadFileWithErrors(
+      info.get(), download_manager_));
   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);
   // This will keep track of what should be displayed on the shelf.
   scoped_ptr<DownloadItemModel> download_item_model(
       new DownloadItemModel(download));
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
   scoped_ptr<ItemObserver> observer(new ItemObserver(download));
 
   // Add some data before finalizing the file name.
   UpdateData(local_id, 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);
 
   // Add more data, but an error occurs.
-  download_file->SetForcedError(net::ERR_FAILED);
+  download_file->set_forced_error(net::ERR_FAILED);
   UpdateData(local_id, 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());
   EXPECT_FALSE(download->file_externally_removed());
   EXPECT_EQ(DownloadItem::INTERRUPTED, download->state());
 
   // Check the download shelf's information.
-  size_t error_size = kTestDataLen * 2;
+  size_t error_size = kTestDataLen * 3;
   size_t total_size = kTestDataLen * 3;
   ui::DataUnits amount_units = ui::GetByteDisplayUnits(kTestDataLen);
   string16 simple_size =
       ui::FormatBytesWithUnits(error_size, amount_units, false);
   string16 simple_total = base::i18n::GetDisplayStringInLTRDirectionality(
       ui::FormatBytesWithUnits(total_size, amount_units, true));
   EXPECT_EQ(l10n_util::GetStringFUTF16(IDS_DOWNLOAD_STATUS_INTERRUPTED,
                                        simple_size,
                                        simple_total),
             download_item_model->GetStatusText());
@@ skipping 12 matching lines @@
   // 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());
   const FilePath new_path(FILE_PATH_LITERAL("foo.zip"));
   const FilePath cr_path(download_util::GetCrDownloadPath(new_path));
 
-  MockDownloadFile* download_file(
-      new MockDownloadFile(info.get(), download_manager_));
+  GMockDownloadFile* download_file(
+      new GMockDownloadFile(info.get(), download_manager_));
   AddDownloadToFileManager(info->download_id.local(), download_file);
 
   // |download_file| is owned by DownloadFileManager.
   ::testing::Mock::AllowLeak(download_file);
   EXPECT_CALL(*download_file, Destructed()).Times(1);
 
   EXPECT_CALL(*download_file, Rename(cr_path)).WillOnce(Return(net::OK));
 
   download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
 
@@ skipping 75 matching lines @@
       new DownloadItemModel(download));
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
   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 DownloadFile(info.get(), new DownloadRequestHandle(),
-                       download_manager_));
+      new DownloadFileImpl(info.get(), new DownloadRequestHandle(),
+                           download_manager_));
   download_file->Rename(cr_path);
   // This creates the .crdownload version of the file.
   download_file->Initialize(false);
   // |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);
 
@@ skipping 55 matching lines @@
       new DownloadItemModel(download));
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
   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 DownloadFile(info.get(), new DownloadRequestHandle(),
-                       download_manager_));
+      new DownloadFileImpl(info.get(), new DownloadRequestHandle(),
+                           download_manager_));
   download_file->Rename(cr_path);
   // This creates the .crdownload version of the file.
   download_file->Initialize(false);
   // |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);
 
@@ skipping 32 matching lines @@
   EXPECT_FALSE(observer->hit_state(DownloadItem::REMOVING));
   EXPECT_TRUE(observer->was_updated());
   EXPECT_FALSE(observer->was_opened());
   EXPECT_TRUE(download->file_externally_removed());
   EXPECT_EQ(DownloadItem::COMPLETE, download->state());
   EXPECT_EQ(download_item_model->GetStatusText(),
             l10n_util::GetStringUTF16(IDS_DOWNLOAD_STATUS_REMOVED));
 
   EXPECT_FALSE(file_util::PathExists(new_path));
 }