Rewrite download manager unit to be actual unit tests.

content/browser/download/download_manager_impl_unittest.cc

Index: content/browser/download/download_manager_impl_unittest.cc
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc
index fe6f4a2b29c7158611e2cad1f3af6c36ecfe4a23..f39bcefafb1842c3065f1347da0623f0e1b6b9ac 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,1193 +52,531 @@ 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_METHOD3(ChooseDownloadPath, void(WebContents*, const FilePath&, int32));
+  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_METHOD3(GetSaveDir, void(WebContents*, FilePath*, FilePath*));
+  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;
-}
-
-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;
-  }
+MockDownloadManagerDelegate::~MockDownloadManagerDelegate() { }
 
-  void set_prompt_user_for_save_location(bool value) {
-    prompt_user_for_save_location_ = value;
-  }
-
-  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;
-  }
+class MockDownloadFileManager : public DownloadFileManager {
+ public:
+  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_;
-    }
-  }
+  MOCK_METHOD0(Shutdown, void());
+  MOCK_METHOD3(MockStartDownload,
+               content::DownloadId(DownloadCreateInfo*,
+                                   content::ByteStreamReader* stream,
+                                   const DownloadRequestHandle&));
 
-  virtual void ChooseDownloadPath(WebContents* web_contents,
-                                  const FilePath& suggested_path,
-                                  int32 download_id) OVERRIDE {
-    if (!expected_suggested_path_.empty()) {
-      EXPECT_STREQ(expected_suggested_path_.value().c_str(),
-                   suggested_path.value().c_str());
-    }
-    if (file_selection_response_.empty()) {
-      BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE,
-          base::Bind(&DownloadManager::FileSelectionCanceled,
-                     download_manager_,
-                     download_id));
-    } else {
-      BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE,
-          base::Bind(&DownloadManager::FileSelected,
-                     download_manager_,
-                     file_selection_response_,
-                     download_id));
-    }
-    expected_suggested_path_.clear();
-    file_selection_response_.clear();
-  }
+  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 bool ShouldOpenFileBasedOnExtension(const FilePath& path) OVERRIDE {
-    return path.Extension() == FilePath::StringType(FILE_PATH_LITERAL(".pdf"));
-  }
+MockDownloadFileManager::MockDownloadFileManager()
+    : DownloadFileManager(NULL) { }
 
-  virtual void AddItemToPersistentStore(DownloadItem* item) OVERRIDE {
-    static int64 db_handle = DownloadItem::kUninitializedHandle;
-    download_manager_->OnItemAddedToPersistentStore(item->GetId(), --db_handle);
-  }
+MockDownloadFileManager::~MockDownloadFileManager() { }
 
-  void SetFileSelectionExpectation(const FilePath& suggested_path,
-                                   const FilePath& response) {
-    expected_suggested_path_ = suggested_path;
-    file_selection_response_ = response;
-  }
+class MockDownloadItemFactory
+    : public content::DownloadItemFactory,
+      public base::SupportsWeakPtr<MockDownloadItemFactory> {
+ public:
+  explicit MockDownloadItemFactory();

[asanka, 2012/06/08 22:14:18]

Minor nit: no need for 'explicit' for 0 argument constructor.

[Randy Smith (Not in Mondays), 2012/06/09 17:32:39]

Done.

+  virtual ~MockDownloadItemFactory();
 
-  void SetMarkContentsDangerous(bool dangerous) {
-    mark_content_dangerous_ = dangerous;
-  }
+  // Access to map of created items.
+  // TODO(rdsmith): Could add type (save page, persisted, etc.)
+  // functionality if it's ever needed by consumers.
 
-  void SetIntermediatePath(const FilePath& intermediate_path,
-                           bool overwrite_intermediate_path) {
-    intermediate_path_ = intermediate_path;
-    overwrite_intermediate_path_ = overwrite_intermediate_path;
-  }
+  // Returns NULL if no item of that id is present.
+  content::MockDownloadItem* GetItem(int id);
 
-  void SetShouldCompleteDownload(bool value) {
-    should_complete_download_ = value;
-  }
+  // Returns a vector of currently active ids.
+  std::vector<int> Ids() const;
 
-  void InvokeDownloadCompletionCallback() {
-    EXPECT_FALSE(download_completion_callback_.is_null());
-    download_completion_callback_.Run();
-    download_completion_callback_.Reset();
-  }
+  // Should be called when the item of this id is removed so that
+  // we don't keep dangling pointers.
+  void RemoveItem(int id);
 
-  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();
-  }
+std::vector<int> MockDownloadItemFactory::Ids() const {
+  std::vector<int> result;
+  for (std::map<int32, content::MockDownloadItem*>::const_iterator it =
+           items_.begin(); it != items_.end(); it++)
+    result.push_back(it->first);

[Randy Smith (Not in Mondays), 2012/06/07 23:27:21]

I'm sure there's an STL-fancy way to do this, but my STL documentation and I are
separated at the moment, I'm uncertain as to the overall architecture of this
interface, and it would save me max two lines.  Let me know if you think it's
worth the effort :-}.

