Download code cleanup: share most of the code between DownloadFile and SaveFile.

I'm not really happy about BaseFile, but removing the remaining differences
will require changes to DownloadFileManager and SaveFileManager.
I prefer to do that in small steps.

TEST=unit_tests, browser_tests, ui_tests
BUG=48913

Review URL: http://codereview.chromium.org/3164039

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57688 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 460 insertions, 354 deletions

diff --git a/chrome/browser/download/base_file.cc b/chrome/browser/download/base_file.cc
new file mode 100644
index 0000000..0bf74eb
--- /dev/null
+++ b/chrome/browser/download/base_file.cc
@@ -0,0 +1,195 @@
+// Copyright (c) 2010 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 "chrome/browser/download/base_file.h"
+
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "net/base/file_stream.h"
+#include "net/base/net_errors.h"
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/download/download_util.h"
+
+#if defined(OS_WIN)
+#include "chrome/common/win_safe_util.h"
+#elif defined(OS_MACOSX)
+#include "chrome/browser/cocoa/file_metadata.h"
+#endif
+
+BaseFile::BaseFile(const FilePath& full_path,
+                   const GURL& source_url,
+                   const GURL& referrer_url,
+                   const linked_ptr<net::FileStream>& file_stream)
+    : full_path_(full_path),
+      path_renamed_(false),
+      source_url_(source_url),
+      referrer_url_(referrer_url),
+      file_stream_(file_stream),
+      bytes_so_far_(0),
+      power_save_blocker_(true) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+}
+
+BaseFile::~BaseFile() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  if (in_progress())
+    Cancel();
+  Close();
+}
+
+bool BaseFile::Initialize() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  if (!full_path_.empty() ||
+      download_util::CreateTemporaryFileForDownload(&full_path_))
+    return Open();
+  return false;
+}
+
+bool BaseFile::AppendDataToFile(const char* data, size_t data_len) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+
+  if (!file_stream_.get())
+    return false;
+
+  // TODO(phajdan.jr): get rid of this check.
+  if (data_len == 0)
+    return true;
+
+  bytes_so_far_ += data_len;
+
+  // FIXME bug 595247: handle errors on file writes.
+  size_t written = file_stream_->Write(data, data_len, NULL);
+  return (written == data_len);
+}
+
+bool BaseFile::Rename(const FilePath& new_path, bool is_final_rename) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+
+  // Save the information whether the download is in progress because
+  // it will be overwritten by closing the file.
+  bool saved_in_progress = in_progress();
+
+  // If the new path is same as the old one, there is no need to perform the
+  // following renaming logic.
+  if (new_path == full_path_) {
+    path_renamed_ = is_final_rename;
+
+    // Don't close the file if we're not done (finished or canceled).
+    if (!saved_in_progress)
+      Close();
+
+    return true;
+  }
+
+  Close();
+
+  file_util::CreateDirectory(new_path.DirName());
+
+#if defined(OS_WIN)
+  // We cannot rename because rename will keep the same security descriptor
+  // on the destination file. We want to recreate the security descriptor
+  // with the security that makes sense in the new path.
+  if (!file_util::RenameFileAndResetSecurityDescriptor(full_path_, new_path))
+    return false;
+#elif defined(OS_POSIX)
+  {
+    // Similarly, on Unix, we're moving a temp file created with permissions
+    // 600 to |new_path|. Here, we try to fix up the destination file with
+    // appropriate permissions.
+    struct stat st;
+    // First check the file existence and create an empty file if it doesn't
+    // exist.
+    if (!file_util::PathExists(new_path))
+      file_util::WriteFile(new_path, "", 0);
+    bool stat_succeeded = (stat(new_path.value().c_str(), &st) == 0);
+
+    // TODO(estade): Move() falls back to copying and deleting when a simple
+    // rename fails. Copying sucks for large downloads. crbug.com/8737
+    if (!file_util::Move(full_path_, new_path))
+      return false;
+
+    if (stat_succeeded)
+      chmod(new_path.value().c_str(), st.st_mode);
+  }
+#endif
+
+  full_path_ = new_path;
+  path_renamed_ = is_final_rename;
+
+  // We don't need to re-open the file if we're done (finished or canceled).
+  if (!saved_in_progress)
+    return true;
+
+  if (!Open())
+    return false;
+
+  return true;
+}
+
+void BaseFile::Cancel() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  Close();
+  if (!full_path_.empty())
+    file_util::Delete(full_path_, false);
+}
+
+void BaseFile::Finish() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  Close();
+}
+
+void BaseFile::AnnotateWithSourceInformation() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+#if defined(OS_WIN)
+  // Sets the Zone to tell Windows that this file comes from the internet.
+  // We ignore the return value because a failure is not fatal.
+  win_util::SetInternetZoneIdentifier(full_path_);
+#elif defined(OS_MACOSX)
+  file_metadata::AddQuarantineMetadataToFile(full_path_, source_url_,
+                                             referrer_url_);
+  file_metadata::AddOriginMetadataToFile(full_path_, source_url_,
+                                         referrer_url_);
+#endif
+}
+
+bool BaseFile::Open() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  DCHECK(!full_path_.empty());
+
+  // Create a new file steram if it is not provided.
+  if (!file_stream_.get()) {
+    file_stream_.reset(new net::FileStream);
+    if (file_stream_->Open(full_path_,
+                           base::PLATFORM_FILE_OPEN_ALWAYS |
+                               base::PLATFORM_FILE_WRITE) != net::OK) {
+      file_stream_.reset();
+      return false;
+    }
+
+    // We may be re-opening the file after rename. Always make sure we're
+    // writing at the end of the file.
+    if (file_stream_->Seek(net::FROM_END, 0) < 0) {
+      file_stream_.reset();
+      return false;
+    }
+  }
+
+#if defined(OS_WIN)
+  AnnotateWithSourceInformation();
+#endif
+  return true;
+}
+
+void BaseFile::Close() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  if (file_stream_.get()) {
+#if defined(OS_CHROMEOS)
+    // Currently we don't really care about the return value, since if it fails
+    // theres not much we can do.  But we might in the future.
+    file_stream_->Flush();
+#endif
+    file_stream_->Close();
+    file_stream_.reset();
+  }
+}
diff --git a/chrome/browser/download/base_file.h b/chrome/browser/download/base_file.h
new file mode 100644
index 0000000..fa3873f
--- /dev/null
+++ b/chrome/browser/download/base_file.h
@@ -0,0 +1,82 @@
+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_DOWNLOAD_BASE_FILE_H_
+#define CHROME_BROWSER_DOWNLOAD_BASE_FILE_H_
+#pragma once
+
+#include "base/file_path.h"
+#include "base/linked_ptr.h"
+#include "chrome/browser/power_save_blocker.h"
+#include "googleurl/src/gurl.h"
+
+namespace net {
+class FileStream;
+}
+
+// File being downloaded and saved to disk. This is a base class
+// for DownloadFile and SaveFile, which keep more state information.
+class BaseFile {
+ public:
+  BaseFile(const FilePath& full_path,
+           const GURL& source_url,
+           const GURL& referrer_url,
+           const linked_ptr<net::FileStream>& file_stream);
+  ~BaseFile();
+
+  bool Initialize();
+
+  // Write a new chunk of data to the file. Returns true on success (all bytes
+  // written to the file).
+  bool AppendDataToFile(const char* data, size_t data_len);
+
+  // Rename the download file. Returns true on success.
+  // |path_renamed_| is set to true only if |is_final_rename| is true.
+  // Marked virtual for testing.
+  virtual bool Rename(const FilePath& full_path, bool is_final_rename);
+
+  // Abort the download and automatically close the file.
+  void Cancel();
+
+  // Indicate that the download has finished. No new data will be received.
+  void Finish();
+
+  // Informs the OS that this file came from the internet.
+  void AnnotateWithSourceInformation();
+
+  FilePath full_path() const { return full_path_; }
+  bool path_renamed() const { return path_renamed_; }
+  bool in_progress() const { return file_stream_ != NULL; }
+  int64 bytes_so_far() const { return bytes_so_far_; }
+
+ protected:
+  bool Open();
+  void Close();
+
+  // Full path to the file including the file name.
+  FilePath full_path_;
+
+  // Whether the download is still using its initial temporary path.
+  bool path_renamed_;
+
+ private:
+  // Source URL for the file being downloaded.
+  GURL source_url_;
+
+  // The URL where the download was initiated.
+  GURL referrer_url_;
+
+  // OS file stream for writing
+  linked_ptr<net::FileStream> file_stream_;
+
+  // Amount of data received up so far, in bytes.
+  int64 bytes_so_far_;
+
+  // RAII handle to keep the system from sleeping while we're downloading.
+  PowerSaveBlocker power_save_blocker_;
+
+  DISALLOW_COPY_AND_ASSIGN(BaseFile);
+};
+
+#endif  // CHROME_BROWSER_DOWNLOAD_BASE_FILE_H_
diff --git a/chrome/browser/download/base_file_unittest.cc b/chrome/browser/download/base_file_unittest.cc
new file mode 100644
index 0000000..d1b93a9
--- /dev/null
+++ b/chrome/browser/download/base_file_unittest.cc
@@ -0,0 +1,154 @@
+// Copyright (c) 2010 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 "base/file_util.h"
+#include "base/message_loop.h"
+#include "base/scoped_temp_dir.h"
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/download/base_file.h"
+#include "net/base/file_stream.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+const char kTestData1[] = "Let's write some data to the file!\n";
+const char kTestData2[] = "Writing more data.\n";
+const char kTestData3[] = "Final line.";
+
+class BaseFileTest : public testing::Test {
+ public:
+  BaseFileTest() : file_thread_(ChromeThread::FILE, &message_loop_) {
+  }
+
+  virtual void SetUp() {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    base_file_.reset(new BaseFile(FilePath(), GURL(), GURL(), file_stream_));
+  }
+
+  virtual void TearDown() {
+    EXPECT_FALSE(base_file_->in_progress());
+    EXPECT_EQ(static_cast<int64>(expected_data_.size()),
+              base_file_->bytes_so_far());
+
+    if (!expected_data_.empty()) {
+      // Make sure the data has been properly written to disk.
+      std::string disk_data;
+      EXPECT_TRUE(file_util::ReadFileToString(base_file_->full_path(),
+                                              &disk_data));
+      EXPECT_EQ(expected_data_, disk_data);
+    }
+
+    // Make sure the mock ChromeThread outlives the BaseFile to satisfy
+    // thread checks inside it.
+    base_file_.reset();
+  }
+
+  void AppendDataToFile(const std::string& data) {
+    ASSERT_TRUE(base_file_->in_progress());
+    base_file_->AppendDataToFile(data.data(), data.size());
+    expected_data_ += data;
+    EXPECT_EQ(static_cast<int64>(expected_data_.size()),
+              base_file_->bytes_so_far());
+  }
+
+ protected:
+  linked_ptr<net::FileStream> file_stream_;
+
+  // BaseClass instance we are testing.
+  scoped_ptr<BaseFile> base_file_;
+
+  // Temporary directory for renamed downloads.
+  ScopedTempDir temp_dir_;
+
+ private:
+  // Keep track of what data should be saved to the disk file.
+  std::string expected_data_;
+
+  // Mock file thread to satisfy debug checks in BaseFile.
+  MessageLoop message_loop_;
+  ChromeThread file_thread_;
+};
+
+// Test the most basic scenario: just create the object and do a sanity check
+// on all its accessors. This is actually a case that rarely happens
+// in production, where we would at least Initialize it.
+TEST_F(BaseFileTest, CreateDestroy) {
+  EXPECT_EQ(FilePath().value(), base_file_->full_path().value());
+  EXPECT_FALSE(base_file_->path_renamed());
+}
+
+// Cancel the download explicitly.
+TEST_F(BaseFileTest, Cancel) {
+  ASSERT_TRUE(base_file_->Initialize());
+  EXPECT_TRUE(file_util::PathExists(base_file_->full_path()));
+  base_file_->Cancel();
+  EXPECT_FALSE(file_util::PathExists(base_file_->full_path()));
+  EXPECT_NE(FilePath().value(), base_file_->full_path().value());
+  EXPECT_FALSE(base_file_->path_renamed());
+}
+
+// Write data to the file once.
+TEST_F(BaseFileTest, SingleWrite) {
+  ASSERT_TRUE(base_file_->Initialize());
+  AppendDataToFile(kTestData1);
+  base_file_->Finish();
+
+  EXPECT_FALSE(base_file_->path_renamed());
+}
+
+// Write data to the file multiple times.
+TEST_F(BaseFileTest, MultipleWrites) {
+  ASSERT_TRUE(base_file_->Initialize());
+  AppendDataToFile(kTestData1);
+  AppendDataToFile(kTestData2);
+  AppendDataToFile(kTestData3);
+  base_file_->Finish();
+
+  EXPECT_FALSE(base_file_->path_renamed());
+}
+
+// Rename the file after all writes to it.
+TEST_F(BaseFileTest, WriteThenRename) {
+  ASSERT_TRUE(base_file_->Initialize());
+
+  FilePath initial_path(base_file_->full_path());
+  EXPECT_TRUE(file_util::PathExists(initial_path));
+  FilePath new_path(temp_dir_.path().AppendASCII("NewFile"));
+  EXPECT_FALSE(file_util::PathExists(new_path));
+
+  AppendDataToFile(kTestData1);
+
+  EXPECT_TRUE(base_file_->Rename(new_path, true));
+  EXPECT_FALSE(file_util::PathExists(initial_path));
+  EXPECT_TRUE(file_util::PathExists(new_path));
+
+  base_file_->Finish();
+
+  EXPECT_TRUE(base_file_->path_renamed());
+}
+
+// Rename the file while the download is still in progress.
+TEST_F(BaseFileTest, RenameWhileInProgress) {
+  ASSERT_TRUE(base_file_->Initialize());
+
+  FilePath initial_path(base_file_->full_path());
+  EXPECT_TRUE(file_util::PathExists(initial_path));
+  FilePath new_path(temp_dir_.path().AppendASCII("NewFile"));
+  EXPECT_FALSE(file_util::PathExists(new_path));
+
+  AppendDataToFile(kTestData1);
+
+  EXPECT_TRUE(base_file_->in_progress());
+  EXPECT_TRUE(base_file_->Rename(new_path, true));
+  EXPECT_FALSE(file_util::PathExists(initial_path));
+  EXPECT_TRUE(file_util::PathExists(new_path));
+
+  AppendDataToFile(kTestData2);
+
+  base_file_->Finish();
+
+  EXPECT_TRUE(base_file_->path_renamed());
+}
+
+}  // namespace
diff --git a/chrome/browser/download/download_file.cc b/chrome/browser/download/download_file.cc
index 3c3b6d9..c877a6e 100644
--- a/chrome/browser/download/download_file.cc
+++ b/chrome/browser/download/download_file.cc
@@ -5,192 +5,32 @@
 #include "chrome/browser/download/download_file.h"
 
 #include "base/file_util.h"
