Rewrite download manager unit to be actual unit tests.

Note that this isn't intended to be complete coverage; it's an attempt to preserve coverage from the old tests while making these "real" unit tests, i.e. with every class except for the main one being tested mocked.  Thorough unit tests are intended for the future after we're more completely done with refactoring.  

BUG=107264
BUG=105200
BUG=110886

Review URL: https://chromiumcodereview.appspot.com/10344024

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

7 files changed, 640 insertions, 1172 deletions

diff --git a/content/browser/browser_context.cc b/content/browser/browser_context.cc
index 17e60d9..522ccd1 100644
--- a/content/browser/browser_context.cc
+++ b/content/browser/browser_context.cc
@@ -6,9 +6,11 @@
 
 #include "content/browser/appcache/chrome_appcache_service.h"
 #include "content/browser/dom_storage/dom_storage_context_impl.h"
+#include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_manager_impl.h"
 #include "content/browser/fileapi/browser_file_system_helper.h"
 #include "content/browser/in_process_webkit/indexed_db_context_impl.h"
+#include "content/browser/renderer_host/resource_dispatcher_host_impl.h"
 #include "content/browser/resource_context_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
@@ -75,7 +77,7 @@ void CreateQuotaManagerAndClients(BrowserContext* context) {
       context->GetPath(), context->IsOffTheRecord(),
       context->GetSpecialStoragePolicy(), quota_manager->proxy(),
       BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE));