+  return result;
+}
 
-  // 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;
-  }
+void MockDownloadItemFactory::RemoveItem(int id) {
+  DCHECK(items_.find(id) != items_.end());
+  items_.erase(id);
+}
 
-  void AddDownloadToFileManager(DownloadId id, DownloadFile* download_file) {
-    file_manager()->downloads_[id] =
-      download_file;
-  }
+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 AddMockDownloadToFileManager(DownloadId id,
-                                    MockDownloadFile* download_file) {
-    AddDownloadToFileManager(id, download_file);
-    EXPECT_CALL(*download_file, GetDownloadManager())
-        .WillRepeatedly(Return(download_manager_));
-  }
+  content::MockDownloadItem* result =
+      new StrictMock<content::MockDownloadItem>;
+  EXPECT_CALL(*result, GetId())
+      .WillRepeatedly(Return(local_id));
+  items_[local_id] = result;
 
-  void OnResponseCompleted(DownloadId download_id, int64 size,
-                           const std::string& hash) {
-    download_manager_->OnResponseCompleted(download_id.local(), size, hash);
-  }
+  return 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);
-  }
+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());
 
-  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::MockDownloadItem* result =
+      new StrictMock<content::MockDownloadItem>;
+  EXPECT_CALL(*result, GetId())
+      .WillRepeatedly(Return(local_id));
+  items_[local_id] = result;
 
-  // Get the download item with ID |id|.
-  DownloadItem* GetActiveDownloadItem(DownloadId id) {
-    return download_manager_->GetActiveDownload(id.local());
-  }
+  return 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();
-  }
+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());
 
- 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::MockDownloadItem* result =
+      new StrictMock<content::MockDownloadItem>;
+  EXPECT_CALL(*result, GetId())
+      .WillRepeatedly(Return(local_id));
+  items_[local_id] = result;
 
-  DownloadFileManager* file_manager() {
-    if (!file_manager_) {
-      file_manager_ = new DownloadFileManager(new MockDownloadFileFactory);
-      download_manager_->SetFileManagerForTesting(file_manager_);
-    }
-    return file_manager_;
-  }
+  return result;
+}
 
-  DISALLOW_COPY_AND_ASSIGN(DownloadManagerTest);
+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() {
+    std::vector<int> active_ids(mock_download_item_factory_->Ids());

[asanka, 2012/06/08 22:14:18]

It seems something like mock_download_item_factory_->PopItem() that removes an
item and returns it would make this simpler.

[Randy Smith (Not in Mondays), 2012/06/09 17:32:39]

Done.

+    for (std::vector<int>::iterator it = active_ids.begin();
+         it != active_ids.end(); ++it) {
+      content::MockDownloadItem& item(
+          *mock_download_item_factory_->GetItem(*it));
+      EXPECT_CALL(item, GetSafetyState())
+          .WillOnce(Return(content::DownloadItem::SAFE));
+      EXPECT_CALL(item, IsPartialDownload())
+          .WillOnce(Return(false));
+      mock_download_item_factory_->RemoveItem(*it);
     }
 
-    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<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<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
-
-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();
-
-    if (test_case.completion == COMPLETES_AFTER_RENAME) {
-      OnResponseCompleted(info->download_id, 1024, std::string("fake_hash"));
-      message_loop_.RunAllPending();
-    }
-
-    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());
-  }
-}
-
-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);
+// 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();
+
+  // Confirm we're internally consistent.
+  EXPECT_EQ(&item, GetActiveDownloadItem(download_id));
+
+  ScopedTempDir download_dir;
+  ASSERT_TRUE(download_dir.CreateUniqueTempDir());
+  FilePath expected_path(download_dir.path().Append(
+      FILE_PATH_LITERAL("location")));
+
+  EXPECT_CALL(item, GetTargetDisposition())
+      .WillOnce(Return(DownloadItem::TARGET_DISPOSITION_PROMPT));
+  EXPECT_CALL(item, GetWebContents())
+      .WillOnce(Return(reinterpret_cast<WebContents*>(0x44)));
+  EXPECT_CALL(item, GetTargetFilePath())
+      .WillOnce(ReturnRef(expected_path));
+  EXPECT_CALL(GetMockDownloadManagerDelegate(), ChooseDownloadPath(
+      reinterpret_cast<WebContents*>(0x44), expected_path, download_id));
+  download_manager_->RestartDownload(download_id);
+
+  // 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<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<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));
 }