-#include "build/build_config.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/history/download_types.h"
-#include "net/base/net_errors.h"
-
-#if defined(OS_WIN)
-#include "chrome/common/win_safe_util.h"
-#elif defined(OS_MACOSX)
-#include "chrome/browser/cocoa/file_metadata.h"
-#endif
 
 DownloadFile::DownloadFile(const DownloadCreateInfo* info)
-    : file_stream_(info->save_info.file_stream),
-      source_url_(info->url),
-      referrer_url_(info->referrer_url),
+    : BaseFile(info->save_info.file_path,
+               info->url,
+               info->referrer_url,
+               info->save_info.file_stream),
       id_(info->download_id),
       child_id_(info->child_id),
-      request_id_(info->request_id),
-      full_path_(info->save_info.file_path),
-      path_renamed_(false),
-      dont_sleep_(true) {
+      request_id_(info->request_id) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
 }
 
 DownloadFile::~DownloadFile() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-  if (in_progress())
-    Cancel();
-  Close();
-}
-
-bool DownloadFile::Initialize() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-  if (!full_path_.empty() ||
-      download_util::CreateTemporaryFileForDownload(&full_path_))
-    return Open();
-  return false;
-}
-
-bool DownloadFile::AppendDataToFile(const char* data, size_t data_len) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-
-  if (!file_stream_.get())
-    return false;
-
-  // FIXME bug 595247: handle errors on file writes.
-  size_t written = file_stream_->Write(data, data_len, NULL);
-  return (written == data_len);
-}
-
-void DownloadFile::Cancel() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-  Close();
-  if (!full_path_.empty())
-    file_util::Delete(full_path_, false);
-}
-
-// The UI has provided us with our finalized name.
-bool DownloadFile::Rename(const FilePath& new_path, bool is_final_rename) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-
-  // Save the information whether the download is in progress because
-  // it will be overwritten by closing the file.
-  bool saved_in_progress = in_progress();
-
-  // If the new path is same as the old one, there is no need to perform the
-  // following renaming logic.
-  if (new_path == full_path_) {
-    path_renamed_ = is_final_rename;
-
-    // Don't close the file if we're not done (finished or canceled).
-    if (!saved_in_progress)
-      Close();
-
-    return true;
-  }
-
-  Close();
-
-  file_util::CreateDirectory(new_path.DirName());
-
-#if defined(OS_WIN)
-  // We cannot rename because rename will keep the same security descriptor
-  // on the destination file. We want to recreate the security descriptor
-  // with the security that makes sense in the new path.
-  if (!file_util::RenameFileAndResetSecurityDescriptor(full_path_, new_path))
-    return false;
-#elif defined(OS_POSIX)
-  {
-    // Similarly, on Unix, we're moving a temp file created with permissions
-    // 600 to |new_path|. Here, we try to fix up the destination file with
-    // appropriate permissions.
-    struct stat st;
-    // First check the file existence and create an empty file if it doesn't
-    // exist.
-    if (!file_util::PathExists(new_path))
-      file_util::WriteFile(new_path, "", 0);
-    bool stat_succeeded = (stat(new_path.value().c_str(), &st) == 0);
-
-    // TODO(estade): Move() falls back to copying and deleting when a simple
-    // rename fails. Copying sucks for large downloads. crbug.com/8737
-    if (!file_util::Move(full_path_, new_path))
-      return false;
-
-    if (stat_succeeded)
-      chmod(new_path.value().c_str(), st.st_mode);
-  }
-#endif
-
-  full_path_ = new_path;
-  path_renamed_ = is_final_rename;
-
-  // We don't need to re-open the file if we're done (finished or canceled).
-  if (!saved_in_progress)
-    return true;
-
-  if (!Open())
-    return false;
-
-  // Move to the end of the new file.
-  if (file_stream_->Seek(net::FROM_END, 0) < 0)
-    return false;
-
-  return true;
 }
 
 void DownloadFile::DeleteCrDownload() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   FilePath crdownload = download_util::GetCrDownloadPath(full_path_);
   file_util::Delete(crdownload, false);
 }
 