-  context->SetUserData(kDatabaseTrackerKeyName, 
+  context->SetUserData(kDatabaseTrackerKeyName,
                        new UserDataAdapter<DatabaseTracker>(db_tracker));
 
   FilePath path = context->IsOffTheRecord() ? FilePath() : context->GetPath();
@@ -142,8 +144,16 @@ DOMStorageContextImpl* GetDOMStorageContextImpl(BrowserContext* context) {
 DownloadManager* BrowserContext::GetDownloadManager(
     BrowserContext* context) {
   if (!context->GetUserData(kDownloadManagerKeyName)) {
-    scoped_refptr<DownloadManager> download_manager = new DownloadManagerImpl(
-        GetContentClient()->browser()->GetNetLog());
+    ResourceDispatcherHostImpl* rdh = ResourceDispatcherHostImpl::Get();
+    DCHECK(rdh);
+    DownloadFileManager* file_manager = rdh->download_file_manager();
+    DCHECK(file_manager);
+    scoped_refptr<DownloadManager> download_manager =
+        new DownloadManagerImpl(
+            file_manager,
+            scoped_ptr<DownloadItemFactory>(),
+            GetContentClient()->browser()->GetNetLog());
+
     context->SetUserData(
         kDownloadManagerKeyName,
         new UserDataAdapter<DownloadManager>(download_manager));
diff --git a/content/browser/download/download_item_factory.h b/content/browser/download/download_item_factory.h
new file mode 100644
index 0000000..84110d9
--- /dev/null
+++ b/content/browser/download/download_item_factory.h
@@ -0,0 +1,63 @@
+// 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.
+//
+// The DownloadItemFactory is used to produce different DownloadItems.
+// It is separate from the DownloadManager to allow download manager
+// unit tests to control the items produced.
+
+#ifndef CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_FACTORY_H_
+#define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_FACTORY_H_
+#pragma once
+
+#include "content/browser/download/download_item_impl.h"
+
+#include <string>
+
+struct DownloadCreateInfo;
+class DownloadRequestHandleInterface;
+class FilePath;
+class GURL;
+
+namespace content {
+class DownloadId;
+class DownloadItem;
+struct DownloadPersistentStoreInfo;
+}
+
+namespace net {
+class BoundNetLog;
+}
+
+namespace content {
+
+class DownloadItemFactory {
+public:
+  virtual ~DownloadItemFactory() {}
+
+  virtual content::DownloadItem* CreatePersistedItem(
+      DownloadItemImpl::Delegate* delegate,
+      content::DownloadId download_id,
+      const content::DownloadPersistentStoreInfo& info,
+      const net::BoundNetLog& bound_net_log) = 0;
+
+  virtual content::DownloadItem* CreateActiveItem(
+      DownloadItemImpl::Delegate* delegate,
+      const DownloadCreateInfo& info,
+      DownloadRequestHandleInterface* request_handle,
+      bool is_otr,
+      const net::BoundNetLog& bound_net_log) = 0;
+
+  virtual content::DownloadItem* CreateSavePageItem(
+      DownloadItemImpl::Delegate* delegate,
+      const FilePath& path,
+      const GURL& url,
+      bool is_otr,
+      content::DownloadId download_id,
+      const std::string& mime_type,
+      const net::BoundNetLog& bound_net_log) = 0;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_FACTORY_H_
diff --git a/content/browser/download/download_item_impl_unittest.cc b/content/browser/download/download_item_impl_unittest.cc
index ed97630..d81d3d1 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -551,6 +551,47 @@ TEST_F(DownloadItemTest, CallbackAfterRenameToIntermediateName) {
   ::testing::Mock::VerifyAndClearExpectations(mock_delegate());
 }
 
+TEST_F(DownloadItemTest, Interrupted) {
+  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+
+  int64 size = 1022;
+  const std::string hash_state("Live beef");
+  const content::DownloadInterruptReason reason(
+      content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
+
+  // Confirm interrupt sets state properly.
+  item->Interrupted(size, hash_state, reason);
+  EXPECT_EQ(size, item->GetReceivedBytes());
+  EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
+  EXPECT_EQ(hash_state, item->GetHashState());
+  EXPECT_EQ(reason, item->GetLastReason());
+
+  // Cancel should result in no change.
+  item->Cancel(true);
+  EXPECT_EQ(size, item->GetReceivedBytes());
+  EXPECT_EQ(DownloadItem::INTERRUPTED, item->GetState());
+  EXPECT_EQ(hash_state, item->GetHashState());
+  EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED,
+            item->GetLastReason());
+}
+
+TEST_F(DownloadItemTest, Canceled) {
+  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+
+  // Confirm cancel sets state properly.
+  EXPECT_CALL(*mock_delegate(), DownloadCancelled(item));
+  item->Cancel(true);
+  EXPECT_EQ(DownloadItem::CANCELLED, item->GetState());
+}
+
+TEST_F(DownloadItemTest, FileRemoved) {
+  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+
+  EXPECT_EQ(false, item->GetFileExternallyRemoved());
+  item->OnDownloadedFileRemoved();
+  EXPECT_EQ(true, item->GetFileExternallyRemoved());
+}
+
 TEST(MockDownloadItem, Compiles) {
   MockDownloadItem mock_item;
 }
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index 479d07a..b374565 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -136,6 +136,42 @@ void EnsureNoPendingDownloadJobsOnIO(bool* result) {
                  download_file_manager, result));
 }
 
+class DownloadItemFactoryImpl : public content::DownloadItemFactory {
+ public:
+    DownloadItemFactoryImpl() {}
+    virtual ~DownloadItemFactoryImpl() {}
+
+    virtual content::DownloadItem* CreatePersistedItem(
+        DownloadItemImpl::Delegate* delegate,
+        content::DownloadId download_id,
+        const content::DownloadPersistentStoreInfo& info,
+        const net::BoundNetLog& bound_net_log) OVERRIDE {
+      return new DownloadItemImpl(delegate, download_id, info, bound_net_log);
+    }
+
+    virtual content::DownloadItem* CreateActiveItem(
+        DownloadItemImpl::Delegate* delegate,
+        const DownloadCreateInfo& info,
+        DownloadRequestHandleInterface* request_handle,
+        bool is_otr,
+        const net::BoundNetLog& bound_net_log) OVERRIDE {
+      return new DownloadItemImpl(delegate, info, request_handle, is_otr,
+                                  bound_net_log);
+    }
+
+    virtual content::DownloadItem* CreateSavePageItem(
+        DownloadItemImpl::Delegate* delegate,
+        const FilePath& path,
+        const GURL& url,
+        bool is_otr,
+        content::DownloadId download_id,
+        const std::string& mime_type,
+        const net::BoundNetLog& bound_net_log) OVERRIDE {
+      return new DownloadItemImpl(delegate, path, url, is_otr, download_id,
+                                  mime_type, bound_net_log);
+    }
+};
+
 }  // namespace
 
 namespace content {
@@ -152,12 +188,18 @@ bool DownloadManager::EnsureNoPendingDownloadsForTesting() {
 
 }  // namespace content
 
-DownloadManagerImpl::DownloadManagerImpl(net::NetLog* net_log)
-    : shutdown_needed_(false),
+DownloadManagerImpl::DownloadManagerImpl(
+    DownloadFileManager* file_manager,
+    scoped_ptr<content::DownloadItemFactory> factory,
+    net::NetLog* net_log)
+    : factory_(factory.Pass()),
+      shutdown_needed_(false),
       browser_context_(NULL),
-      file_manager_(NULL),
-      delegate_(NULL),
+      file_manager_(file_manager),
       net_log_(net_log) {
+  DCHECK(file_manager);
+  if (!factory_.get())
+    factory_.reset(new DownloadItemFactoryImpl());
 }
 
 DownloadManagerImpl::~DownloadManagerImpl() {
@@ -209,11 +251,11 @@ void DownloadManagerImpl::Shutdown() {
   FOR_EACH_OBSERVER(Observer, observers_, ManagerGoingDown(this));
   // TODO(benjhayden): Consider clearing observers_.
 
-  if (file_manager_) {
-    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-        base::Bind(&DownloadFileManager::OnDownloadManagerShutdown,
-                   file_manager_, make_scoped_refptr(this)));
-  }
+  DCHECK(file_manager_);
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&DownloadFileManager::OnDownloadManagerShutdown,
+                 file_manager_, make_scoped_refptr(this)));
 
   AssertContainersConsistent();
 
@@ -316,7 +358,6 @@ void DownloadManagerImpl::SearchDownloads(const string16& query,
   }
 }
 
-// Query the history service for information about all persisted downloads.
 bool DownloadManagerImpl::Init(content::BrowserContext* browser_context) {
   DCHECK(browser_context);
   DCHECK(!shutdown_needed_)  << "DownloadManager already initialized.";
@@ -324,15 +365,6 @@ bool DownloadManagerImpl::Init(content::BrowserContext* browser_context) {
 
   browser_context_ = browser_context;
 
-  // In test mode, there may be no ResourceDispatcherHostImpl.  In this case
-  // it's safe to avoid setting |file_manager_| because we only call a small
-  // set of functions, none of which need it.
-  ResourceDispatcherHostImpl* rdh = ResourceDispatcherHostImpl::Get();
-  if (rdh) {
-    file_manager_ = rdh->download_file_manager();
-    DCHECK(file_manager_);
-  }
-
   return true;
 }
 
@@ -425,8 +457,9 @@ net::BoundNetLog DownloadManagerImpl::CreateDownloadItem(
 
   net::BoundNetLog bound_net_log =
       net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
-  info->download_id = GetNextId();
-  DownloadItem* download = new DownloadItemImpl(
+  if (!info->download_id.IsValid())
+    info->download_id = GetNextId();
+  DownloadItem* download = factory_->CreateActiveItem(
       this, *info, new DownloadRequestHandle(request_handle),
       browser_context_->IsOffTheRecord(), bound_net_log);
   int32 download_id = info->download_id.local();
@@ -446,7 +479,7 @@ DownloadItem* DownloadManagerImpl::CreateSavePackageDownloadItem(
     DownloadItem::Observer* observer) {
   net::BoundNetLog bound_net_log =
       net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
-  DownloadItem* download = new DownloadItemImpl(
+  DownloadItem* download = factory_->CreateSavePageItem(
       this,
       main_file_path,
       page_url,
@@ -680,8 +713,8 @@ void DownloadManagerImpl::DownloadCancelled(DownloadItem* download) {
   // should already have been updated.
   AssertStateConsistent(download);
 
-  if (file_manager_)
-    download->OffThreadCancel(file_manager_);
+  DCHECK(file_manager_);
+  download->OffThreadCancel(file_manager_);
 }
 
 void DownloadManagerImpl::OnDownloadInterrupted(
@@ -880,7 +913,7 @@ void DownloadManagerImpl::OnPersistentStoreQueryComplete(
 
     net::BoundNetLog bound_net_log =
         net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
-    DownloadItem* download = new DownloadItemImpl(
+    DownloadItem* download = factory_->CreatePersistedItem(
         this, GetNextId(), entries->at(i), bound_net_log);
     downloads_.insert(download);
     history_downloads_[download->GetDbHandle()] = download;
@@ -1150,8 +1183,3 @@ void DownloadManagerImpl::DownloadRenamedToFinalName(
         download, download->GetFullPath());
   }
 }
-
-void DownloadManagerImpl::SetFileManagerForTesting(
-      DownloadFileManager* file_manager) {
-  file_manager_ = file_manager;
-}
diff --git a/content/browser/download/download_manager_impl.h b/content/browser/download/download_manager_impl.h
index 44a9528..0f6670a 100644
--- a/content/browser/download/download_manager_impl.h
+++ b/content/browser/download/download_manager_impl.h
@@ -1,7 +1,6 @@
 // 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.
-//
 
 #ifndef CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_IMPL_H_
 #define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_IMPL_H_
@@ -17,15 +16,25 @@
 #include "base/message_loop_helpers.h"
 #include "base/observer_list.h"
 #include "base/synchronization/lock.h"
+#include "content/browser/download/download_item_factory.h"
 #include "content/browser/download/download_item_impl.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/download_manager.h"
 
+class DownloadFileManager;
+
 class CONTENT_EXPORT DownloadManagerImpl
     : public content::DownloadManager,
       public DownloadItemImpl::Delegate {
  public:
-  explicit DownloadManagerImpl(net::NetLog* net_log);
+  // Caller guarantees that |file_manager| and |net_log| will remain valid
+  // for the lifetime of DownloadManagerImpl (until Shutdown() is called).
+  // |factory| may be a default constructed (null) scoped_ptr; if so,
+  // the DownloadManagerImpl creates and takes ownership of the
+  // default DownloadItemFactory.
+  DownloadManagerImpl(DownloadFileManager* file_manager,
+                      scoped_ptr<content::DownloadItemFactory> factory,
+                      net::NetLog* net_log);
 
   // content::DownloadManager functions.
   virtual void SetDelegate(content::DownloadManagerDelegate* delegate) OVERRIDE;
@@ -109,9 +118,6 @@ class CONTENT_EXPORT DownloadManagerImpl
   virtual void AssertStateConsistent(
       content::DownloadItem* download) const OVERRIDE;
 
-  // For unit tests only.
-  void SetFileManagerForTesting(DownloadFileManager* file_manager);
-
  private:
   typedef std::set<content::DownloadItem*> DownloadSet;
   typedef base::hash_map<int64, content::DownloadItem*> DownloadMap;
@@ -179,6 +185,9 @@ class CONTENT_EXPORT DownloadManagerImpl
   // Called when Save Page As entry is committed to the persistent store.
   void OnSavePageItemAddedToPersistentStore(int32 download_id, int64 db_handle);
 
+  // Factory for creation of downloads items.
+  scoped_ptr<content::DownloadItemFactory> factory_;
+
   // |downloads_| is the owning set for all downloads known to the
   // DownloadManager.  This includes downloads started by the user in
   // this session, downloads initialized from the history system, and
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc
index b60024d..4fe7761 100644
--- a/content/browser/download/download_manager_impl_unittest.cc
+++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -8,62 +8,43 @@
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_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/byte_stream.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_item_factory.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/browser_context.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager_delegate.h"
-#include "content/public/test/mock_download_manager.h"
+#include "content/public/test/mock_download_item.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread.h"
-#include "net/base/io_buffer.h"
 #include "net/base/net_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gmock_mutant.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if defined(USE_AURA) && defined(OS_WIN)
-// http://crbug.com/105200
-#define MAYBE_StartDownload DISABLED_StartDownload
-#define MAYBE_DownloadOverwriteTest DISABLED_DownloadOverwriteTest
-#define MAYBE_DownloadRemoveTest DISABLED_DownloadRemoveTest
-#define MAYBE_DownloadFileErrorTest DownloadFileErrorTest
-#elif defined(OS_LINUX)
-// http://crbug.com/110886 for Linux
-#define MAYBE_StartDownload DISABLED_StartDownload
-#define MAYBE_DownloadOverwriteTest DISABLED_DownloadOverwriteTest
-#define MAYBE_DownloadRemoveTest DISABLED_DownloadRemoveTest
-#define MAYBE_DownloadFileErrorTest DISABLED_DownloadFileErrorTest
-#else
-#define MAYBE_StartDownload StartDownload
-#define MAYBE_DownloadOverwriteTest DownloadOverwriteTest
-#define MAYBE_DownloadRemoveTest DownloadRemoveTest
-#define MAYBE_DownloadFileErrorTest DownloadFileErrorTest
-#endif
-
-using content::BrowserContext;
-using content::BrowserThread;
-using content::DownloadFile;
-using content::DownloadId;
+using ::testing::DoAll;
+using ::testing::Ref;
+using ::testing::Return;
+using ::testing::ReturnRef;
+using ::testing::SetArgPointee;
+using ::testing::StrictMock;
+using ::testing::_;
 using content::DownloadItem;
 using content::DownloadManager;
 using content::WebContents;
-using ::testing::NiceMock;
-using ::testing::ReturnRef;
-using ::testing::Return;
 
 namespace content {
 class ByteStreamReader;
@@ -71,1191 +52,526 @@ class ByteStreamReader;
 
 namespace {
 
-FilePath GetTempDownloadPath(const FilePath& suggested_path) {
-  return FilePath(suggested_path.value() + FILE_PATH_LITERAL(".temp"));
-}
-
-class MockDownloadFileFactory
-    : public DownloadFileManager::DownloadFileFactory {
+class MockDownloadManagerDelegate : public content::DownloadManagerDelegate {
  public:
-  MockDownloadFileFactory() {}
-
-  virtual DownloadFile* CreateFile(
-      DownloadCreateInfo* info,
-      scoped_ptr<content::ByteStreamReader> stream,
-      const DownloadRequestHandle& request_handle,
-      DownloadManager* download_manager,
-      bool calculate_hash,
-      const net::BoundNetLog& bound_net_log) OVERRIDE;
+  MockDownloadManagerDelegate();
+  virtual ~MockDownloadManagerDelegate();
+
+  MOCK_METHOD0(Shutdown, void());
+  MOCK_METHOD0(GetNextId, content::DownloadId());
+  MOCK_METHOD1(ShouldStartDownload, bool(int32));
+  MOCK_METHOD1(ChooseDownloadPath, void(DownloadItem*));
+  MOCK_METHOD2(GetIntermediatePath, FilePath(const DownloadItem&, bool*));
+  MOCK_METHOD0(GetAlternativeWebContentsToNotifyForDownload, WebContents*());
+  MOCK_METHOD1(ShouldOpenFileBasedOnExtension, bool(const FilePath&));
+  MOCK_METHOD2(ShouldCompleteDownload, bool(
+      DownloadItem*, const base::Closure&));
+  MOCK_METHOD1(ShouldOpenDownload, bool(DownloadItem*));
+  MOCK_METHOD0(GenerateFileHash, bool());
+  MOCK_METHOD1(AddItemToPersistentStore, void(DownloadItem*));
+  MOCK_METHOD1(UpdateItemInPersistentStore, void(DownloadItem*));
+  MOCK_METHOD2(UpdatePathForItemInPersistentStore,
+               void(DownloadItem*, const FilePath&));
+  MOCK_METHOD1(RemoveItemFromPersistentStore, void(DownloadItem*));
+  MOCK_METHOD2(RemoveItemsFromPersistentStoreBetween, void(
+      base::Time remove_begin, base::Time remove_end));
+  MOCK_METHOD4(GetSaveDir, void(WebContents*, FilePath*, FilePath*, bool*));
+  MOCK_METHOD5(ChooseSavePath, void(
+      WebContents*, const FilePath&, const FilePath::StringType&,
+      bool, const content::SavePackagePathPickedCallback&));
 };
 
-DownloadFile* MockDownloadFileFactory::CreateFile(
-    DownloadCreateInfo* info,
-    scoped_ptr<content::ByteStreamReader> stream,
-    const DownloadRequestHandle& request_handle,
-    DownloadManager* download_manager,
-    bool calculate_hash,
-    const net::BoundNetLog& bound_net_log) {
-  NOTREACHED();
-  return NULL;
-}
+MockDownloadManagerDelegate::MockDownloadManagerDelegate() { }
 
-class TestDownloadManagerDelegate : public content::DownloadManagerDelegate {
- public:
-  explicit TestDownloadManagerDelegate(content::DownloadManager* dm)
-      : mark_content_dangerous_(false),
-        prompt_user_for_save_location_(false),
-        should_complete_download_(true),
-        download_manager_(dm) {
-  }
+MockDownloadManagerDelegate::~MockDownloadManagerDelegate() { }
 
-  void set_download_directory(const FilePath& path) {
-    download_directory_ = path;
-  }
+class MockDownloadFileManager : public DownloadFileManager {
+ public:
+  MockDownloadFileManager();
 
-  void set_prompt_user_for_save_location(bool value) {
-    prompt_user_for_save_location_ = value;
-  }
+  MOCK_METHOD0(Shutdown, void());
+  MOCK_METHOD3(MockStartDownload,
+               content::DownloadId(DownloadCreateInfo*,
+                                   content::ByteStreamReader* stream,
+                                   const DownloadRequestHandle&));
 
-  virtual bool ShouldStartDownload(int32 download_id) OVERRIDE {
-    DownloadItem* item = download_manager_->GetActiveDownloadItem(download_id);
-    FilePath path = net::GenerateFileName(item->GetURL(),
-                                          item->GetContentDisposition(),
-                                          item->GetReferrerCharset(),
-                                          item->GetSuggestedFilename(),
-                                          item->GetMimeType(),
-                                          std::string());
-    DownloadItem::TargetDisposition disposition = item->GetTargetDisposition();
-    if (!ShouldOpenFileBasedOnExtension(path) && prompt_user_for_save_location_)
-      disposition = DownloadItem::TARGET_DISPOSITION_PROMPT;
-    item->OnTargetPathDetermined(download_directory_.Append(path),
-                                 disposition,
-                                 content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-    return true;
-  }
+  virtual content::DownloadId StartDownload(
+      scoped_ptr<DownloadCreateInfo> info,
+      scoped_ptr<content::ByteStreamReader> stream,
+      const DownloadRequestHandle& request_handle) OVERRIDE {
+    return MockStartDownload(info.get(), stream.get(), request_handle);
+  }
+
+  MOCK_METHOD1(CancelDownload, void(content::DownloadId));
+  MOCK_METHOD1(CompleteDownload, void(content::DownloadId));
+  MOCK_METHOD1(OnDownloadManagerShutdown, void(content::DownloadManager*));
+  MOCK_METHOD4(RenameInProgressDownloadFile,
+               void(content::DownloadId,
+                    const FilePath&,
+                    bool,
+                    const RenameCompletionCallback&));
+  MOCK_METHOD4(RenameCompletingDownloadFile,
+               void(content::DownloadId,
+                    const FilePath&,
+                    bool,
+                    const RenameCompletionCallback&));
+  MOCK_CONST_METHOD0(NumberOfActiveDownloads, int());
+ protected:
+  virtual ~MockDownloadFileManager();
+};
 
-  virtual FilePath GetIntermediatePath(const DownloadItem& item,
-                                       bool* ok_to_overwrite) OVERRIDE {
-    if (intermediate_path_.empty()) {
-      *ok_to_overwrite = true;
-      return GetTempDownloadPath(item.GetTargetFilePath());
-    } else {
-      *ok_to_overwrite = overwrite_intermediate_path_;
-      return intermediate_path_;
-    }
-  }
+MockDownloadFileManager::MockDownloadFileManager()
+    : DownloadFileManager(NULL) { }
 
-  virtual void ChooseDownloadPath(DownloadItem* item) OVERRIDE {
-    if (!expected_suggested_path_.empty()) {
-      EXPECT_STREQ(expected_suggested_path_.value().c_str(),
-          item->GetTargetFilePath().value().c_str());
-    }
-    if (file_selection_response_.empty()) {
-      BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE,
-          base::Bind(&DownloadManager::FileSelectionCanceled,
-                     download_manager_,
-                     item->GetId()));
-    } else {
-      BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE,
-          base::Bind(&DownloadManager::FileSelected,
-                     download_manager_,
-                     file_selection_response_,
-                     item->GetId()));
-    }
-    expected_suggested_path_.clear();
-    file_selection_response_.clear();
-  }
+MockDownloadFileManager::~MockDownloadFileManager() { }
 
-  virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path) OVERRIDE {
-    return path.Extension() == FilePath::StringType(FILE_PATH_LITERAL(".pdf"));
-  }
+class MockDownloadItemFactory
+    : public content::DownloadItemFactory,
+      public base::SupportsWeakPtr<MockDownloadItemFactory> {
+ public:
+  MockDownloadItemFactory();
+  virtual ~MockDownloadItemFactory();
 
-  virtual void AddItemToPersistentStore(DownloadItem* item) OVERRIDE {
-    static int64 db_handle = DownloadItem::kUninitializedHandle;
-    download_manager_->OnItemAddedToPersistentStore(item->GetId(), --db_handle);
-  }
+  // Access to map of created items.
+  // TODO(rdsmith): Could add type (save page, persisted, etc.)
+  // functionality if it's ever needed by consumers.
 
-  void SetFileSelectionExpectation(const FilePath& suggested_path,
-                                   const FilePath& response) {
-    expected_suggested_path_ = suggested_path;
-    file_selection_response_ = response;
-  }
+  // Returns NULL if no item of that id is present.
+  content::MockDownloadItem* GetItem(int id);
 
-  void SetMarkContentsDangerous(bool dangerous) {
-    mark_content_dangerous_ = dangerous;
-  }
+  // Remove and return an item made by the factory.
+  // Generally used during teardown.
+  content::MockDownloadItem* PopItem();
 
-  void SetIntermediatePath(const FilePath& intermediate_path,
-                           bool overwrite_intermediate_path) {
-    intermediate_path_ = intermediate_path;
-    overwrite_intermediate_path_ = overwrite_intermediate_path;
-  }
+  // Should be called when the item of this id is removed so that
+  // we don't keep dangling pointers.
+  void RemoveItem(int id);
 
-  void SetShouldCompleteDownload(bool value) {
-    should_complete_download_ = value;
-  }
-
-  void InvokeDownloadCompletionCallback() {
-    EXPECT_FALSE(download_completion_callback_.is_null());
-    download_completion_callback_.Run();
-    download_completion_callback_.Reset();
-  }
-
-  virtual bool ShouldCompleteDownload(
-      DownloadItem* item,
-      const base::Closure& complete_callback) OVERRIDE {
-    download_completion_callback_ = complete_callback;
-    if (mark_content_dangerous_) {
-      CHECK(!complete_callback.is_null());
-      BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE,
-          base::Bind(&TestDownloadManagerDelegate::MarkContentDangerous,
-                     base::Unretained(this), item->GetId()));
-      mark_content_dangerous_ = false;
-      return false;
-    } else {
-      return should_complete_download_;
-    }
-  }
+  // Overridden methods from DownloadItemFactory.
+  virtual content::DownloadItem* CreatePersistedItem(
+      DownloadItemImpl::Delegate* delegate,
+      content::DownloadId download_id,
+      const content::DownloadPersistentStoreInfo& info,
+      const net::BoundNetLog& bound_net_log) OVERRIDE;
+  virtual content::DownloadItem* CreateActiveItem(
+      DownloadItemImpl::Delegate* delegate,
+      const DownloadCreateInfo& info,
+      DownloadRequestHandleInterface* request_handle,
+      bool is_otr,
+      const net::BoundNetLog& bound_net_log) OVERRIDE;
+  virtual content::DownloadItem* CreateSavePageItem(
+      DownloadItemImpl::Delegate* delegate,
+      const FilePath& path,
+      const GURL& url,
+      bool is_otr,
+      content::DownloadId download_id,
+      const std::string& mime_type,
+      const net::BoundNetLog& bound_net_log) OVERRIDE;
 
  private:
-  void MarkContentDangerous(
-      int32 download_id) {
-    DownloadItem* item = download_manager_->GetActiveDownloadItem(download_id);
-    if (!item)
-      return;
-    item->OnContentCheckCompleted(
-        content::DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT);
-    InvokeDownloadCompletionCallback();
-  }
+  std::map<int32, content::MockDownloadItem*> items_;
 
-  FilePath download_directory_;
-  FilePath expected_suggested_path_;
-  FilePath file_selection_response_;
-  FilePath intermediate_path_;
-  bool overwrite_intermediate_path_;
-  bool mark_content_dangerous_;
-  bool prompt_user_for_save_location_;
-  bool should_complete_download_;
-  DownloadManager* download_manager_;
-  base::Closure download_completion_callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestDownloadManagerDelegate);
+  DISALLOW_COPY_AND_ASSIGN(MockDownloadItemFactory);
 };
 
-} // namespace
+MockDownloadItemFactory::MockDownloadItemFactory() {}
 
-class DownloadManagerTest : public testing::Test {
- public:
-  static const char* kTestData;
-  static const size_t kTestDataLen;
+MockDownloadItemFactory::~MockDownloadItemFactory() {}
 
-  DownloadManagerTest()
-      : browser_context(new content::TestBrowserContext()),
-        download_manager_(new DownloadManagerImpl(NULL)),
-        ui_thread_(BrowserThread::UI, &message_loop_),
-        file_thread_(BrowserThread::FILE, &message_loop_) {
-    download_manager_delegate_.reset(
-        new TestDownloadManagerDelegate(download_manager_.get()));
-    download_manager_->SetDelegate(download_manager_delegate_.get());
-    download_manager_->Init(browser_context.get());
-  }
+content::MockDownloadItem* MockDownloadItemFactory::GetItem(int id) {
+  if (items_.find(id) == items_.end())
+    return NULL;
+  return items_[id];
+}
 
-  ~DownloadManagerTest() {
-    download_manager_->Shutdown();
-    // browser_context must outlive download_manager_, so we explicitly delete
-    // download_manager_ first.
-    download_manager_ = NULL;
-    download_manager_delegate_.reset();
-    browser_context.reset(NULL);
-    message_loop_.RunAllPending();
-  }
+content::MockDownloadItem* MockDownloadItemFactory::PopItem() {
+  if (items_.empty())
+    return NULL;
 
-  // Create a temporary directory as the downloads directory.
-  bool CreateTempDownloadsDirectory() {
-    if (!scoped_download_dir_.CreateUniqueTempDir())
-      return false;
-    download_manager_delegate_->set_download_directory(
-        scoped_download_dir_.path());
-    return true;
-  }
+  std::map<int32, content::MockDownloadItem*>::iterator first_item
+      = items_.begin();
+  content::MockDownloadItem* result = first_item->second;
+  items_.erase(first_item);
+  return result;
+}
 
-  void AddDownloadToFileManager(DownloadId id, DownloadFile* download_file) {
-    file_manager()->downloads_[id] =
-      download_file;
-  }
+void MockDownloadItemFactory::RemoveItem(int id) {
+  DCHECK(items_.find(id) != items_.end());
+  items_.erase(id);
+}
 
-  void AddMockDownloadToFileManager(DownloadId id,
-                                    MockDownloadFile* download_file) {
-    AddDownloadToFileManager(id, download_file);
-    EXPECT_CALL(*download_file, GetDownloadManager())
-        .WillRepeatedly(Return(download_manager_));
-  }
+content::DownloadItem* MockDownloadItemFactory::CreatePersistedItem(
+    DownloadItemImpl::Delegate* delegate,
+    content::DownloadId download_id,
+    const content::DownloadPersistentStoreInfo& info,
+    const net::BoundNetLog& bound_net_log) {
+  int local_id = download_id.local();
+  DCHECK(items_.find(local_id) == items_.end());
 
-  void OnResponseCompleted(DownloadId download_id, int64 size,
-                           const std::string& hash) {
-    download_manager_->OnResponseCompleted(download_id.local(), size, hash);
-  }
+  content::MockDownloadItem* result =
+      new StrictMock<content::MockDownloadItem>;
+  EXPECT_CALL(*result, GetId())
+      .WillRepeatedly(Return(local_id));
+  items_[local_id] = result;
 
-  void ContinueDownloadWithPath(DownloadItem* download, const FilePath& path) {
-    download->OnTargetPathDetermined(
-        path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-        content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-    download_manager_->OnTargetPathAvailable(download);
-  }
+  return result;
+}
 
-  void OnDownloadInterrupted(DownloadId download_id, int64 size,
-                             const std::string& hash_state,
-                             content::DownloadInterruptReason reason) {
-    download_manager_->OnDownloadInterrupted(download_id.local(), size,
-                                             hash_state, reason);
-  }
+content::DownloadItem* MockDownloadItemFactory::CreateActiveItem(
+    DownloadItemImpl::Delegate* delegate,
+    const DownloadCreateInfo& info,
+    DownloadRequestHandleInterface* request_handle,
+    bool is_otr,
+    const net::BoundNetLog& bound_net_log) {
+  int local_id = info.download_id.local();
+  DCHECK(items_.find(local_id) == items_.end());
 
-  // Get the download item with ID |id|.
-  DownloadItem* GetActiveDownloadItem(DownloadId id) {
-    return download_manager_->GetActiveDownload(id.local());
-  }
+  content::MockDownloadItem* result =
+      new StrictMock<content::MockDownloadItem>;
+  EXPECT_CALL(*result, GetId())
+      .WillRepeatedly(Return(local_id));
+  items_[local_id] = result;
 
-  FilePath GetPathInDownloadsDir(const FilePath::StringType& fragment) {
-    DCHECK(scoped_download_dir_.IsValid());
-    FilePath full_path(scoped_download_dir_.path().Append(fragment));
-    return full_path.NormalizePathSeparators();
-  }
+  return result;
+}
 
- protected:
-  scoped_ptr<content::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_;
-  ScopedTempDir scoped_download_dir_;
+content::DownloadItem* MockDownloadItemFactory::CreateSavePageItem(
+    DownloadItemImpl::Delegate* delegate,
+    const FilePath& path,
+    const GURL& url,
+    bool is_otr,
+    content::DownloadId download_id,
+    const std::string& mime_type,
+    const net::BoundNetLog& bound_net_log) {
+  int local_id = download_id.local();
+  DCHECK(items_.find(local_id) == items_.end());
 
-  DownloadFileManager* file_manager() {
-    if (!file_manager_) {
-      file_manager_ = new DownloadFileManager(new MockDownloadFileFactory);
-      download_manager_->SetFileManagerForTesting(file_manager_);
-    }
-    return file_manager_;
-  }
+  content::MockDownloadItem* result =
+      new StrictMock<content::MockDownloadItem>;
+  EXPECT_CALL(*result, GetId())
+      .WillRepeatedly(Return(local_id));
+  items_[local_id] = result;
 
-  DISALLOW_COPY_AND_ASSIGN(DownloadManagerTest);
+  return result;
+}
+
+class MockBrowserContext : public content::BrowserContext {
+ public:
+  MockBrowserContext() { }
+  ~MockBrowserContext() { }
+
+  MOCK_METHOD0(GetPath, FilePath());
+  MOCK_CONST_METHOD0(IsOffTheRecord, bool());
+  MOCK_METHOD0(GetRequestContext, net::URLRequestContextGetter*());
+  MOCK_METHOD1(GetRequestContextForRenderProcess,
+               net::URLRequestContextGetter*(int renderer_child_id));
+  MOCK_METHOD0(GetRequestContextForMedia, net::URLRequestContextGetter*());
+  MOCK_METHOD0(GetResourceContext, content::ResourceContext*());
+  MOCK_METHOD0(GetDownloadManagerDelegate, content::DownloadManagerDelegate*());
+  MOCK_METHOD0(GetGeolocationPermissionContext,
+               content::GeolocationPermissionContext* ());
+  MOCK_METHOD0(GetSpeechRecognitionPreferences,
+               content::SpeechRecognitionPreferences* ());
+  MOCK_METHOD0(DidLastSessionExitCleanly, bool());
+  MOCK_METHOD0(GetSpecialStoragePolicy, quota::SpecialStoragePolicy*());
 };
 
-const char* DownloadManagerTest::kTestData = "a;sdlfalsdfjalsdkfjad";
-const size_t DownloadManagerTest::kTestDataLen =
-    strlen(DownloadManagerTest::kTestData);
+} // namespace
 
-// A DownloadFile that we can inject errors into.
-class DownloadFileWithErrors : public DownloadFileImpl {
+class DownloadManagerTest : public testing::Test {
  public:
-  DownloadFileWithErrors(DownloadCreateInfo* info,
-                         scoped_ptr<content::ByteStreamReader> stream,
-                         DownloadManager* manager,
-                         bool calculate_hash);
-  virtual ~DownloadFileWithErrors() {}
-
-  // BaseFile delegated functions.
-  virtual net::Error Initialize() OVERRIDE;
-  virtual net::Error AppendDataToFile(const char* data,
-                                      size_t data_len) OVERRIDE;
-  virtual net::Error Rename(const FilePath& full_path) OVERRIDE;
-
-  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_; }
+  static const char* kTestData;
+  static const size_t kTestDataLen;
 
- private:
-  net::Error ReturnError(net::Error function_error) {
-    if (forced_error_ != net::OK) {
-      net::Error ret = forced_error_;
-      clear_forced_error();
-      return ret;
+  DownloadManagerTest()
+      : ui_thread_(content::BrowserThread::UI, &message_loop_),
+        file_thread_(content::BrowserThread::FILE, &message_loop_),
+        next_download_id_(0) {
+  }
+
+  // We tear down everything in TearDown().
+  ~DownloadManagerTest() {}
+
+  // Create a MockDownloadItemFactory, MockDownloadManagerDelegate,
+  // and MockDownloadFileManager, then create a DownloadManager that points
+  // at all of those.
+  virtual void SetUp() {
+    DCHECK(!download_manager_.get());
+
+    mock_download_item_factory_ = (new MockDownloadItemFactory())->AsWeakPtr();
+    mock_download_manager_delegate_.reset(
+        new StrictMock<MockDownloadManagerDelegate>);
+    EXPECT_CALL(*mock_download_manager_delegate_.get(), Shutdown())
+        .WillOnce(Return());
+    mock_download_file_manager_ = new StrictMock<MockDownloadFileManager>;
+    EXPECT_CALL(*mock_download_file_manager_.get(),
+                OnDownloadManagerShutdown(_));
+    mock_browser_context_.reset(new StrictMock<MockBrowserContext>);
+    EXPECT_CALL(*mock_browser_context_.get(), IsOffTheRecord())
+        .WillRepeatedly(Return(false));
+
+    download_manager_ = new DownloadManagerImpl(
+        mock_download_file_manager_.get(),
+        scoped_ptr<content::DownloadItemFactory>(
+            mock_download_item_factory_.get()).Pass(), NULL);
+    download_manager_->SetDelegate(mock_download_manager_delegate_.get());
+    download_manager_->Init(mock_browser_context_.get());
+  }
+
+  virtual void TearDown() {
+    while (content::MockDownloadItem*
+           item = mock_download_item_factory_->PopItem()) {
+      EXPECT_CALL(*item, GetSafetyState())
+          .WillOnce(Return(content::DownloadItem::SAFE));
+      EXPECT_CALL(*item, IsPartialDownload())
+          .WillOnce(Return(false));
     }
 
-    return function_error;
+    download_manager_->Shutdown();
+    download_manager_ = NULL;
+    message_loop_.RunAllPending();
+    ASSERT_EQ(NULL, mock_download_item_factory_.get());
+    message_loop_.RunAllPending();
+    mock_download_manager_delegate_.reset();
+    mock_download_file_manager_ = NULL;
+    mock_browser_context_.reset();
   }
 
-  net::Error forced_error_;
-};
-
-DownloadFileWithErrors::DownloadFileWithErrors(
-    DownloadCreateInfo* info,
-    scoped_ptr<content::ByteStreamReader> stream,
-    DownloadManager* manager,
-    bool calculate_hash)
-    : DownloadFileImpl(info,
-                       stream.Pass(),
-                       new DownloadRequestHandle(),
-                       manager,
-                       calculate_hash,
-                       scoped_ptr<content::PowerSaveBlocker>(NULL).Pass(),
-                       net::BoundNetLog()),
-      forced_error_(net::OK) {
-}
+  // Returns download id.
+  content::MockDownloadItem& AddItemToManager() {
+    DownloadCreateInfo info;
+    DownloadRequestHandle handle;
 
-net::Error DownloadFileWithErrors::Initialize() {
-  return ReturnError(DownloadFileImpl::Initialize());
-}
+    static const char* kDownloadIdDomain = "Test download id domain";
 
-net::Error DownloadFileWithErrors::AppendDataToFile(const char* data,
-                                                    size_t data_len) {
-  return ReturnError(DownloadFileImpl::AppendDataToFile(data, data_len));
-}
+    // Args are ignored except for download id, so everything else can be
+    // null.
+    int id = next_download_id_;
+    ++next_download_id_;
+    info.download_id = content::DownloadId(kDownloadIdDomain, id);
+    download_manager_->CreateDownloadItem(&info, handle);
 
-net::Error DownloadFileWithErrors::Rename(const FilePath& full_path) {
-  return ReturnError(DownloadFileImpl::Rename(full_path));
-}
+    DCHECK(mock_download_item_factory_->GetItem(id));
+    content::MockDownloadItem& item(*mock_download_item_factory_->GetItem(id));
+    ON_CALL(item, GetId())
+        .WillByDefault(Return(id));
 
-namespace {
+    return item;
+  }
 
-const struct {
-  const char* url;
-  const char* mime_type;
-  bool save_as;
-  bool prompt_for_download;
-  bool expected_save_as;
-} kStartDownloadCases[] = {
-  { "http://www.foo.com/dont-open.html",
-    "text/html",
-    false,
-    false,
-    false, },
-  { "http://www.foo.com/save-as.html",
-    "text/html",
-    true,
-    false,
-    true, },
-  { "http://www.foo.com/always-prompt.html",
-    "text/html",
-    false,
-    true,
-    true, },
-  { "http://www.foo.com/user-script-text-html-mimetype.user.js",
-    "text/html",
-    false,
-    false,
-    false, },
-  { "http://www.foo.com/extensionless-extension",
-    "application/x-chrome-extension",
-    true,
-    false,
-    true, },
-  { "http://www.foo.com/save-as.pdf",
-    "application/pdf",
-    true,
-    false,
-    true, },
-  { "http://www.foo.com/sometimes_prompt.pdf",
-    "application/pdf",
-    false,
-    true,
-    false, },
-  { "http://www.foo.com/always_prompt.jar",
-    "application/jar",
-    false,
-    true,
-    true, },
-};
+  content::MockDownloadItem& GetMockDownloadItem(int id) {
+    content::MockDownloadItem* itemp = mock_download_item_factory_->GetItem(id);
 
-// This is an observer that records what download IDs have opened a select
-// file dialog.
-class SelectFileObserver : public DownloadManager::Observer {
- public:
-  explicit SelectFileObserver(DownloadManager* download_manager)
-      : download_manager_(download_manager) {
-    DCHECK(download_manager_.get());
-    download_manager_->AddObserver(this);
+    DCHECK(itemp);
+    return *itemp;
   }
 
-  ~SelectFileObserver() {
-    download_manager_->RemoveObserver(this);
+  void RemoveMockDownloadItem(int id) {
+    // Owned by DownloadManager; should be deleted there.
+    mock_download_item_factory_->RemoveItem(id);
   }
 
-  // Downloadmanager::Observer functions.
-  virtual void ModelChanged(DownloadManager* manager) OVERRIDE {}
-  virtual void ManagerGoingDown(DownloadManager* manager) OVERRIDE {}
-  virtual void SelectFileDialogDisplayed(
-      DownloadManager* manager, int32 id) OVERRIDE {
-    file_dialog_ids_.insert(id);
+  MockDownloadManagerDelegate& GetMockDownloadManagerDelegate() {
+    return *mock_download_manager_delegate_;
   }
 
-  bool ShowedFileDialogForId(int32 id) {
-    return file_dialog_ids_.find(id) != file_dialog_ids_.end();
+  MockDownloadFileManager& GetMockDownloadFileManager() {
+    return *mock_download_file_manager_;
   }
 
- private:
-  std::set<int32> file_dialog_ids_;
-  scoped_refptr<DownloadManager> download_manager_;
-};
-
-// This observer tracks the progress of |DownloadItem|s.
-class ItemObserver : public DownloadItem::Observer {
- public:
-  explicit ItemObserver(DownloadItem* tracked)
-      : tracked_(tracked), states_hit_(0),
-        was_updated_(false), was_opened_(false) {
-    DCHECK(tracked_);
-    tracked_->AddObserver(this);
-    // Record the initial state.
-    OnDownloadUpdated(tracked_);
-  }
-  ~ItemObserver() {
-    tracked_->RemoveObserver(this);
+  // Probe at private internals.
+  DownloadItem* GetActiveDownloadItem(int32 id) {
+    return download_manager_->GetActiveDownload(id);
   }
 
-  bool hit_state(int state) const {
-    return (1 << state) & states_hit_;
-  }
-  bool was_updated() const { return was_updated_; }
-  bool was_opened() const { return was_opened_; }
+  void AddItemToHistory(content::MockDownloadItem& item, int64 db_handle) {
+    // For DCHECK in AddDownloadItemToHistory.  Don't want to use
+    // WillRepeatedly as it may have to return true after this.
+    if (DCHECK_IS_ON())
+      EXPECT_CALL(item, IsPersisted())
+          .WillOnce(Return(false));
 
- private:
-  // DownloadItem::Observer methods
-  virtual void OnDownloadUpdated(DownloadItem* download) OVERRIDE {
-    DCHECK_EQ(tracked_, download);
-    states_hit_ |= (1 << download->GetState());
-    was_updated_ = true;
-  }
-  virtual void OnDownloadOpened(DownloadItem* download) OVERRIDE{
-    DCHECK_EQ(tracked_, download);
-    states_hit_ |= (1 << download->GetState());
-    was_opened_ = true;
-  }
+    EXPECT_CALL(item, SetDbHandle(db_handle));
+    EXPECT_CALL(item, SetIsPersisted());
+    EXPECT_CALL(item, GetDbHandle())
+        .WillRepeatedly(Return(db_handle));
 
-  DownloadItem* tracked_;
-  int states_hit_;
-  bool was_updated_;
-  bool was_opened_;
-};
+    // Null out ShowDownloadInBrowser
+    EXPECT_CALL(item, GetWebContents())
+        .WillOnce(Return(static_cast<WebContents*>(NULL)));
+    EXPECT_CALL(GetMockDownloadManagerDelegate(),
+                GetAlternativeWebContentsToNotifyForDownload())
+        .WillOnce(Return(static_cast<WebContents*>(NULL)));
 
-}  // namespace
-
-TEST_F(DownloadManagerTest, MAYBE_StartDownload) {
-  content::TestBrowserThread io_thread(BrowserThread::IO, &message_loop_);
-  ASSERT_TRUE(CreateTempDownloadsDirectory());
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kStartDownloadCases); ++i) {
-    DVLOG(1) << "Test case " << i;
-    download_manager_delegate_->set_prompt_user_for_save_location(
-        kStartDownloadCases[i].prompt_for_download);
-
-    SelectFileObserver observer(download_manager_);
-    // 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->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());
-
-    scoped_ptr<content::ByteStreamWriter> stream_writer;
-    scoped_ptr<content::ByteStreamReader> stream_reader;
-    content::CreateByteStream(message_loop_.message_loop_proxy(),
-                              message_loop_.message_loop_proxy(),
-                              kTestDataLen, &stream_writer, &stream_reader);
-
-    DownloadFile* download_file(
-        new DownloadFileImpl(info.get(),
-                             stream_reader.Pass(),
-                             new DownloadRequestHandle(),
-                             download_manager_, false,
-                             scoped_ptr<content::PowerSaveBlocker>(NULL).Pass(),
-                             net::BoundNetLog()));
-    AddDownloadToFileManager(info->download_id, download_file);
-    download_file->Initialize();
-    download_manager_->StartDownload(info->download_id.local());
-    message_loop_.RunAllPending();
+    EXPECT_CALL(item, IsInProgress())
+        .WillOnce(Return(true));
 
-    // 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(info->download_id.local()));
+    // Null out MaybeCompleteDownload
+    EXPECT_CALL(item, AllDataSaved())
+        .WillOnce(Return(false));
 
-    file_manager()->CancelDownload(info->download_id);
+    download_manager_->OnItemAddedToPersistentStore(item.GetId(), db_handle);
   }
-}
-
-namespace {
 
-enum OverwriteDownloadPath {
-  DONT_OVERWRITE,
-  OVERWRITE
-};
+ protected:
+  // Key test variable; we'll keep it available to sub-classes.
+  scoped_refptr<DownloadManagerImpl> download_manager_;
 
-enum ResponseCompletionTime {
-  COMPLETES_BEFORE_RENAME,
-  COMPLETES_AFTER_RENAME
-};
+ private:
+  MessageLoopForUI message_loop_;
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread file_thread_;
+  base::WeakPtr<MockDownloadItemFactory> mock_download_item_factory_;
+  scoped_ptr<MockDownloadManagerDelegate> mock_download_manager_delegate_;
+  scoped_refptr<MockDownloadFileManager> mock_download_file_manager_;
+  scoped_ptr<MockBrowserContext> mock_browser_context_;
+  int next_download_id_;
 
-// Test cases to be used with DownloadFilenameTest.  The paths are relative to
-// the temporary downloads directory used for testing.
-const struct DownloadFilenameTestCase {
-  // DownloadItem properties prior to calling RestartDownload().
-  const FilePath::CharType*       target_path;
-  DownloadItem::TargetDisposition target_disposition;
-
-  // If we receive a ChooseDownloadPath() call to prompt the user for a download
-  // location, |expected_prompt_path| is the expected prompt path. The
-  // TestDownloadManagerDelegate will respond with |chosen_path|. If
-  // |chosen_path| is empty, then the file choose dialog be cancelled.
-  const FilePath::CharType*       expected_prompt_path;
-  const FilePath::CharType*       chosen_path;
-
-  // Response to GetIntermediatePath(). If |intermediate_path| is empty, then a
-  // temporary path is constructed with
-  // GetTempDownloadPath(item->GetTargetFilePath()).
-  const FilePath::CharType*       intermediate_path;
-  OverwriteDownloadPath           overwrite_intermediate_path;
-
-  // Determines when OnResponseCompleted() is called. If this is
-  // COMPLETES_BEFORE_RENAME, then the response completes before the filename is
-  // determines.
-  ResponseCompletionTime          completion;
-
-  // The expected intermediate path for the download.
-  const FilePath::CharType*       expected_intermediate_path;
-
-  // The expected final path for the download.
-  const FilePath::CharType*       expected_final_path;
-} kDownloadFilenameTestCases[] = {
-
-#define TARGET                FILE_PATH_LITERAL
-#define INTERMEDIATE          FILE_PATH_LITERAL
-#define EXPECTED_PROMPT       FILE_PATH_LITERAL
-#define PROMPT_RESPONSE       FILE_PATH_LITERAL
-#define EXPECTED_INTERMEDIATE FILE_PATH_LITERAL
-#define EXPECTED_FINAL        FILE_PATH_LITERAL
-
-  {
-    // 0: No prompting.
-    TARGET("foo.txt"),
-    DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-
-    EXPECTED_PROMPT(""),
-    PROMPT_RESPONSE(""),
-
-    INTERMEDIATE("foo.txt.intermediate"),
-    OVERWRITE,
-
-    COMPLETES_AFTER_RENAME,
-    EXPECTED_INTERMEDIATE("foo.txt.intermediate"),
-    EXPECTED_FINAL("foo.txt"),
-  },
-  {
-    // 1: With prompting. No rename.
-    TARGET("foo.txt"),
-    DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-
-    EXPECTED_PROMPT("foo.txt"),
-    PROMPT_RESPONSE("foo.txt"),
-
-    INTERMEDIATE("foo.txt.intermediate"),
-    OVERWRITE,
-
-    COMPLETES_AFTER_RENAME,
-    EXPECTED_INTERMEDIATE("foo.txt.intermediate"),
-    EXPECTED_FINAL("foo.txt"),
-  },
-  {
-    // 2: With prompting. Download is renamed.
-    TARGET("foo.txt"),
-    DownloadItem::TARGET_DISPOSITION_PROMPT,
-
-    EXPECTED_PROMPT("foo.txt"),
-    PROMPT_RESPONSE("bar.txt"),
-
-    INTERMEDIATE("bar.txt.intermediate"),
-    OVERWRITE,
-
-    COMPLETES_AFTER_RENAME,
-    EXPECTED_INTERMEDIATE("bar.txt.intermediate"),
-    EXPECTED_FINAL("bar.txt"),
-  },
-  {
-    // 3: With prompting. Download path is changed.
-    TARGET("foo.txt"),
-    DownloadItem::TARGET_DISPOSITION_PROMPT,
-
-    EXPECTED_PROMPT("foo.txt"),
-    PROMPT_RESPONSE("Foo/bar.txt"),
-
-    INTERMEDIATE("Foo/bar.txt.intermediate"),
-    OVERWRITE,
-
-    COMPLETES_AFTER_RENAME,
-    EXPECTED_INTERMEDIATE("Foo/bar.txt.intermediate"),
-    EXPECTED_FINAL("Foo/bar.txt"),
-  },
-  {
-    // 4: No prompting. Intermediate path exists. Doesn't overwrite
-    // intermediate path.
-    TARGET("exists.png"),
-    DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-
-    EXPECTED_PROMPT(""),
-    PROMPT_RESPONSE(""),
-
-    INTERMEDIATE("exists.png.temp"),
-    DONT_OVERWRITE,
-
-    COMPLETES_AFTER_RENAME,
-    EXPECTED_INTERMEDIATE("exists.png (1).temp"),
-    EXPECTED_FINAL("exists.png"),
-  },
-  {
-    // 5: No prompting. Intermediate path exists. Overwrites.
-    TARGET("exists.png"),
-    DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-
-    EXPECTED_PROMPT(""),
-    PROMPT_RESPONSE(""),
-
-    INTERMEDIATE("exists.png.temp"),
-    OVERWRITE,
-
-    COMPLETES_AFTER_RENAME,
-    EXPECTED_INTERMEDIATE("exists.png.temp"),
-    EXPECTED_FINAL("exists.png"),
-  },
-  {
-    // 6: No prompting. Final path exists. Doesn't overwrite.
-    TARGET("exists.txt"),
-    DownloadItem::TARGET_DISPOSITION_UNIQUIFY,
-
-    EXPECTED_PROMPT(""),
-    PROMPT_RESPONSE(""),
-
-    INTERMEDIATE("exists.txt.temp"),
-    OVERWRITE,
-
-    COMPLETES_AFTER_RENAME,
-    EXPECTED_INTERMEDIATE("exists.txt.temp"),
-    EXPECTED_FINAL("exists (1).txt"),
-  },
-  {
-    // 7: No prompting. Final path exists. Overwrites.
-    TARGET("exists.txt"),
-    DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-
-    EXPECTED_PROMPT(""),
-    PROMPT_RESPONSE(""),
-
-    INTERMEDIATE("exists.txt.temp"),
-    OVERWRITE,
-
-    COMPLETES_AFTER_RENAME,
-    EXPECTED_INTERMEDIATE("exists.txt.temp"),
-    EXPECTED_FINAL("exists.txt"),
-  },
-  {
-    // 8: No prompting. Response completes before filename determination.
-    TARGET("foo.txt"),
-    DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-
-    EXPECTED_PROMPT(""),
-    PROMPT_RESPONSE(""),
-
-    INTERMEDIATE("foo.txt.intermediate"),
-    OVERWRITE,
-
-    COMPLETES_BEFORE_RENAME,
-    EXPECTED_INTERMEDIATE("foo.txt.intermediate"),
-    EXPECTED_FINAL("foo.txt"),
-  },
-  {
-    // 9: With prompting. Download path is changed. Response completes before
-    // filename determination.
-    TARGET("foo.txt"),
-    DownloadItem::TARGET_DISPOSITION_PROMPT,
-
-    EXPECTED_PROMPT("foo.txt"),
-    PROMPT_RESPONSE("Foo/bar.txt"),
-
-    INTERMEDIATE("Foo/bar.txt.intermediate"),
-    OVERWRITE,
-
-    COMPLETES_BEFORE_RENAME,
-    EXPECTED_INTERMEDIATE("Foo/bar.txt.intermediate"),
-    EXPECTED_FINAL("Foo/bar.txt"),
-  },
-
-#undef TARGET
-#undef INTERMEDIATE
-#undef EXPECTED_PROMPT
-#undef PROMPT_RESPONSE
-#undef EXPECTED_INTERMEDIATE
-#undef EXPECTED_FINAL
+  DISALLOW_COPY_AND_ASSIGN(DownloadManagerTest);
 };
 
-} // namespace
+// Does the DownloadManager prompt when requested?
+TEST_F(DownloadManagerTest, RestartDownload) {
+  // Put a mock we have a handle to on the download manager.
+  content::MockDownloadItem& item(AddItemToManager());
+  int download_id = item.GetId();
 
-TEST_F(DownloadManagerTest, DownloadFilenameTest) {
-  ASSERT_TRUE(CreateTempDownloadsDirectory());
-
-  // We create a known file to test file uniquification.
-  ASSERT_TRUE(file_util::WriteFile(
-      GetPathInDownloadsDir(FILE_PATH_LITERAL("exists.txt")), "", 0) == 0);
-  ASSERT_TRUE(file_util::WriteFile(
-      GetPathInDownloadsDir(FILE_PATH_LITERAL("exists.png.temp")), "", 0) == 0);
-
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kDownloadFilenameTestCases); ++i) {
-    const DownloadFilenameTestCase& test_case = kDownloadFilenameTestCases[i];
-    scoped_ptr<DownloadCreateInfo> info(new DownloadCreateInfo);
-
-    SCOPED_TRACE(testing::Message() << "Running test case " << i);
-    info->url_chain.push_back(GURL());
-
-    MockDownloadFile* download_file(
-        new ::testing::StrictMock<MockDownloadFile>());
-    FilePath target_path = GetPathInDownloadsDir(test_case.target_path);
-    FilePath expected_prompt_path =
-        GetPathInDownloadsDir(test_case.expected_prompt_path);
-    FilePath chosen_path = GetPathInDownloadsDir(test_case.chosen_path);
-    FilePath intermediate_path =
-        GetPathInDownloadsDir(test_case.intermediate_path);
-    FilePath expected_intermediate_path =
-        GetPathInDownloadsDir(test_case.expected_intermediate_path);
-    FilePath expected_final_path =
-        GetPathInDownloadsDir(test_case.expected_final_path);
-
-    EXPECT_CALL(*download_file, Rename(expected_intermediate_path))
-        .WillOnce(Return(net::OK));
-    EXPECT_CALL(*download_file, Rename(expected_final_path))
-        .WillOnce(Return(net::OK));
-#if defined(OS_MACOSX)
-    EXPECT_CALL(*download_file, AnnotateWithSourceInformation());
-#endif
-    EXPECT_CALL(*download_file, Detach());
-
-    download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
-    AddMockDownloadToFileManager(info->download_id, download_file);
-    DownloadItem* download = GetActiveDownloadItem(info->download_id);
-    ASSERT_TRUE(download != NULL);
-
-    if (test_case.completion == COMPLETES_BEFORE_RENAME)
-      OnResponseCompleted(info->download_id, 1024, std::string("fake_hash"));
-
-    download->OnTargetPathDetermined(
-        target_path, test_case.target_disposition,
-        content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
-    download_manager_delegate_->SetFileSelectionExpectation(
-        expected_prompt_path, chosen_path);
-    download_manager_delegate_->SetIntermediatePath(
-        intermediate_path, test_case.overwrite_intermediate_path);
-    download_manager_delegate_->SetShouldCompleteDownload(false);
-    download_manager_->RestartDownload(info->download_id.local());
-    message_loop_.RunAllPending();
+  // Confirm we're internally consistent.
+  EXPECT_EQ(&item, GetActiveDownloadItem(download_id));
 
-    if (test_case.completion == COMPLETES_AFTER_RENAME) {
-      OnResponseCompleted(info->download_id, 1024, std::string("fake_hash"));
-      message_loop_.RunAllPending();
-    }
+  ScopedTempDir download_dir;
+  ASSERT_TRUE(download_dir.CreateUniqueTempDir());
+  FilePath expected_path(download_dir.path().Append(
+      FILE_PATH_LITERAL("location")));
 
-    EXPECT_EQ(expected_intermediate_path.value(),
-              download->GetFullPath().value());
-    download_manager_delegate_->SetShouldCompleteDownload(true);
-    download_manager_delegate_->InvokeDownloadCompletionCallback();
-    message_loop_.RunAllPending();
-    EXPECT_EQ(expected_final_path.value(),
-              download->GetTargetFilePath().value());
-  }
-}
+  EXPECT_CALL(item, GetTargetDisposition())
+      .WillOnce(Return(DownloadItem::TARGET_DISPOSITION_PROMPT));
+  EXPECT_CALL(GetMockDownloadManagerDelegate(), ChooseDownloadPath(&item));
+  download_manager_->RestartDownload(download_id);
 
-void WriteCopyToStream(const char *source,
-                     size_t len, content::ByteStreamWriter* stream) {
-  scoped_refptr<net::IOBuffer> copy(new net::IOBuffer(len));
-  memcpy(copy.get()->data(), source, len);
-  stream->Write(copy, len);
+  // The alternative pathway goes straight to OnTargetPathAvailable,
+  // so it more naturally belongs below.
 }
 
-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->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>());
-
-  EXPECT_CALL(*download_file, Rename(cr_path))
-      .WillOnce(Return(net::OK));
-  EXPECT_CALL(*download_file, Cancel());
-
-  download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
-  // |download_file| is owned by DownloadFileManager.
-  AddMockDownloadToFileManager(info->download_id, download_file);
-
-  DownloadItem* download = GetActiveDownloadItem(info->download_id);
-  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(info->download_id) != NULL);
-
-  int64 error_size = 3;
-  OnDownloadInterrupted(info->download_id, error_size, "",
-                        content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
-  message_loop_.RunAllPending();
-
-  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == 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->GetFileExternallyRemoved());
-  EXPECT_EQ(DownloadItem::INTERRUPTED, download->GetState());
-
-  download->Cancel(true);
-
-  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->GetFileExternallyRemoved());
-  EXPECT_EQ(DownloadItem::INTERRUPTED, download->GetState());
-  EXPECT_EQ(download->GetReceivedBytes(), error_size);
-  EXPECT_EQ(download->GetTotalBytes(), static_cast<int64>(kTestDataLen));
+// Do the results of GetIntermediatePath get passed through to the
+// download?  Note that this path is tested from RestartDownload
+// to test the non-prompting path in RestartDownload as well.
+TEST_F(DownloadManagerTest, OnTargetPathAvailable) {
+  // Put a mock we have a handle to on the download manager.
+  content::MockDownloadItem& item(AddItemToManager());
+
+  ScopedTempDir download_dir;
+  ASSERT_TRUE(download_dir.CreateUniqueTempDir());
+  FilePath target_path(download_dir.path().Append(
+      FILE_PATH_LITERAL("location")));
+  FilePath intermediate_path(download_dir.path().Append(
+      FILE_PATH_LITERAL("location.crdownload")));
+
+  EXPECT_CALL(item, GetTargetDisposition())
+      .WillOnce(Return(DownloadItem::TARGET_DISPOSITION_UNIQUIFY));
+  EXPECT_CALL(GetMockDownloadManagerDelegate(),
+              GetIntermediatePath(Ref(item), _))
+      .WillOnce(DoAll(SetArgPointee<1>(true), Return(intermediate_path)));
+  // Finesse DCHECK with WillRepeatedly.
+  EXPECT_CALL(item, GetTargetFilePath())
+      .WillRepeatedly(ReturnRef(target_path));
+  EXPECT_CALL(item, OnIntermediatePathDetermined(
+      &GetMockDownloadFileManager(), intermediate_path, true));
+  download_manager_->RestartDownload(item.GetId());
 }
 
-// Test the behavior of DownloadFileManager and DownloadManager in the event
-// of a file error while writing the download to disk.
-TEST_F(DownloadManagerTest, MAYBE_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::FileStream* stream = new net::FileStream(NULL);
-  ASSERT_EQ(0, stream->OpenSync(
-      path,
-      base::PLATFORM_FILE_OPEN_ALWAYS | base::PLATFORM_FILE_WRITE));
-
-  // Make sure the DFM exists; we'll need it later.
-  // TODO(rdsmith): This is ugly and should be rewritten, but a large
-  // rewrite of this test is in progress in a different CL, so doing it
-  // the hacky way for now.
-  (void) file_manager();
-
-  // 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->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::ByteStreamWriter> stream_input;
-  scoped_ptr<content::ByteStreamReader> stream_output;
-  content::CreateByteStream(message_loop_.message_loop_proxy(),
-                            message_loop_.message_loop_proxy(),
-                            kTestDataLen, &stream_input, &stream_output);
-
-  download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
-
-  // Create a download file that we can insert errors into.
-  scoped_ptr<DownloadFileWithErrors> download_file(new DownloadFileWithErrors(
-      info.get(), stream_output.Pass(), download_manager_, false));
-  download_file->Initialize();
-
-  DownloadItem* download = GetActiveDownloadItem(info->download_id);
-  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.
-  WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());
-
-  // Finalize the file name.
-  ContinueDownloadWithPath(download, path);
-  message_loop_.RunAllPending();
-  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != NULL);
-
-  // Add more data.
-  WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());
-
-  // Add more data, but an error occurs.
-  download_file->set_forced_error(net::ERR_FAILED);
-  WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());
-  message_loop_.RunAllPending();
-
-  // Check the state.  The download should have been interrupted.
-  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == 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->GetFileExternallyRemoved());
-  EXPECT_EQ(DownloadItem::INTERRUPTED, download->GetState());
-
-  // Clean up.
-  download->Cancel(true);
-  message_loop_.RunAllPending();
+// Do the results of an OnDownloadInterrupted get passed through properly
+// to the DownloadItem?  This test tests non-persisted downloads.
+TEST_F(DownloadManagerTest, OnDownloadInterrupted_NonPersisted) {
+  // Put a mock we have a handle to on the download manager.
+  content::MockDownloadItem& item(AddItemToManager());
+  int download_id = item.GetId();
+
+  int64 size = 0xdeadbeef;
+  const std::string hash_state("Undead beef");
+  content::DownloadInterruptReason reason(
+      content::DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
+
+  EXPECT_CALL(item, Interrupted(size, hash_state, reason));
+  EXPECT_CALL(item, OffThreadCancel(&GetMockDownloadFileManager()));
+  EXPECT_CALL(item, IsPersisted())
+      .WillOnce(Return(false));
+  download_manager_->OnDownloadInterrupted(
+      download_id, size, hash_state, reason);
+  EXPECT_EQ(&item, GetActiveDownloadItem(download_id));
 }
 
-TEST_F(DownloadManagerTest, DownloadCancelTest) {
-  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->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>());
-
-  // |download_file| is owned by DownloadFileManager.
-  EXPECT_CALL(*download_file, Rename(cr_path))
-      .WillOnce(Return(net::OK));
-  EXPECT_CALL(*download_file, Cancel());
-
-  download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
-  AddMockDownloadToFileManager(info->download_id, download_file);
-
-  DownloadItem* download = GetActiveDownloadItem(info->download_id);
-  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(info->download_id) != NULL);
-
-  download->Cancel(false);
-  message_loop_.RunAllPending();
-
-  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != 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());
-  EXPECT_FALSE(observer->was_opened());
-  EXPECT_FALSE(download->GetFileExternallyRemoved());
-  EXPECT_EQ(DownloadItem::CANCELLED, download->GetState());
-
-  file_manager()->CancelDownload(info->download_id);
-
-  EXPECT_FALSE(file_util::PathExists(new_path));
-  EXPECT_FALSE(file_util::PathExists(cr_path));
+// Do the results of an OnDownloadInterrupted get passed through properly
+// to the DownloadItem?  This test tests persisted downloads.
+TEST_F(DownloadManagerTest, OnDownloadInterrupted_Persisted) {
+  // Put a mock we have a handle to on the download manager.
+  content::MockDownloadItem& item(AddItemToManager());
+  int download_id = item.GetId();
+  int64 db_handle = 0x7;
+  AddItemToHistory(item, db_handle);
+
+  int64 size = 0xdeadbeef;
+  const std::string hash_state("Undead beef");
+  content::DownloadInterruptReason reason(
+      content::DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
+
+  EXPECT_CALL(item, Interrupted(size, hash_state, reason));
+  EXPECT_CALL(item, OffThreadCancel(&GetMockDownloadFileManager()));
+  EXPECT_CALL(item, IsPersisted())
+      .WillOnce(Return(true));
+  EXPECT_CALL(GetMockDownloadManagerDelegate(),
+              UpdateItemInPersistentStore(&item));
+  download_manager_->OnDownloadInterrupted(
+      download_id, size, hash_state, reason);
+  EXPECT_EQ(NULL, GetActiveDownloadItem(download_id));
+
+  // Remove so we don't get errors on shutdown.
+  EXPECT_CALL(GetMockDownloadManagerDelegate(),
+              RemoveItemFromPersistentStore(&item));
+  download_manager_->DownloadRemoved(&item);
+  RemoveMockDownloadItem(download_id);
 }
 
-TEST_F(DownloadManagerTest, MAYBE_DownloadOverwriteTest) {
-  using ::testing::_;
-  using ::testing::CreateFunctor;
-  using ::testing::Invoke;
-  using ::testing::Return;
-
-  ASSERT_TRUE(CreateTempDownloadsDirectory());
-  // File names we're using.
-  const FilePath new_path(GetPathInDownloadsDir(FILE_PATH_LITERAL("foo.txt")));
-  const FilePath cr_path(GetTempDownloadPath(new_path));
-  EXPECT_FALSE(file_util::PathExists(new_path));
-
-  // Create the file that we will overwrite.  Will be automatically cleaned
-  // up when temp_dir_ is destroyed.
-  FILE* fp = file_util::OpenFile(new_path, "w");
-  file_util::CloseFile(fp);
-  EXPECT_TRUE(file_util::PathExists(new_path));
-
-  // Construct the unique file name that normally would be created, but
-  // which we will override.
-  int uniquifier =
-      file_util::GetUniquePathNumber(new_path, FILE_PATH_LITERAL(""));
-  FilePath unique_new_path = new_path;
-  EXPECT_NE(0, uniquifier);
-  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->prompt_user_for_save_location = true;
-  info->url_chain.push_back(GURL());
-  scoped_ptr<content::ByteStreamWriter> stream_input;
-  scoped_ptr<content::ByteStreamReader> stream_output;
-  content::CreateByteStream(message_loop_.message_loop_proxy(),
-                            message_loop_.message_loop_proxy(),
-                            kTestDataLen, &stream_input, &stream_output);
-
-  download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
-
-  DownloadItem* download = GetActiveDownloadItem(info->download_id);
-  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(), stream_output.Pass(),
-                           new DownloadRequestHandle(),
-                           download_manager_, false,
-                           scoped_ptr<content::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, download_file);
-
-  ContinueDownloadWithPath(download, new_path);
-  message_loop_.RunAllPending();
-  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != NULL);
-
-  WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());
-
-  // Finish the download.
-  OnResponseCompleted(info->download_id, kTestDataLen, "");
-  message_loop_.RunAllPending();
-
-  // Download is complete.
-  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == NULL);
-  EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
-  EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
-  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_FALSE(download->GetFileExternallyRemoved());
-  EXPECT_EQ(DownloadItem::COMPLETE, download->GetState());
-
-  EXPECT_TRUE(file_util::PathExists(new_path));
-  EXPECT_FALSE(file_util::PathExists(cr_path));
-  EXPECT_FALSE(file_util::PathExists(unique_new_path));
-  std::string file_contents;
-  EXPECT_TRUE(file_util::ReadFileToString(new_path, &file_contents));
-  EXPECT_EQ(std::string(kTestData), file_contents);
+// Does DownloadCancelled remove Download from appropriate queues?
+// This test tests non-persisted downloads.
+TEST_F(DownloadManagerTest, OnDownloadCancelled_NonPersisted) {
+  // Put a mock we have a handle to on the download manager.
+  content::MockDownloadItem& item(AddItemToManager());
+
+  EXPECT_CALL(item, IsPersisted())
+      .WillRepeatedly(Return(false));
+  EXPECT_CALL(item, GetState())
+      .WillRepeatedly(Return(DownloadItem::CANCELLED));
+  EXPECT_CALL(item, GetDbHandle())
+      .WillRepeatedly(Return(DownloadItem::kUninitializedHandle));
+
+  EXPECT_CALL(item, OffThreadCancel(&GetMockDownloadFileManager()));
+  download_manager_->DownloadCancelled(&item);
+  // TODO(rdsmith): Confirm that the download item is no longer on the
+  // active list by calling GetActiveDownloadItem(id).  Currently, the
+  // item is left on the active list for rendez-vous with the history
+  // system :-{.
 }
 
-TEST_F(DownloadManagerTest, MAYBE_DownloadRemoveTest) {
-  using ::testing::_;
-  using ::testing::CreateFunctor;
-  using ::testing::Invoke;
-  using ::testing::Return;
-
-  ASSERT_TRUE(CreateTempDownloadsDirectory());
-
-  // File names we're using.
-  const FilePath new_path(GetPathInDownloadsDir(FILE_PATH_LITERAL("foo.txt")));
-  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->prompt_user_for_save_location = true;
-  info->url_chain.push_back(GURL());
-  scoped_ptr<content::ByteStreamWriter> stream_input;
-  scoped_ptr<content::ByteStreamReader> stream_output;
-  content::CreateByteStream(message_loop_.message_loop_proxy(),
-                            message_loop_.message_loop_proxy(),
-                            kTestDataLen, &stream_input, &stream_output);
-
-  download_manager_->CreateDownloadItem(info.get(), DownloadRequestHandle());
-
-  DownloadItem* download = GetActiveDownloadItem(info->download_id);
-  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(), stream_output.Pass(),
-                           new DownloadRequestHandle(),
-                           download_manager_, false,
-                           scoped_ptr<content::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, download_file);
-
-  ContinueDownloadWithPath(download, new_path);
-  message_loop_.RunAllPending();
-  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) != NULL);
-
-  WriteCopyToStream(kTestData, kTestDataLen, stream_input.get());
-
-  // Finish the download.
-  OnResponseCompleted(info->download_id, kTestDataLen, "");
-  message_loop_.RunAllPending();
-
-  // Download is complete.
-  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == NULL);
-  EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
-  EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
-  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_FALSE(download->GetFileExternallyRemoved());
-  EXPECT_EQ(DownloadItem::COMPLETE, download->GetState());
-
-  EXPECT_TRUE(file_util::PathExists(new_path));
-  EXPECT_FALSE(file_util::PathExists(cr_path));
-
-  // Remove the downloaded file.
-  ASSERT_TRUE(file_util::Delete(new_path, false));
-  download->OnDownloadedFileRemoved();
-  message_loop_.RunAllPending();
-
-  EXPECT_TRUE(GetActiveDownloadItem(info->download_id) == NULL);
-  EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
-  EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
-  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));
+// Does DownloadCancelled remove Download from appropriate queues?
+// This test tests persisted downloads.
+TEST_F(DownloadManagerTest, OnDownloadCancelled_Persisted) {
+  // Put a mock we have a handle to on the download manager.
+  content::MockDownloadItem& item(AddItemToManager());
+  int download_id = item.GetId();
+  int64 db_handle = 0x7;
+  AddItemToHistory(item, db_handle);
+
+  EXPECT_CALL(item, IsPersisted())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(GetMockDownloadManagerDelegate(),
+              UpdateItemInPersistentStore(&item));
+  EXPECT_CALL(item, GetState())
+      .WillRepeatedly(Return(DownloadItem::CANCELLED));
+  EXPECT_CALL(item, GetDbHandle())
+      .WillRepeatedly(Return(db_handle));
+
+  EXPECT_CALL(item, OffThreadCancel(&GetMockDownloadFileManager()));
+  download_manager_->DownloadCancelled(&item);
+  EXPECT_EQ(NULL, GetActiveDownloadItem(download_id));
 }
diff --git a/content/content_browser.gypi b/content/content_browser.gypi
index 2c6bf36..4637cdb 100644
--- a/content/content_browser.gypi
+++ b/content/content_browser.gypi
@@ -276,6 +276,7 @@
     'browser/download/download_file_impl.h',
     'browser/download/download_file_manager.cc',
     'browser/download/download_file_manager.h',
+    'browser/download/download_item_factory.h',
     'browser/download/download_item_impl.cc',
     'browser/download/download_item_impl.h',
     'browser/download/download_manager_impl.cc',