-void DownloadFile::Finish() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-  Close();
-}
-
-void DownloadFile::Close() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-  if (file_stream_.get()) {
-#if defined(OS_CHROMEOS)
-    // Currently we don't really care about the return value, since if it fails
-    // theres not much we can do.  But we might in the future.
-    file_stream_->Flush();
-#endif
-    file_stream_->Close();
-    file_stream_.reset();
-  }
-}
-
-bool DownloadFile::Open() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-  DCHECK(!full_path_.empty());
-
-  // Create a new file steram if it is not provided.
-  if (!file_stream_.get()) {
-    file_stream_.reset(new net::FileStream);
-    if (file_stream_->Open(full_path_,
-                          base::PLATFORM_FILE_OPEN_ALWAYS |
-                              base::PLATFORM_FILE_WRITE) != net::OK) {
-      file_stream_.reset();
-      return false;
-    }
-  }
-
-#if defined(OS_WIN)
-  AnnotateWithSourceInformation();
-#endif
-  return true;
-}
-
-void DownloadFile::AnnotateWithSourceInformation() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-#if defined(OS_WIN)
-  // Sets the Zone to tell Windows that this file comes from the internet.
-  // We ignore the return value because a failure is not fatal.
-  win_util::SetInternetZoneIdentifier(full_path_);
-#elif defined(OS_MACOSX)
-  file_metadata::AddQuarantineMetadataToFile(full_path_, source_url_,
-                                             referrer_url_);
-  file_metadata::AddOriginMetadataToFile(full_path_, source_url_,
-                                         referrer_url_);
-#endif
-}
-
 void DownloadFile::CancelDownloadRequest(ResourceDispatcherHost* rdh) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   ChromeThread::PostTask(
diff --git a/chrome/browser/download/download_file.h b/chrome/browser/download/download_file.h
index 24025fd..ca188c9 100644
--- a/chrome/browser/download/download_file.h
+++ b/chrome/browser/download/download_file.h
@@ -7,12 +7,8 @@
 #pragma once
 
 #include "base/basictypes.h"
-#include "base/file_path.h"
-#include "base/hash_tables.h"
-#include "base/linked_ptr.h"
+#include "chrome/browser/download/base_file.h"
 #include "chrome/browser/download/download_types.h"
-#include "chrome/browser/power_save_blocker.h"
-#include "googleurl/src/gurl.h"
 
 struct DownloadCreateInfo;
 class ResourceDispatcherHost;
@@ -21,31 +17,11 @@ class ResourceDispatcherHost;
 // operations for one download. These objects live only for the duration that
 // the download is 'in progress': once the download has been completed or
 // cancelled, the DownloadFile is destroyed.
-class DownloadFile {
+class DownloadFile : public BaseFile {
  public:
   explicit DownloadFile(const DownloadCreateInfo* info);
   virtual ~DownloadFile();
 
-  bool Initialize();
-
-  // Write a new chunk of data to the file. Returns true on success (all bytes
-  // written to the file).
-  bool AppendDataToFile(const char* data, size_t data_len);
-
-  // Abort the download and automatically close the file.
-  void Cancel();
-
-  // Rename the download file. Returns 'true' if the rename was successful.
-  // path_renamed_ is set true only if |is_final_rename| is true.
-  // Marked virtual for testing.
-  virtual bool Rename(const FilePath& full_path, bool is_final_rename);
-
-  // Indicate that the download has finished. No new data will be received.
-  void Finish();
-
-  // Informs the OS that this file came from the internet.
-  void AnnotateWithSourceInformation();
-
   // Deletes its .crdownload intermediate file.
   // Marked virtual for testing.
   virtual void DeleteCrDownload();
@@ -53,27 +29,9 @@ class DownloadFile {
   // Cancels the download request associated with this file.
   void CancelDownloadRequest(ResourceDispatcherHost* rdh);
 
-  // Accessors.
   int id() const { return id_; }
-  bool path_renamed() const { return path_renamed_; }
-  bool in_progress() const { return file_stream_ != NULL; }
 
  private:
-  // Open or Close the OS file stream. The stream is opened in the constructor
-  // based on creation information passed to it, and automatically closed in
-  // the destructor.
-  void Close();
-  bool Open();
-
-  // OS file stream for writing
-  linked_ptr<net::FileStream> file_stream_;
-
-  // Source URL for the file being downloaded.
-  GURL source_url_;
-
-  // The URL where the download was initiated.
-  GURL referrer_url_;
-
   // The unique identifier for this download, assigned at creation by
   // the DownloadFileManager for its internal record keeping.
   int id_;
@@ -84,15 +42,6 @@ class DownloadFile {
   // Handle for informing the ResourceDispatcherHost of a UI based cancel.
   int request_id_;
 
-  // Full path to the downloaded file.
-  FilePath full_path_;
-
-  // Whether the download is still using its initial temporary path.
-  bool path_renamed_;
-
-  // RAII handle to keep the system from sleeping while we're downloading.
-  PowerSaveBlocker dont_sleep_;
-
   DISALLOW_COPY_AND_ASSIGN(DownloadFile);
 };
 
diff --git a/chrome/browser/download/save_file.cc b/chrome/browser/download/save_file.cc
index b4755ff..b05f7c5 100644
--- a/chrome/browser/download/save_file.cc
+++ b/chrome/browser/download/save_file.cc
@@ -1,106 +1,22 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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 "chrome/browser/download/save_file.h"
 
-#include "base/basictypes.h"
-#include "base/file_util.h"
 #include "base/logging.h"
-#include "base/path_service.h"
-#include "base/string_util.h"
-#include "chrome/browser/download/save_types.h"
-#if defined(OS_WIN)
-#include "chrome/common/win_safe_util.h"
-#endif
+#include "chrome/browser/chrome_thread.h"
+#include "net/base/file_stream.h"
 
 SaveFile::SaveFile(const SaveFileCreateInfo* info)
-    : info_(info),
-      file_(NULL),
-      bytes_so_far_(0),
-      path_renamed_(false),
-      in_progress_(true) {
+    : BaseFile(FilePath(), info->url, GURL(), linked_ptr<net::FileStream>()),
+      info_(info) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+
   DCHECK(info);
   DCHECK(info->path.empty());
-  if (file_util::CreateTemporaryFile(&full_path_))
-    Open("wb");
 }
 
 SaveFile::~SaveFile() {
-  Close();
-}
-
-// Return false indicate that we got disk error, save file manager will tell
-// SavePackage this error, then SavePackage will call its Cancel() method to
-// cancel whole save job.
-bool SaveFile::AppendDataToFile(const char* data, size_t data_len) {
-  if (file_) {
-    if (data_len == fwrite(data, 1, data_len, file_)) {
-      bytes_so_far_ += data_len;
-      return true;
-    } else {
-      Close();
-      return false;
-    }
-  }
-  // No file_, treat it as disk error.
-  return false;
-}
-
-void SaveFile::Cancel() {
-  Close();
-  // If this job has been canceled, and it has created file,
-  // We need to delete this created file.
-  if (!full_path_.empty()) {
-    file_util::Delete(full_path_, false);
-  }
-}
-
-// Rename the file when we have final name.
-bool SaveFile::Rename(const FilePath& new_path) {
-  Close();
-
-  DCHECK(!path_renamed());
-  // We cannot rename because rename will keep the same security descriptor
-  // on the destination file. We want to recreate the security descriptor
-  // with the security that makes sense in the new path.
-  if (!file_util::CopyFile(full_path_, new_path))
-    return false;
-
-  file_util::Delete(full_path_, false);
-
-  full_path_ = new_path;
-  path_renamed_ = true;
-
-  // Still in saving process, reopen the file.
-  if (in_progress_ && !Open("a+b"))
-    return false;
-  return true;
-}
-
-void SaveFile::Finish() {
-  Close();
-  in_progress_ = false;
-}
-
-void SaveFile::Close() {
-  if (file_) {
-    file_util::CloseFile(file_);
-    file_ = NULL;
-  }
-}
-
-bool SaveFile::Open(const char* open_mode) {
-  DCHECK(!full_path_.empty());
-  file_ = file_util::OpenFile(full_path_, open_mode);
-  if (!file_) {
-    return false;
-  }
-#if defined(OS_WIN)
-  // Sets the zone to tell Windows that this file comes from the Internet.
-  // We ignore the return value because a failure is not fatal.
-  // TODO(port): Similarly mark on Mac.
-  win_util::SetInternetZoneIdentifier(full_path_);
-#endif
-  return true;
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
 }
diff --git a/chrome/browser/download/save_file.h b/chrome/browser/download/save_file.h
index 9960e74..5932c70 100644
--- a/chrome/browser/download/save_file.h
+++ b/chrome/browser/download/save_file.h
@@ -2,13 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_DOWNLOAD_SAVE_FILE_H__
-#define CHROME_BROWSER_DOWNLOAD_SAVE_FILE_H__
+#ifndef CHROME_BROWSER_DOWNLOAD_SAVE_FILE_H_
+#define CHROME_BROWSER_DOWNLOAD_SAVE_FILE_H_
 #pragma once
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/scoped_ptr.h"
+#include "chrome/browser/download/base_file.h"
 #include "chrome/browser/download/save_types.h"
 
 // SaveFile ----------------------------------------------------------------
@@ -18,22 +19,11 @@
 // the saving job is 'in progress': once the saving job has been completed or
 // canceled, the SaveFile is destroyed. One SaveFile object represents one item
 // in a save session.
-class SaveFile {
+class SaveFile : public BaseFile {
  public:
   explicit SaveFile(const SaveFileCreateInfo* info);
   ~SaveFile();
 
-  // Write a new chunk of data to the file. Returns true on success.
-  bool AppendDataToFile(const char* data, size_t data_len);
-
-  // Abort the saving job and automatically close the file.
-  void Cancel();
-
-  // Rename the saved file. Returns 'true' if the rename was successful.
-  bool Rename(const FilePath& full_path);
-
-  void Finish();
-
   // Accessors.
   int save_id() const { return info_->save_id; }
   int render_process_id() const { return info_->render_process_id; }
@@ -43,37 +33,10 @@ class SaveFile {
     return info_->save_source;
   }
 
-  int64 bytes_so_far() const { return bytes_so_far_; }
-  FilePath full_path() const { return full_path_; }
-  bool path_renamed() const { return path_renamed_; }
-  bool in_progress() const { return in_progress_; }
-
  private:
-  // Open or Close the OS file handle. The file is opened in the constructor
-  // based on creation information passed to it, and automatically closed in
-  // the destructor.
-  void Close();
-  bool Open(const char* open_mode);
-
   scoped_ptr<const SaveFileCreateInfo> info_;
 
-  // OS file handle for writing
-  FILE* file_;
-
-  // Amount of data received up to this point. We may not know in advance how
-  // much data to expect since some servers don't provide that information.
-  int64 bytes_so_far_;
-
-  // Full path to the saved file including the file name.
-  FilePath full_path_;
-
-  // Whether the saved file is still using its initial temporary path.
-  bool path_renamed_;
-
-  // Whether the saved file is still receiving data.
-  bool in_progress_;
-
   DISALLOW_COPY_AND_ASSIGN(SaveFile);
 };
 
-#endif  // CHROME_BROWSER_DOWNLOAD_SAVE_FILE_H__
+#endif  // CHROME_BROWSER_DOWNLOAD_SAVE_FILE_H_
diff --git a/chrome/browser/download/save_file_manager.cc b/chrome/browser/download/save_file_manager.cc
index f106d6a..00e9251 100644
--- a/chrome/browser/download/save_file_manager.cc
+++ b/chrome/browser/download/save_file_manager.cc
@@ -220,6 +220,10 @@ void SaveFileManager::StartSave(SaveFileCreateInfo* info) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   DCHECK(info);
   SaveFile* save_file = new SaveFile(info);
+
+  // TODO(phajdan.jr): We should check the return value and handle errors here.
+  save_file->Initialize();
+
   DCHECK(!LookupSaveFile(info->save_id));
   save_file_map_[info->save_id] = save_file;
   info->path = save_file->full_path();
@@ -486,7 +490,7 @@ void SaveFileManager::RenameAllFiles(
     if (it != save_file_map_.end()) {
       SaveFile* save_file = it->second;
       DCHECK(!save_file->in_progress());
-      save_file->Rename(i->second);
+      save_file->Rename(i->second, true);
       delete save_file;
       save_file_map_.erase(it);
     }
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 5f8e8ea..e515aad 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1276,6 +1276,8 @@
         'browser/dom_ui/tips_handler.h',
         'browser/dom_ui/value_helper.cc',
         'browser/dom_ui/value_helper.h',
+        'browser/download/base_file.cc',
+        'browser/download/base_file.h',
         'browser/download/download_exe.cc',
         'browser/download/download_file.cc',
         'browser/download/download_file.h',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 9888c23..f1c716d 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1044,6 +1044,7 @@
         'browser/dom_ui/dom_ui_unittest.cc',
         'browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc',
         'browser/dom_ui/shown_sections_handler_unittest.cc',
+        'browser/download/base_file_unittest.cc',
         'browser/download/download_manager_unittest.cc',
         'browser/download/download_request_infobar_delegate_unittest.cc',
         'browser/download/download_request_limiter_unittest.cc',