Change DownloadManagerImpl to operate on DownloadItemImpl

(rather than DownloadItem).

Also make SavePackage aware of DownloadItemImpl and DownloadManagerImpl. 
This allows some cleanup of the DownloadItem interface to make it closer
to the information that should be exported from content, as opposed to 
having a lot of interfaces that are just about the internals of the
progression of the download.

BUG=136639
R=benjhayden@chromium.org


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

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

11 files changed, 521 insertions, 317 deletions

diff --git a/content/browser/download/download_item_factory.h b/content/browser/download/download_item_factory.h
index 9dbbf2d..fc47bea 100644
--- a/content/browser/download/download_item_factory.h
+++ b/content/browser/download/download_item_factory.h
@@ -12,15 +12,17 @@
 #include <string>
 
 #include "base/memory/scoped_ptr.h"
-#include "content/browser/download/download_item_impl.h"
+#include "content/public/browser/download_id.h"
 
 struct DownloadCreateInfo;
+
+class DownloadItemImpl;
+class DownloadItemImplDelegate;
 class DownloadRequestHandleInterface;
 class FilePath;
 class GURL;
 
 namespace content {
-class DownloadId;
 class DownloadItem;
 struct DownloadPersistentStoreInfo;
 }
@@ -35,21 +37,21 @@ class DownloadItemFactory {
 public:
   virtual ~DownloadItemFactory() {}
 
-  virtual content::DownloadItem* CreatePersistedItem(
-      DownloadItemImpl::Delegate* delegate,
+  virtual DownloadItemImpl* CreatePersistedItem(
+      DownloadItemImplDelegate* delegate,
       content::DownloadId download_id,
       const content::DownloadPersistentStoreInfo& info,
       const net::BoundNetLog& bound_net_log) = 0;
 
-  virtual content::DownloadItem* CreateActiveItem(
-      DownloadItemImpl::Delegate* delegate,
+  virtual DownloadItemImpl* CreateActiveItem(
+      DownloadItemImplDelegate* delegate,
       const DownloadCreateInfo& info,
       scoped_ptr<DownloadRequestHandleInterface> request_handle,
       bool is_otr,
       const net::BoundNetLog& bound_net_log) = 0;
 
-  virtual content::DownloadItem* CreateSavePageItem(
-      DownloadItemImpl::Delegate* delegate,
+  virtual DownloadItemImpl* CreateSavePageItem(
+      DownloadItemImplDelegate* delegate,
       const FilePath& path,
       const GURL& url,
       bool is_otr,
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index 3c9521d..6716123 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -21,6 +21,7 @@
 #include "content/browser/download/download_file.h"
 #include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_interrupt_reasons_impl.h"
+#include "content/browser/download/download_item_impl_delegate.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/download_stats.h"
 #include "content/browser/web_contents/web_contents_impl.h"
@@ -135,31 +136,13 @@ const char DownloadItem::kEmptyFileHash[] = "";
 
 }
 
-// Infrastructure in DownloadItemImpl::Delegate to assert invariant that
-// delegate always outlives all attached DownloadItemImpls.
-DownloadItemImpl::Delegate::Delegate()
-    : count_(0) {}
-
-DownloadItemImpl::Delegate::~Delegate() {
-  DCHECK_EQ(0, count_);
-}
-
-void DownloadItemImpl::Delegate::Attach() {
-  ++count_;
-}
-
-void DownloadItemImpl::Delegate::Detach() {
-  DCHECK_LT(0, count_);
-  --count_;
-}
-
 // Our download table ID starts at 1, so we use 0 to represent a download that
 // has started, but has not yet had its data persisted in the table. We use fake
 // database handles in incognito mode starting at -1 and progressively getting
 // more negative.
 
 // Constructor for reading from the history service.
-DownloadItemImpl::DownloadItemImpl(Delegate* delegate,
+DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
                                    DownloadId download_id,
                                    const DownloadPersistentStoreInfo& info,
                                    const net::BoundNetLog& bound_net_log)
@@ -207,7 +190,7 @@ DownloadItemImpl::DownloadItemImpl(Delegate* delegate,
 
 // Constructing for a regular download:
 DownloadItemImpl::DownloadItemImpl(
-    Delegate* delegate,
+    DownloadItemImplDelegate* delegate,
     const DownloadCreateInfo& info,
     scoped_ptr<DownloadRequestHandleInterface> request_handle,
     bool is_otr,
@@ -267,7 +250,7 @@ DownloadItemImpl::DownloadItemImpl(
 }
 
 // Constructing for the "Save Page As..." feature:
-DownloadItemImpl::DownloadItemImpl(Delegate* delegate,
+DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate,
                                    const FilePath& path,
                                    const GURL& url,
                                    bool is_otr,
diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h
index af0b505..80cdc1b 100644
--- a/content/browser/download/download_item_impl.h
+++ b/content/browser/download/download_item_impl.h
@@ -23,78 +23,25 @@
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 
+class DownloadItemImplDelegate;
+
 // See download_item.h for usage.
 class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
  public:
-  // Delegate is defined in DownloadItemImpl (rather than DownloadItem)
-  // as it's relevant to the class implementation (class methods need to
-  // call into it) and doesn't have anything to do with its interface.
-  // Despite this, the delegate methods take DownloadItems as arguments
-  // (rather than DownloadItemImpls) so that classes that inherit from it
-  // can be used with DownloadItem mocks rather than being tied to
-  // DownloadItemImpls.
-  class CONTENT_EXPORT Delegate {
-   public:
-    Delegate();
-
-    // Used for catching use-after-free errors.
-    void Attach();
-    void Detach();
-
-    // Tests if a file type should be opened automatically.
-    virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path) = 0;
-
-    // Allows the delegate to override the opening of a download. If it returns
-    // true then it's reponsible for opening the item.
-    virtual bool ShouldOpenDownload(DownloadItem* download) = 0;
-
-    // Checks whether a downloaded file still exists and updates the
-    // file's state if the file is already removed.
-    // The check may or may not result in a later asynchronous call
-    // to OnDownloadedFileRemoved().
-    virtual void CheckForFileRemoval(DownloadItem* download_item) = 0;
-
-    // If all pre-requisites have been met, complete download processing.
-    // TODO(rdsmith): Move into DownloadItem.
-    virtual void MaybeCompleteDownload(DownloadItem* download) = 0;
-
-    // For contextual issues like language and prefs.
-    virtual content::BrowserContext* GetBrowserContext() const = 0;
-
-    // Handle any delegate portions of a state change operation on the
-    // DownloadItem.
-    virtual void DownloadStopped(DownloadItem* download) = 0;
-    virtual void DownloadCompleted(DownloadItem* download) = 0;
-    virtual void DownloadOpened(DownloadItem* download) = 0;
-    virtual void DownloadRemoved(DownloadItem* download) = 0;
-    virtual void DownloadRenamedToIntermediateName(DownloadItem* download) = 0;
-    virtual void DownloadRenamedToFinalName(DownloadItem* download) = 0;
-
-    // Assert consistent state for delgate object at various transitions.
-    virtual void AssertStateConsistent(DownloadItem* download) const = 0;
-
-   protected:
-    virtual ~Delegate();
-
-   private:
-    // For "Outlives attached DownloadItemImpl" invariant assertion.
-    int count_;
-  };
-
   // Note that it is the responsibility of the caller to ensure that a
-  // DownloadItemImpl::Delegate passed to a DownloadItemImpl constructor
+  // DownloadItemImplDelegate passed to a DownloadItemImpl constructor
   // outlives the DownloadItemImpl.
 
   // Constructing from persistent store:
   // |bound_net_log| is constructed externally for our use.
-  DownloadItemImpl(Delegate* delegate,
+  DownloadItemImpl(DownloadItemImplDelegate* delegate,
                    content::DownloadId download_id,
                    const content::DownloadPersistentStoreInfo& info,
                    const net::BoundNetLog& bound_net_log);
 
   // Constructing for a regular download.
   // |bound_net_log| is constructed externally for our use.
-  DownloadItemImpl(Delegate* delegate,
+  DownloadItemImpl(DownloadItemImplDelegate* delegate,
                    const DownloadCreateInfo& info,
                    scoped_ptr<DownloadRequestHandleInterface> request_handle,
                    bool is_otr,
@@ -102,7 +49,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
 
   // Constructing for the "Save Page As..." feature:
   // |bound_net_log| is constructed externally for our use.
-  DownloadItemImpl(Delegate* delegate,
+  DownloadItemImpl(DownloadItemImplDelegate* delegate,
                    const FilePath& path,
                    const GURL& url,
                    bool is_otr,
@@ -112,6 +59,51 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
 
   virtual ~DownloadItemImpl();
 
+  // Implementation functions (not part of the DownloadItem interface).
+
+  // Indicate that an error has occurred on the download.
+  virtual void Interrupt(content::DownloadInterruptReason reason);
+
+  // Mark the item as having been persisted.
+  virtual void SetIsPersisted();
+
+  // Set the item's DB handle.
+  virtual void SetDbHandle(int64 handle);
+
+  // Cancels the off-thread aspects of the download.
+  // TODO(rdsmith): This should be private and only called from
+  // DownloadItem::Cancel/Interrupt; it isn't now because we can't
+  // call those functions from
+  // DownloadManager::FileSelectionCancelled() without doing some
+  // rewrites of the DownloadManager queues.
+  virtual void OffThreadCancel(DownloadFileManager* file_manager);
+
+  // Called when the downloaded file is removed.
+  virtual void OnDownloadedFileRemoved();
+
+  // Called when the download is ready to complete.
+  // This may perform final rename if necessary and will eventually call
+  // DownloadItem::Completed().
+  virtual void OnDownloadCompleting(DownloadFileManager* file_manager);
+
+  // Called periodically from the download thread, or from the UI thread
+  // for saving packages.
+  // |bytes_so_far| is the number of bytes received so far.
+  // |hash_state| is the current hash state.
+  virtual void UpdateProgress(int64 bytes_so_far,
+                              int64 bytes_per_sec,
+                              const std::string& hash_state);
+
+  // Called by SavePackage to display progress when the DownloadItem
+  // should be considered complete.
+  virtual void MarkAsComplete();
+
+  // Called when all data has been saved. Only has display effects.
+  virtual void OnAllDataSaved(int64 size, const std::string& final_hash);
+
+  // Called by SavePackage to set the total number of bytes on the item.
+  virtual void SetTotalBytes(int64 total_bytes);
+
   // Overridden from DownloadItem.
   virtual void AddObserver(DownloadItem::Observer* observer) OVERRIDE;
   virtual void RemoveObserver(DownloadItem::Observer* observer) OVERRIDE;
@@ -122,16 +114,8 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   virtual void OpenDownload() OVERRIDE;
   virtual void ShowDownloadInShell() OVERRIDE;
   virtual void DangerousDownloadValidated() OVERRIDE;
-  virtual void UpdateProgress(int64 bytes_so_far,
-                              int64 bytes_per_sec,
-                              const std::string& hash_state) OVERRIDE;
   virtual void Cancel(bool user_cancel) OVERRIDE;
-  virtual void Interrupt(content::DownloadInterruptReason reason) OVERRIDE;
-  virtual void MarkAsComplete() OVERRIDE;
   virtual void DelayedDownloadOpened(bool auto_opened) OVERRIDE;
-  virtual void OnAllDataSaved(
-      int64 size, const std::string& final_hash) OVERRIDE;
-  virtual void OnDownloadedFileRemoved() OVERRIDE;
   virtual void Delete(DeleteReason reason) OVERRIDE;
   virtual void Remove() OVERRIDE;
   virtual bool TimeRemaining(base::TimeDelta* remaining) const OVERRIDE;
@@ -139,7 +123,6 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   virtual int PercentComplete() const OVERRIDE;
   virtual bool AllDataSaved() const OVERRIDE;
   virtual void TogglePause() OVERRIDE;
-  virtual void OnDownloadCompleting(DownloadFileManager* file_manager) OVERRIDE;
   virtual bool MatchesQuery(const string16& query) const OVERRIDE;
   virtual bool IsPartialDownload() const OVERRIDE;
   virtual bool IsInProgress() const OVERRIDE;
@@ -170,7 +153,6 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   virtual std::string GetReferrerCharset() const OVERRIDE;
   virtual std::string GetRemoteAddress() const OVERRIDE;
   virtual int64 GetTotalBytes() const OVERRIDE;
-  virtual void SetTotalBytes(int64 total_bytes) OVERRIDE;
   virtual const std::string& GetHash() const OVERRIDE;
   virtual int64 GetReceivedBytes() const OVERRIDE;
   virtual const std::string& GetHashState() const OVERRIDE;
@@ -178,9 +160,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   virtual content::DownloadId GetGlobalId() const OVERRIDE;
   virtual base::Time GetStartTime() const OVERRIDE;
   virtual base::Time GetEndTime() const OVERRIDE;
-  virtual void SetIsPersisted() OVERRIDE;
   virtual bool IsPersisted() const OVERRIDE;
-  virtual void SetDbHandle(int64 handle) OVERRIDE;
   virtual int64 GetDbHandle() const OVERRIDE;
   virtual bool IsPaused() const OVERRIDE;
   virtual bool GetOpenWhenComplete() const OVERRIDE;
@@ -209,7 +189,6 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   virtual FilePath GetFileNameToReportUser() const OVERRIDE;
   virtual void SetDisplayName(const FilePath& name) OVERRIDE;
   virtual FilePath GetUserVerifiedFilePath() const OVERRIDE;
-  virtual void OffThreadCancel(DownloadFileManager* file_manager) OVERRIDE;
   virtual std::string DebugString(bool verbose) const OVERRIDE;
   virtual void MockDownloadOpenForTesting() OVERRIDE;
   virtual ExternalData* GetExternalData(const void* key) OVERRIDE;
@@ -377,7 +356,7 @@ class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
   int64 db_handle_;
 
   // Our delegate.
-  Delegate* delegate_;
+  DownloadItemImplDelegate* delegate_;
 
   // In progress downloads may be paused by the user, we note it here.
   bool is_paused_;
diff --git a/content/browser/download/download_item_impl_delegate.cc b/content/browser/download/download_item_impl_delegate.cc
new file mode 100644
index 0000000..7efcd1c
--- /dev/null
+++ b/content/browser/download/download_item_impl_delegate.cc
@@ -0,0 +1,62 @@
+// 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.
+
+#include "base/logging.h"
+#include "content/browser/download/download_item_impl_delegate.h"
+
+class DownloadItemImpl;
+
+// Infrastructure in DownloadItemImplDelegate to assert invariant that
+// delegate always outlives all attached DownloadItemImpls.
+DownloadItemImplDelegate::DownloadItemImplDelegate()
+    : count_(0) {}
+
+DownloadItemImplDelegate::~DownloadItemImplDelegate() {
+  DCHECK_EQ(0, count_);
+}
+
+void DownloadItemImplDelegate::Attach() {
+  ++count_;
+}
+
+void DownloadItemImplDelegate::Detach() {
+  DCHECK_LT(0, count_);
+  --count_;
+}
+
+bool DownloadItemImplDelegate::ShouldOpenFileBasedOnExtension(
+    const FilePath& path) {
+  return false;
+}
+
+bool DownloadItemImplDelegate::ShouldOpenDownload(DownloadItemImpl* download) {
+  return false;
+}
+
+void DownloadItemImplDelegate::CheckForFileRemoval(
+    DownloadItemImpl* download_item) {}
+
+void DownloadItemImplDelegate::MaybeCompleteDownload(
+    DownloadItemImpl* download) {}
+
+content::BrowserContext* DownloadItemImplDelegate::GetBrowserContext() const {
+  return NULL;
+}
+
+void DownloadItemImplDelegate::DownloadStopped(DownloadItemImpl* download) {}
+
+void DownloadItemImplDelegate::DownloadCompleted(DownloadItemImpl* download) {}
+
+void DownloadItemImplDelegate::DownloadOpened(DownloadItemImpl* download) {}
+
+void DownloadItemImplDelegate::DownloadRemoved(DownloadItemImpl* download) {}
+
+void DownloadItemImplDelegate::DownloadRenamedToIntermediateName(
+    DownloadItemImpl* download) {}
+
+void DownloadItemImplDelegate::DownloadRenamedToFinalName(
+    DownloadItemImpl* download) {}
+
+void DownloadItemImplDelegate::AssertStateConsistent(
+    DownloadItemImpl* download) const {}
diff --git a/content/browser/download/download_item_impl_delegate.h b/content/browser/download/download_item_impl_delegate.h
new file mode 100644
index 0000000..b947af7
--- /dev/null
+++ b/content/browser/download/download_item_impl_delegate.h
@@ -0,0 +1,70 @@
+// 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_ITEM_IMPL_DELEGATE_H_
+#define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_IMPL_DELEGATE_H_
+
+#include "base/file_path.h"
+#include "content/common/content_export.h"
+
+class DownloadItemImpl;
+
+namespace content {
+class BrowserContext;
+}
+
+// Delegate for operations that a DownloadItemImpl can't do for itself.
+// The base implementation of this class does nothing (returning false
+// on predicates) so interfaces not of interest to a derived class may
+// be left unimplemented.
+class CONTENT_EXPORT DownloadItemImplDelegate {
+ public:
+  DownloadItemImplDelegate();
+  virtual ~DownloadItemImplDelegate();
+
+  // Used for catching use-after-free errors.
+  void Attach();
+  void Detach();
+
+  // Tests if a file type should be opened automatically.
+  virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path);
+
+  // Allows the delegate to override the opening of a download. If it returns
+  // true then it's reponsible for opening the item.
+  virtual bool ShouldOpenDownload(DownloadItemImpl* download);
+
+  // Checks whether a downloaded file still exists and updates the
+  // file's state if the file is already removed.
+  // The check may or may not result in a later asynchronous call
+  // to OnDownloadedFileRemoved().
+  virtual void CheckForFileRemoval(DownloadItemImpl* download_item);
+
+  // If all pre-requisites have been met, complete download processing.
+  // TODO(rdsmith): Move into DownloadItem.
+  virtual void MaybeCompleteDownload(DownloadItemImpl* download);
+
+  // For contextual issues like language and prefs.
+  virtual content::BrowserContext* GetBrowserContext() const;
+
+  // Handle any delegate portions of a state change operation on the
+  // DownloadItem.
+  virtual void DownloadStopped(DownloadItemImpl* download);
+  virtual void DownloadCompleted(DownloadItemImpl* download);
+  virtual void DownloadOpened(DownloadItemImpl* download);
+  virtual void DownloadRemoved(DownloadItemImpl* download);
+  virtual void DownloadRenamedToIntermediateName(
+      DownloadItemImpl* download);
+  virtual void DownloadRenamedToFinalName(DownloadItemImpl* download);
+
+  // Assert consistent state for delgate object at various transitions.
+  virtual void AssertStateConsistent(DownloadItemImpl* download) const;
+
+ private:
+  // For "Outlives attached DownloadItemImpl" invariant assertion.
+  int count_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadItemImplDelegate);
+};
+
+#endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_IMPL_DELEGATE_H_
diff --git a/content/browser/download/download_item_impl_unittest.cc b/content/browser/download/download_item_impl_unittest.cc
index c7ac84d..fecb6f5 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -9,6 +9,7 @@
 #include "content/browser/download/download_create_info.h"
 #include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_item_impl.h"
+#include "content/browser/download/download_item_impl_delegate.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/public/browser/download_id.h"
 #include "content/public/browser/download_interrupt_reasons.h"
@@ -31,20 +32,21 @@ using ::testing::Return;
 DownloadId::Domain kValidDownloadItemIdDomain = "valid DownloadId::Domain";
 
 namespace {
-class MockDelegate : public DownloadItemImpl::Delegate {
+class MockDelegate : public DownloadItemImplDelegate {
  public:
   MOCK_METHOD1(ShouldOpenFileBasedOnExtension, bool(const FilePath& path));
-  MOCK_METHOD1(ShouldOpenDownload, bool(DownloadItem* download));
-  MOCK_METHOD1(CheckForFileRemoval, void(DownloadItem* download));
-  MOCK_METHOD1(MaybeCompleteDownload, void(DownloadItem* download));
+  MOCK_METHOD1(ShouldOpenDownload, bool(DownloadItemImpl* download));
+  MOCK_METHOD1(CheckForFileRemoval, void(DownloadItemImpl* download));
+  MOCK_METHOD1(MaybeCompleteDownload, void(DownloadItemImpl* download));
   MOCK_CONST_METHOD0(GetBrowserContext, content::BrowserContext*());
-  MOCK_METHOD1(DownloadStopped, void(DownloadItem* download));
-  MOCK_METHOD1(DownloadCompleted, void(DownloadItem* download));
-  MOCK_METHOD1(DownloadOpened, void(DownloadItem* download));
-  MOCK_METHOD1(DownloadRemoved, void(DownloadItem* download));
-  MOCK_METHOD1(DownloadRenamedToIntermediateName, void(DownloadItem* download));
-  MOCK_METHOD1(DownloadRenamedToFinalName, void(DownloadItem* download));
-  MOCK_CONST_METHOD1(AssertStateConsistent, void(DownloadItem* download));
+  MOCK_METHOD1(DownloadStopped, void(DownloadItemImpl* download));
+  MOCK_METHOD1(DownloadCompleted, void(DownloadItemImpl* download));
+  MOCK_METHOD1(DownloadOpened, void(DownloadItemImpl* download));
+  MOCK_METHOD1(DownloadRemoved, void(DownloadItemImpl* download));
+  MOCK_METHOD1(DownloadRenamedToIntermediateName,
+               void(DownloadItemImpl* download));
+  MOCK_METHOD1(DownloadRenamedToFinalName, void(DownloadItemImpl* download));
+  MOCK_CONST_METHOD1(AssertStateConsistent, void(DownloadItemImpl* download));
 };
 
 class MockRequestHandle : public DownloadRequestHandleInterface {
@@ -162,7 +164,7 @@ class DownloadItemTest : public testing::Test {
   // This class keeps ownership of the created download item; it will
   // be torn down at the end of the test unless DestroyDownloadItem is
   // called.
-  DownloadItem* CreateDownloadItem(DownloadItem::DownloadState state) {
+  DownloadItemImpl* CreateDownloadItem(DownloadItem::DownloadState state) {
     // 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.
@@ -178,7 +180,7 @@ class DownloadItemTest : public testing::Test {
 
     scoped_ptr<DownloadRequestHandleInterface> request_handle(
         new testing::NiceMock<MockRequestHandle>);
-    DownloadItem* download =
+    DownloadItemImpl* download =
         new DownloadItemImpl(&delegate_, *(info_.get()),
                              request_handle.Pass(), false, net::BoundNetLog());
     allocated_downloads_.insert(download);
@@ -226,7 +228,7 @@ const FilePath::CharType kDummyPath[] = FILE_PATH_LITERAL("/testpath");
 //  set_* mutators
 
 TEST_F(DownloadItemTest, NotificationAfterUpdate) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->UpdateProgress(kDownloadChunkSize, kDownloadSpeed, "");
@@ -235,13 +237,14 @@ TEST_F(DownloadItemTest, NotificationAfterUpdate) {
 }
 
 TEST_F(DownloadItemTest, NotificationAfterCancel) {
-  DownloadItem* user_cancel = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* user_cancel = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer1(user_cancel);
 
   user_cancel->Cancel(true);
   ASSERT_TRUE(observer1.CheckUpdated());
 
-  DownloadItem* system_cancel = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* system_cancel =
+      CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer2(system_cancel);
 
   system_cancel->Cancel(false);
@@ -249,7 +252,7 @@ TEST_F(DownloadItemTest, NotificationAfterCancel) {
 }
 
 TEST_F(DownloadItemTest, NotificationAfterComplete) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->OnAllDataSaved(kDownloadChunkSize, DownloadItem::kEmptyFileHash);
@@ -260,7 +263,7 @@ TEST_F(DownloadItemTest, NotificationAfterComplete) {
 }
 
 TEST_F(DownloadItemTest, NotificationAfterDownloadedFileRemoved) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->OnDownloadedFileRemoved();
@@ -268,7 +271,7 @@ TEST_F(DownloadItemTest, NotificationAfterDownloadedFileRemoved) {
 }
 
 TEST_F(DownloadItemTest, NotificationAfterInterrupted) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->Interrupt(content::DOWNLOAD_INTERRUPT_REASON_NONE);
@@ -276,7 +279,7 @@ TEST_F(DownloadItemTest, NotificationAfterInterrupted) {
 }
 
 TEST_F(DownloadItemTest, NotificationAfterDelete) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->Delete(DownloadItem::DELETE_DUE_TO_BROWSER_SHUTDOWN);
@@ -284,7 +287,7 @@ TEST_F(DownloadItemTest, NotificationAfterDelete) {
 }
 
 TEST_F(DownloadItemTest, NotificationAfterRemove) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->Remove();
@@ -292,7 +295,7 @@ TEST_F(DownloadItemTest, NotificationAfterRemove) {
 }
 
 TEST_F(DownloadItemTest, NotificationAfterOnTargetPathDetermined) {
-  DownloadItem* safe_item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* safe_item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver safe_observer(safe_item);
 
   // Calling OnTargetPathDetermined does not trigger notification if danger type
@@ -302,7 +305,8 @@ TEST_F(DownloadItemTest, NotificationAfterOnTargetPathDetermined) {
       content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
   EXPECT_FALSE(safe_observer.CheckUpdated());
 
-  DownloadItem* dangerous_item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* dangerous_item =
+      CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver dangerous_observer(dangerous_item);
 
   // Calling OnTargetPathDetermined does trigger notification if danger type
@@ -314,7 +318,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnTargetPathDetermined) {
 }
 
 TEST_F(DownloadItemTest, NotificationAfterOnTargetPathSelected) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->OnTargetPathDetermined(
@@ -326,7 +330,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnTargetPathSelected) {
 
 TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
   // Setting to NOT_DANGEROUS does not trigger a notification.
-  DownloadItem* safe_item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* safe_item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver safe_observer(safe_item);
 
   safe_item->OnTargetPathDetermined(
@@ -340,7 +344,8 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
   EXPECT_FALSE(safe_observer.CheckUpdated());
 
   // Setting to unsafe url or unsafe file should trigger a notification.
-  DownloadItem* unsafeurl_item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* unsafeurl_item =
+      CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver unsafeurl_observer(unsafeurl_item);
 
   unsafeurl_item->OnTargetPathDetermined(
@@ -356,7 +361,8 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
   unsafeurl_item->DangerousDownloadValidated();
   EXPECT_TRUE(unsafeurl_observer.CheckUpdated());
 
-  DownloadItem* unsafefile_item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* unsafefile_item =
+      CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver unsafefile_observer(unsafefile_item);
 
   unsafefile_item->OnTargetPathDetermined(
@@ -379,7 +385,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
 // name. Check that observers are updated when the new filename is available and
 // not before.
 TEST_F(DownloadItemTest, NotificationAfterOnIntermediatePathDetermined) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
   FilePath intermediate_path(kDummyPath);
   FilePath new_intermediate_path(intermediate_path.AppendASCII("foo"));
@@ -397,7 +403,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnIntermediatePathDetermined) {
 }
 
 TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   MockObserver observer(item);
 
   item->TogglePause();
@@ -408,7 +414,7 @@ TEST_F(DownloadItemTest, NotificationAfterTogglePause) {
 }
 
 TEST_F(DownloadItemTest, DisplayName) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   item->OnTargetPathDetermined(FilePath(kDummyPath).AppendASCII("foo.bar"),
                                DownloadItem::TARGET_DISPOSITION_OVERWRITE,
                                content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
@@ -431,8 +437,8 @@ class TestExternalData : public DownloadItem::ExternalData {
 };
 
 TEST_F(DownloadItemTest, ExternalData) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
-  const DownloadItem* const_item = item;
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  const DownloadItemImpl* const_item = item;
 
   // Shouldn't be anything there before set.
   EXPECT_EQ(NULL, item->GetExternalData(&external_data_test_string));
@@ -492,7 +498,7 @@ TEST_F(DownloadItemTest, ExternalData) {
 // Delegate::DownloadRenamedToFinalName() should be invoked after the final
 // rename.
 TEST_F(DownloadItemTest, CallbackAfterRename) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
   FilePath intermediate_path(kDummyPath);
   FilePath new_intermediate_path(intermediate_path.AppendASCII("foo"));
   FilePath final_path(intermediate_path.AppendASCII("bar"));
@@ -546,7 +552,7 @@ TEST_F(DownloadItemTest, CallbackAfterRename) {
 }
 
 TEST_F(DownloadItemTest, Interrupted) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
 
   const content::DownloadInterruptReason reason(
       content::DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
@@ -564,7 +570,7 @@ TEST_F(DownloadItemTest, Interrupted) {
 }
 
 TEST_F(DownloadItemTest, Canceled) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
 
   // Confirm cancel sets state properly.
   EXPECT_CALL(*mock_delegate(), DownloadStopped(item));
@@ -573,7 +579,7 @@ TEST_F(DownloadItemTest, Canceled) {
 }
 
 TEST_F(DownloadItemTest, FileRemoved) {
-  DownloadItem* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
+  DownloadItemImpl* item = CreateDownloadItem(DownloadItem::IN_PROGRESS);
 
   EXPECT_FALSE(item->GetFileExternallyRemoved());
   item->OnDownloadedFileRemoved();
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index f742c7c..a2806ba 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -165,16 +165,16 @@ class DownloadItemFactoryImpl : public content::DownloadItemFactory {
     DownloadItemFactoryImpl() {}
     virtual ~DownloadItemFactoryImpl() {}
 
-    virtual content::DownloadItem* CreatePersistedItem(
-        DownloadItemImpl::Delegate* delegate,
+    virtual DownloadItemImpl* CreatePersistedItem(
+        DownloadItemImplDelegate* 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,
+    virtual DownloadItemImpl* CreateActiveItem(
+        DownloadItemImplDelegate* delegate,
         const DownloadCreateInfo& info,
         scoped_ptr<DownloadRequestHandleInterface> request_handle,
         bool is_otr,
@@ -183,8 +183,8 @@ class DownloadItemFactoryImpl : public content::DownloadItemFactory {
                                   is_otr, bound_net_log);
     }
 
-    virtual content::DownloadItem* CreateSavePageItem(
-        DownloadItemImpl::Delegate* delegate,
+    virtual DownloadItemImpl* CreateSavePageItem(
+        DownloadItemImplDelegate* delegate,
         const FilePath& path,
         const GURL& url,
         bool is_otr,
@@ -244,7 +244,7 @@ DownloadId DownloadManagerImpl::GetNextId() {
   return id;
 }
 
-bool DownloadManagerImpl::ShouldOpenDownload(DownloadItem* item) {
+bool DownloadManagerImpl::ShouldOpenDownload(DownloadItemImpl* item) {
   if (!delegate_)
     return true;
 
@@ -288,7 +288,7 @@ void DownloadManagerImpl::Shutdown() {
   // Go through all downloads in downloads_.  Dangerous ones we need to
   // remove on disk, and in progress ones we need to cancel.
   for (DownloadMap::iterator it = downloads_.begin(); it != downloads_.end();) {
-    DownloadItem* download = it->second;
+    DownloadItemImpl* download = it->second;
 
     // Save iterator from potential erases in this set done by called code.
     // Iterators after an erasure point are still valid for lists and
@@ -339,7 +339,7 @@ void DownloadManagerImpl::GetTemporaryDownloads(
 
   for (DownloadMap::iterator it = downloads_.begin();
        it != downloads_.end(); ++it) {
-    DownloadItem* item = it->second;
+    DownloadItemImpl* item = it->second;
     // TODO(benjhayden): Don't check IsPersisted().
     if (item->IsTemporary() &&
         item->IsPersisted() &&
@@ -355,7 +355,7 @@ void DownloadManagerImpl::GetAllDownloads(
 
   for (DownloadMap::iterator it = downloads_.begin();
        it != downloads_.end(); ++it) {
-    DownloadItem* item = it->second;
+    DownloadItemImpl* item = it->second;
     // TODO(benjhayden): Don't check IsPersisted().
     if (!item->IsTemporary() &&
         item->IsPersisted() &&
@@ -371,7 +371,7 @@ void DownloadManagerImpl::SearchDownloads(const string16& query,
 
   for (DownloadMap::iterator it = downloads_.begin();
        it != downloads_.end(); ++it) {
-    DownloadItem* download_item = it->second;
+    DownloadItemImpl* download_item = it->second;
     // Display Incognito downloads only in Incognito window, and vice versa.
     // The Incognito Downloads page will get the list of non-Incognito downloads
     // from its parent profile.
@@ -446,13 +446,13 @@ void DownloadManagerImpl::CheckForHistoryFilesRemoval() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   for (DownloadMap::iterator it = downloads_.begin();
        it != downloads_.end(); ++it) {
-    DownloadItem* item = it->second;
+    DownloadItemImpl* item = it->second;
     if (item->IsPersisted())
       CheckForFileRemoval(item);
   }
 }
 
-void DownloadManagerImpl::CheckForFileRemoval(DownloadItem* download_item) {
+void DownloadManagerImpl::CheckForFileRemoval(DownloadItemImpl* download_item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (download_item->IsComplete() &&
       !download_item->GetFileExternallyRemoved()) {
@@ -478,17 +478,16 @@ void DownloadManagerImpl::CheckForFileRemovalOnFileThread(
 
 void DownloadManagerImpl::OnFileRemovalDetected(int32 download_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DownloadItem* download = GetDownload(download_id);
-  if (download)
-    download->OnDownloadedFileRemoved();
+  if (ContainsKey(downloads_, download_id))
+    downloads_[download_id]->OnDownloadedFileRemoved();
 }
 
 void DownloadManagerImpl::RestartDownload(int32 download_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  DownloadItem* download = GetActiveDownloadItem(download_id);
-  if (!download)
+  if (!ContainsKey(active_downloads_, download_id))
     return;
+  DownloadItemImpl* download = active_downloads_[download_id];
 
   VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
@@ -525,7 +524,7 @@ net::BoundNetLog DownloadManagerImpl::CreateDownloadItem(
       net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
   if (!info->download_id.IsValid())
     info->download_id = GetNextId();
-  DownloadItem* download = factory_->CreateActiveItem(
+  DownloadItemImpl* download = factory_->CreateActiveItem(
       this, *info,
       scoped_ptr<DownloadRequestHandleInterface>(
           new DownloadRequestHandle(info->request_handle)).Pass(),
@@ -539,7 +538,7 @@ net::BoundNetLog DownloadManagerImpl::CreateDownloadItem(
   return bound_net_log;
 }
 
-DownloadItem* DownloadManagerImpl::CreateSavePackageDownloadItem(
+DownloadItemImpl* DownloadManagerImpl::CreateSavePackageDownloadItem(
     const FilePath& main_file_path,
     const GURL& page_url,
     bool is_otr,
@@ -547,7 +546,7 @@ DownloadItem* DownloadManagerImpl::CreateSavePackageDownloadItem(
     DownloadItem::Observer* observer) {
   net::BoundNetLog bound_net_log =
       net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
-  DownloadItem* download = factory_->CreateSavePageItem(
+  DownloadItemImpl* download = factory_->CreateSavePageItem(
       this,
       main_file_path,
       page_url,
@@ -573,7 +572,7 @@ DownloadItem* DownloadManagerImpl::CreateSavePackageDownloadItem(
 
 // The target path for the download item is now valid. We proceed with the
 // determination of an intermediate path.
-void DownloadManagerImpl::OnTargetPathAvailable(DownloadItem* download) {
+void DownloadManagerImpl::OnTargetPathAvailable(DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(download);
   DCHECK(ContainsKey(downloads_, download->GetId()));
@@ -608,7 +607,7 @@ void DownloadManagerImpl::UpdateDownload(int32 download_id,
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadMap::iterator it = active_downloads_.find(download_id);
   if (it != active_downloads_.end()) {
-    DownloadItem* download = it->second;
+    DownloadItemImpl* download = it->second;
     if (download->IsInProgress()) {
       download->UpdateProgress(bytes_so_far, bytes_per_sec, hash_state);
       if (delegate_)
@@ -630,12 +629,13 @@ void DownloadManagerImpl::OnResponseCompleted(int32 download_id,
   if (active_downloads_.count(download_id) == 0)
     return;
 
-  DownloadItem* download = active_downloads_[download_id];
+  DownloadItemImpl* download = active_downloads_[download_id];
   download->OnAllDataSaved(size, hash);
   MaybeCompleteDownload(download);
 }
 
-void DownloadManagerImpl::AssertStateConsistent(DownloadItem* download) const {
+void DownloadManagerImpl::AssertStateConsistent(
+    DownloadItemImpl* download) const {
   if (download->GetState() == DownloadItem::REMOVING) {
     DCHECK(!ContainsKey(downloads_, download->GetId()));
     DCHECK(!ContainsKey(active_downloads_, download->GetId()));
@@ -657,7 +657,8 @@ void DownloadManagerImpl::AssertStateConsistent(DownloadItem* download) const {
     CHECK(ContainsKey(active_downloads_, download->GetId()));
 }
 
-bool DownloadManagerImpl::IsDownloadReadyForCompletion(DownloadItem* download) {
+bool DownloadManagerImpl::IsDownloadReadyForCompletion(
+    DownloadItemImpl* download) {
   // If we don't have all the data, the download is not ready for
   // completion.
   if (!download->AllDataSaved())
@@ -692,7 +693,8 @@ bool DownloadManagerImpl::IsDownloadReadyForCompletion(DownloadItem* download) {
 // downloads. SavePackage always uses its own Finish() to mark downloads
 // complete.
 
-void DownloadManagerImpl::MaybeCompleteDownload(DownloadItem* download) {
+void DownloadManagerImpl::MaybeCompleteDownload(
+    DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   VLOG(20) << __FUNCTION__ << "()" << " download = "
            << download->DebugString(false);
@@ -730,12 +732,11 @@ void DownloadManagerImpl::MaybeCompleteDownload(DownloadItem* download) {
 }
 
 void DownloadManagerImpl::MaybeCompleteDownloadById(int download_id) {
-  DownloadItem* download_item = GetActiveDownload(download_id);
-  if (download_item != NULL)
-    MaybeCompleteDownload(download_item);
+  if (ContainsKey(active_downloads_, download_id))
+    MaybeCompleteDownload(active_downloads_[download_id]);
 }
 
-void DownloadManagerImpl::DownloadCompleted(DownloadItem* download) {
+void DownloadManagerImpl::DownloadCompleted(DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(download);
   if (delegate_)
@@ -745,16 +746,13 @@ void DownloadManagerImpl::DownloadCompleted(DownloadItem* download) {
 }
 
 void DownloadManagerImpl::CancelDownload(int32 download_id) {
-  DownloadItem* download = GetActiveDownload(download_id);
   // A cancel at the right time could remove the download from the
   // |active_downloads_| map before we get here.
-  if (!download)
-    return;
-
-  download->Cancel(true);
+  if (ContainsKey(active_downloads_, download_id))
+    active_downloads_[download_id]->Cancel(true);
 }
 
-void DownloadManagerImpl::DownloadStopped(DownloadItem* download) {
+void DownloadManagerImpl::DownloadStopped(DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   VLOG(20) << __FUNCTION__ << "()"
@@ -776,28 +774,15 @@ void DownloadManagerImpl::OnDownloadInterrupted(
     content::DownloadInterruptReason reason) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  DownloadItem* download = GetActiveDownload(download_id);
-  if (!download)
+  if (!ContainsKey(active_downloads_, download_id))
     return;
+
+  DownloadItemImpl* download = active_downloads_[download_id];
   download->UpdateProgress(size, 0, hash_state);
   download->Interrupt(reason);
 }
 
-DownloadItem* DownloadManagerImpl::GetActiveDownload(int32 download_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DownloadMap::iterator it = active_downloads_.find(download_id);
-  if (it == active_downloads_.end())
-    return NULL;
-
-  DownloadItem* download = it->second;
-
-  DCHECK(download);
-  DCHECK_EQ(download_id, download->GetId());
-
-  return download;
-}
-
-void DownloadManagerImpl::RemoveFromActiveList(DownloadItem* download) {
+void DownloadManagerImpl::RemoveFromActiveList(DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(download);
 
@@ -815,15 +800,15 @@ bool DownloadManagerImpl::GenerateFileHash() {
 }
 
 int DownloadManagerImpl::RemoveDownloadItems(
-    const DownloadVector& pending_deletes) {
+    const DownloadItemImplVector& pending_deletes) {
   if (pending_deletes.empty())
     return 0;
 
   // Delete from internal maps.
-  for (DownloadVector::const_iterator it = pending_deletes.begin();
+  for (DownloadItemImplVector::const_iterator it = pending_deletes.begin();
       it != pending_deletes.end();
       ++it) {
-    DownloadItem* download = *it;
+    DownloadItemImpl* download = *it;
     DCHECK(download);
     downloads_.erase(download->GetId());
   }
@@ -837,7 +822,7 @@ int DownloadManagerImpl::RemoveDownloadItems(
   return num_deleted;
 }
 
-void DownloadManagerImpl::DownloadRemoved(DownloadItem* download) {
+void DownloadManagerImpl::DownloadRemoved(DownloadItemImpl* download) {
   if (!download ||
       downloads_.find(download->GetId()) == downloads_.end())
     return;
@@ -851,7 +836,8 @@ void DownloadManagerImpl::DownloadRemoved(DownloadItem* download) {
     delegate_->RemoveItemFromPersistentStore(download);
 
   // Remove from our tables and delete.
-  int downloads_count = RemoveDownloadItems(DownloadVector(1, download));
+  int downloads_count =
+      RemoveDownloadItems(DownloadItemImplVector(1, download));
   DCHECK_EQ(1, downloads_count);
 }
 
@@ -862,11 +848,11 @@ int DownloadManagerImpl::RemoveDownloadsBetween(base::Time remove_begin,
 
   // All downloads visible to the user will be in the history,
   // so scan that map.
-  DownloadVector pending_deletes;
+  DownloadItemImplVector pending_deletes;
   for (DownloadMap::const_iterator it = downloads_.begin();
       it != downloads_.end();
       ++it) {
-    DownloadItem* download = it->second;
+    DownloadItemImpl* download = it->second;
     if (download->IsPersisted() &&
         download->GetStartTime() >= remove_begin &&
         (remove_end.is_null() || download->GetStartTime() < remove_end) &&
@@ -917,9 +903,10 @@ void DownloadManagerImpl::FileSelected(const FilePath& path,
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!path.empty());
 
-  DownloadItem* download = GetActiveDownloadItem(download_id);
-  if (!download)
+  if (!ContainsKey(active_downloads_, download_id))
     return;
+  DownloadItemImpl* download = active_downloads_[download_id];
+
   VLOG(20) << __FUNCTION__ << "()" << " path = \"" << path.value() << "\""
            << " download = " << download->DebugString(true);
 
@@ -938,9 +925,9 @@ void DownloadManagerImpl::FileSelectionCanceled(int32 download_id) {
   // download that's already in progress to the temporary location.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  DownloadItem* download = GetActiveDownloadItem(download_id);
-  if (!download)
+  if (!ContainsKey(active_downloads_, download_id))
     return;
+  DownloadItemImpl* download = active_downloads_[download_id];
 
   VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
@@ -961,7 +948,7 @@ void DownloadManagerImpl::OnPersistentStoreQueryComplete(
 
     net::BoundNetLog bound_net_log =
         net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
-    DownloadItem* download = factory_->CreatePersistedItem(
+    DownloadItemImpl* download = factory_->CreatePersistedItem(
         this, GetNextId(), entries->at(i), bound_net_log);
     DCHECK(!ContainsKey(downloads_, download->GetId()));
     downloads_[download->GetId()] = download;
@@ -972,7 +959,7 @@ void DownloadManagerImpl::OnPersistentStoreQueryComplete(
   CheckForHistoryFilesRemoval();
 }
 
-void DownloadManagerImpl::AddDownloadItemToHistory(DownloadItem* download,
+void DownloadManagerImpl::AddDownloadItemToHistory(DownloadItemImpl* download,
                                                    int64 db_handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK_NE(DownloadItem::kUninitializedHandle, db_handle);
@@ -995,10 +982,11 @@ void DownloadManagerImpl::AddDownloadItemToHistory(DownloadItem* download,
 
 void DownloadManagerImpl::OnItemAddedToPersistentStore(int32 download_id,
                                                        int64 db_handle) {
-  DownloadItem* item = GetDownload(download_id);
   // It's valid that we don't find a matching item, i.e. on shutdown.
-  if (!item)
+  if (!ContainsKey(downloads_, download_id))
     return;
+
+  DownloadItemImpl* item = downloads_[download_id];
   AddDownloadItemToHistory(item, db_handle);
   if (SavePageExternalData::Get(item)) {
     OnSavePageItemAddedToPersistentStore(item);
@@ -1014,9 +1002,8 @@ void DownloadManagerImpl::OnItemAddedToPersistentStore(int32 download_id,
 // item when it's created so they can observe it directly. Are there any
 // clients that actually need to know when the item is added to the history?).
 void DownloadManagerImpl::OnDownloadItemAddedToPersistentStore(
-    DownloadItem* item) {
+    DownloadItemImpl* item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
   VLOG(20) << __FUNCTION__ << "()" << " db_handle = " << item->GetDbHandle()
            << " download_id = " << item->GetId()
            << " download = " << item->DebugString(true);
@@ -1039,7 +1026,7 @@ void DownloadManagerImpl::OnDownloadItemAddedToPersistentStore(
   }
 }
 
-void DownloadManagerImpl::ShowDownloadInBrowser(DownloadItem* download) {
+void DownloadManagerImpl::ShowDownloadInBrowser(DownloadItemImpl* download) {
   // The 'contents' may no longer exist if the user closed the contents before
   // we get this start completion event.
   WebContents* content = download->GetWebContents();
@@ -1060,7 +1047,7 @@ int DownloadManagerImpl::InProgressCount() const {
   int count = 0;
   for (DownloadMap::const_iterator it = active_downloads_.begin();
        it != active_downloads_.end(); ++it) {
-    DownloadItem* item = it->second;
+    DownloadItemImpl* item = it->second;
     if (item->IsInProgress())
       ++count;
   }
@@ -1140,7 +1127,7 @@ void DownloadManagerImpl::AssertContainersConsistent() const {
 // to solve that without canceling on Remove (which would then update the DB).
 
 void DownloadManagerImpl::OnSavePageItemAddedToPersistentStore(
-    DownloadItem* item) {
+    DownloadItemImpl* item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Finalize this download if it finished before the history callback.
@@ -1148,7 +1135,8 @@ void DownloadManagerImpl::OnSavePageItemAddedToPersistentStore(
     SavePageDownloadFinished(item);
 }
 
-void DownloadManagerImpl::SavePageDownloadFinished(DownloadItem* download) {
+void DownloadManagerImpl::SavePageDownloadFinished(
+    content::DownloadItem* download) {
   if (download->IsPersisted()) {
     if (delegate_)
       delegate_->UpdateItemInPersistentStore(download);
@@ -1160,13 +1148,13 @@ void DownloadManagerImpl::SavePageDownloadFinished(DownloadItem* download) {
   }
 }
 
-void DownloadManagerImpl::DownloadOpened(DownloadItem* download) {
+void DownloadManagerImpl::DownloadOpened(DownloadItemImpl* download) {
   if (delegate_)
     delegate_->UpdateItemInPersistentStore(download);
   int num_unopened = 0;
   for (DownloadMap::iterator it = downloads_.begin();
        it != downloads_.end(); ++it) {
-    DownloadItem* item = it->second;
+    DownloadItemImpl* item = it->second;
     if (item->IsComplete() &&
         !item->GetOpened())
       ++num_unopened;
@@ -1175,7 +1163,7 @@ void DownloadManagerImpl::DownloadOpened(DownloadItem* download) {
 }
 
 void DownloadManagerImpl::DownloadRenamedToIntermediateName(
-    DownloadItem* download) {
+    DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // If the rename failed, we receive an OnDownloadInterrupted() call before we
   // receive the DownloadRenamedToIntermediateName() call.
@@ -1184,7 +1172,7 @@ void DownloadManagerImpl::DownloadRenamedToIntermediateName(
 }
 
 void DownloadManagerImpl::DownloadRenamedToFinalName(
-    DownloadItem* download) {
+    DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // If the rename failed, we receive an OnDownloadInterrupted() call before we
   // receive the DownloadRenamedToFinalName() call.
diff --git a/content/browser/download/download_manager_impl.h b/content/browser/download/download_manager_impl.h
index ad79935..43e69da 100644
--- a/content/browser/download/download_manager_impl.h
+++ b/content/browser/download/download_manager_impl.h
@@ -16,15 +16,16 @@
 #include "base/sequenced_task_runner_helpers.h"
 #include "base/synchronization/lock.h"
 #include "content/browser/download/download_item_factory.h"
-#include "content/browser/download/download_item_impl.h"
+#include "content/browser/download/download_item_impl_delegate.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/download_manager.h"
 
 class DownloadFileManager;
+class DownloadItemImpl;
 
 class CONTENT_EXPORT DownloadManagerImpl
     : public content::DownloadManager,
-      public DownloadItemImpl::Delegate {
+      private DownloadItemImplDelegate {
  public:
   // Caller guarantees that |file_manager| and |net_log| will remain valid
   // for the lifetime of DownloadManagerImpl (until Shutdown() is called).
@@ -35,6 +36,18 @@ class CONTENT_EXPORT DownloadManagerImpl
                       scoped_ptr<content::DownloadItemFactory> factory,
                       net::NetLog* net_log);
 
+  // Implementation functions (not part of the DownloadManager interface).
+
+  // Creates a download item for the SavePackage system.
+  // Must be called on the UI thread.  Note that the DownloadManager
+  // retains ownership.
+  virtual DownloadItemImpl* CreateSavePackageDownloadItem(
+      const FilePath& main_file_path,
+      const GURL& page_url,
+      bool is_otr,
+      const std::string& mime_type,
+      content::DownloadItem::Observer* observer);
+
   // content::DownloadManager functions.
   virtual void SetDelegate(content::DownloadManagerDelegate* delegate) OVERRIDE;
   virtual content::DownloadManagerDelegate* GetDelegate() const OVERRIDE;
@@ -76,14 +89,6 @@ class CONTENT_EXPORT DownloadManagerImpl
   virtual int InProgressCount() const OVERRIDE;
   virtual content::BrowserContext* GetBrowserContext() const OVERRIDE;
   virtual FilePath LastDownloadPath() OVERRIDE;
-  virtual net::BoundNetLog CreateDownloadItem(
-      DownloadCreateInfo* info) OVERRIDE;
-  virtual content::DownloadItem* CreateSavePackageDownloadItem(
-      const FilePath& main_file_path,
-      const GURL& page_url,
-      bool is_otr,
-      const std::string& mime_type,
-      content::DownloadItem::Observer* observer) OVERRIDE;
   virtual void ClearLastDownloadPath() OVERRIDE;
   virtual void FileSelected(const FilePath& path, int32 download_id) OVERRIDE;
   virtual void FileSelectionCanceled(int32 download_id) OVERRIDE;
@@ -96,32 +101,10 @@ class CONTENT_EXPORT DownloadManagerImpl
   virtual content::DownloadItem* GetActiveDownloadItem(int id) OVERRIDE;
   virtual bool GenerateFileHash() OVERRIDE;
 
-  // Overridden from DownloadItemImpl::Delegate
-  // (Note that |GetBrowserContext| are present in both interfaces.)
-  virtual bool ShouldOpenDownload(content::DownloadItem* item) OVERRIDE;
-  virtual bool ShouldOpenFileBasedOnExtension(
-      const FilePath& path) OVERRIDE;
-  virtual void CheckForFileRemoval(
-      content::DownloadItem* download_item) OVERRIDE;
-  virtual void MaybeCompleteDownload(
-      content::DownloadItem* download) OVERRIDE;
-  virtual void DownloadStopped(
-      content::DownloadItem* download) OVERRIDE;
-  virtual void DownloadCompleted(
-      content::DownloadItem* download) OVERRIDE;
-  virtual void DownloadOpened(
-      content::DownloadItem* download) OVERRIDE;
-  virtual void DownloadRemoved(content::DownloadItem* download) OVERRIDE;
-  virtual void DownloadRenamedToIntermediateName(
-      content::DownloadItem* download) OVERRIDE;
-  virtual void DownloadRenamedToFinalName(
-      content::DownloadItem* download) OVERRIDE;
-  virtual void AssertStateConsistent(
-      content::DownloadItem* download) const OVERRIDE;
-
  private:
   typedef std::set<content::DownloadItem*> DownloadSet;
-  typedef base::hash_map<int32, content::DownloadItem*> DownloadMap;
+  typedef base::hash_map<int32, DownloadItemImpl*> DownloadMap;
+  typedef std::vector<DownloadItemImpl*> DownloadItemImplVector;
 
   // For testing.
   friend class DownloadManagerTest;
@@ -131,16 +114,20 @@ class CONTENT_EXPORT DownloadManagerImpl
 
   virtual ~DownloadManagerImpl();
 
+  // Creates the download item.  Must be called on the UI thread.
+  // Returns the |BoundNetLog| used by the |DownloadItem|.
+  virtual net::BoundNetLog CreateDownloadItem(DownloadCreateInfo* info);
+
   // Does nothing if |download_id| is not an active download.
   void MaybeCompleteDownloadById(int download_id);
 
   // Determine if the download is ready for completion, i.e. has had
   // all data saved, and completed the filename determination and
   // history insertion.
-  bool IsDownloadReadyForCompletion(content::DownloadItem* download);
+  bool IsDownloadReadyForCompletion(DownloadItemImpl* download);
 
   // Show the download in the browser.
-  void ShowDownloadInBrowser(content::DownloadItem* download);
+  void ShowDownloadInBrowser(DownloadItemImpl* download);
 
   // Get next download id.
   content::DownloadId GetNextId();
@@ -156,16 +143,12 @@ class CONTENT_EXPORT DownloadManagerImpl
   // Called back after a target path for the file to be downloaded to has been
   // determined, either automatically based on the suggested file name, or by
   // the user in a Save As dialog box.
-  void OnTargetPathAvailable(content::DownloadItem* download);
-
-  // Retrieves the download from the |download_id|.
-  // Returns NULL if the download is not active.
-  content::DownloadItem* GetActiveDownload(int32 download_id);
+  void OnTargetPathAvailable(DownloadItemImpl* download);
 
   // Removes |download| from the active and in progress maps.
   // Called when the download is cancelled or has an error.
   // Does nothing if the download is not in the history DB.
-  void RemoveFromActiveList(content::DownloadItem* download);
+  void RemoveFromActiveList(DownloadItemImpl* download);
 
   // Inform observers that the model has changed.
   void NotifyModelChanged();
@@ -175,10 +158,10 @@ class CONTENT_EXPORT DownloadManagerImpl
   void AssertContainersConsistent() const;
 
   // Add a DownloadItem to history_downloads_.
-  void AddDownloadItemToHistory(content::DownloadItem* item, int64 db_handle);
+  void AddDownloadItemToHistory(DownloadItemImpl* item, int64 db_handle);
 
   // Remove from internal maps.
-  int RemoveDownloadItems(const DownloadVector& pending_deletes);
+  int RemoveDownloadItems(const DownloadItemImplVector& pending_deletes);
 
   // Called in response to our request to the DownloadFileManager to
   // create a DownloadFile.  A |reason| of
@@ -187,10 +170,25 @@ class CONTENT_EXPORT DownloadManagerImpl
       int32 download_id, content::DownloadInterruptReason reason);
 
   // Called when a download entry is committed to the persistent store.
-  void OnDownloadItemAddedToPersistentStore(content::DownloadItem* item);
+  void OnDownloadItemAddedToPersistentStore(DownloadItemImpl* item);
 
   // Called when Save Page As entry is committed to the persistent store.
-  void OnSavePageItemAddedToPersistentStore(content::DownloadItem* item);
+  void OnSavePageItemAddedToPersistentStore(DownloadItemImpl* item);
+
+  // Overridden from DownloadItemImplDelegate
+  // (Note that |GetBrowserContext| are present in both interfaces.)
+  virtual bool ShouldOpenDownload(DownloadItemImpl* item) OVERRIDE;
+  virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path) OVERRIDE;
+  virtual void CheckForFileRemoval(DownloadItemImpl* download_item) OVERRIDE;
+  virtual void MaybeCompleteDownload(DownloadItemImpl* download) OVERRIDE;
+  virtual void DownloadStopped(DownloadItemImpl* download) OVERRIDE;
+  virtual void DownloadCompleted(DownloadItemImpl* download) OVERRIDE;
+  virtual void DownloadOpened(DownloadItemImpl* download) OVERRIDE;
+  virtual void DownloadRemoved(DownloadItemImpl* download) OVERRIDE;
+  virtual void DownloadRenamedToIntermediateName(
+      DownloadItemImpl* download) OVERRIDE;
+  virtual void DownloadRenamedToFinalName(DownloadItemImpl* download) OVERRIDE;
+  virtual void AssertStateConsistent(DownloadItemImpl* download) const OVERRIDE;
 
   // Factory for creation of downloads items.
   scoped_ptr<content::DownloadItemFactory> factory_;
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc
index 124d4ce..9aae513 100644
--- a/content/browser/download/download_manager_impl_unittest.cc
+++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -20,6 +20,8 @@
 #include "content/browser/download/download_create_info.h"
 #include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_item_factory.h"
+#include "content/browser/download/download_item_impl.h"
+#include "content/browser/download/download_item_impl_delegate.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"
@@ -42,6 +44,7 @@ using ::testing::ReturnRef;
 using ::testing::SetArgPointee;
 using ::testing::StrictMock;
 using ::testing::_;
+using content::DownloadInterruptReason;
 using content::DownloadItem;
 using content::DownloadManager;
 using content::WebContents;
@@ -52,6 +55,116 @@ class ByteStreamReader;
 
 namespace {
 
+class MockDownloadItemImpl : public DownloadItemImpl {
+ public:
+  // Use history constructor for minimal base object.
+  MockDownloadItemImpl(DownloadItemImplDelegate* delegate)
+      : DownloadItemImpl(delegate, content::DownloadId(),
+                         content::DownloadPersistentStoreInfo(),
+                         net::BoundNetLog()) {}
+  virtual ~MockDownloadItemImpl() {}
+
+  MOCK_METHOD1(AddObserver, void(content::DownloadItem::Observer*));
+  MOCK_METHOD1(RemoveObserver, void(content::DownloadItem::Observer*));
+  MOCK_METHOD0(UpdateObservers, void());
+  MOCK_METHOD0(CanShowInFolder, bool());
+  MOCK_METHOD0(CanOpenDownload, bool());
+  MOCK_METHOD0(ShouldOpenFileBasedOnExtension, bool());
+  MOCK_METHOD0(OpenDownload, void());
+  MOCK_METHOD0(ShowDownloadInShell, void());
+  MOCK_METHOD0(DangerousDownloadValidated, void());
+  MOCK_METHOD3(UpdateProgress, void(int64, int64, const std::string&));
+  MOCK_METHOD1(Cancel, void(bool));
+  MOCK_METHOD0(MarkAsComplete, void());
+  MOCK_METHOD1(DelayedDownloadOpened, void(bool));
+  MOCK_METHOD2(OnAllDataSaved, void(int64, const std::string&));
+  MOCK_METHOD0(OnDownloadedFileRemoved, void());
+  MOCK_METHOD0(MaybeCompleteDownload, void());
+  MOCK_METHOD1(Interrupt, void(DownloadInterruptReason));
+  MOCK_METHOD1(Delete, void(DeleteReason));
+  MOCK_METHOD0(Remove, void());
+  MOCK_CONST_METHOD1(TimeRemaining, bool(base::TimeDelta*));
+  MOCK_CONST_METHOD0(CurrentSpeed, int64());
+  MOCK_CONST_METHOD0(PercentComplete, int());
+  MOCK_CONST_METHOD0(AllDataSaved, bool());
+  MOCK_METHOD0(TogglePause, void());
+  MOCK_METHOD1(OnDownloadCompleting, void(DownloadFileManager*));
+  MOCK_CONST_METHOD1(MatchesQuery, bool(const string16& query));
+  MOCK_CONST_METHOD0(IsPartialDownload, bool());
+  MOCK_CONST_METHOD0(IsInProgress, bool());
+  MOCK_CONST_METHOD0(IsCancelled, bool());
+  MOCK_CONST_METHOD0(IsInterrupted, bool());
+  MOCK_CONST_METHOD0(IsComplete, bool());
+  MOCK_CONST_METHOD0(GetFullPath, const FilePath&());
+  MOCK_CONST_METHOD0(GetTargetFilePath, const FilePath&());
+  MOCK_CONST_METHOD0(GetTargetDisposition, TargetDisposition());
+  MOCK_METHOD3(OnTargetPathDetermined, void(const FilePath&,
+                                            TargetDisposition,
+                                            content::DownloadDangerType));
+  MOCK_METHOD1(OnTargetPathSelected, void(const FilePath&));
+  MOCK_METHOD1(OnContentCheckCompleted, void(content::DownloadDangerType));
+  MOCK_METHOD2(OnIntermediatePathDetermined, void(DownloadFileManager*,
+                                                  const FilePath&));
+  MOCK_CONST_METHOD0(GetState, DownloadState());
+  MOCK_CONST_METHOD0(GetUrlChain, const std::vector<GURL>&());
+  MOCK_METHOD1(SetTotalBytes, void(int64));
+  MOCK_CONST_METHOD0(GetURL, const GURL&());
+  MOCK_CONST_METHOD0(GetOriginalUrl, const GURL&());
+  MOCK_CONST_METHOD0(GetReferrerUrl, const GURL&());
+  MOCK_CONST_METHOD0(GetSuggestedFilename, std::string());
+  MOCK_CONST_METHOD0(GetContentDisposition, std::string());
+  MOCK_CONST_METHOD0(GetMimeType, std::string());
+  MOCK_CONST_METHOD0(GetOriginalMimeType, std::string());
+  MOCK_CONST_METHOD0(GetReferrerCharset, std::string());
+  MOCK_CONST_METHOD0(GetRemoteAddress, std::string());
+  MOCK_CONST_METHOD0(GetTotalBytes, int64());
+  MOCK_CONST_METHOD0(GetReceivedBytes, int64());
+  MOCK_CONST_METHOD0(GetHashState, const std::string&());
+  MOCK_CONST_METHOD0(GetHash, const std::string&());
+  MOCK_CONST_METHOD0(GetId, int32());
+  MOCK_CONST_METHOD0(GetGlobalId, content::DownloadId());
+  MOCK_CONST_METHOD0(GetStartTime, base::Time());
+  MOCK_CONST_METHOD0(GetEndTime, base::Time());
+  MOCK_METHOD0(SetIsPersisted, void());
+  MOCK_CONST_METHOD0(IsPersisted, bool());
+  MOCK_METHOD1(SetDbHandle, void(int64));
+  MOCK_CONST_METHOD0(GetDbHandle, int64());
+  MOCK_METHOD0(GetDownloadManager, content::DownloadManager*());
+  MOCK_CONST_METHOD0(IsPaused, bool());
+  MOCK_CONST_METHOD0(GetOpenWhenComplete, bool());
+  MOCK_METHOD1(SetOpenWhenComplete, void(bool));
+  MOCK_CONST_METHOD0(GetFileExternallyRemoved, bool());
+  MOCK_CONST_METHOD0(GetSafetyState, SafetyState());
+  MOCK_CONST_METHOD0(GetDangerType, content::DownloadDangerType());
+  MOCK_CONST_METHOD0(IsDangerous, bool());
+  MOCK_METHOD0(GetAutoOpened, bool());
+  MOCK_CONST_METHOD0(GetTargetName, FilePath());
+  MOCK_CONST_METHOD0(GetForcedFilePath, const FilePath&());
+  MOCK_CONST_METHOD0(HasUserGesture, bool());
+  MOCK_CONST_METHOD0(GetTransitionType, content::PageTransition());
+  MOCK_CONST_METHOD0(IsOtr, bool());
+  MOCK_CONST_METHOD0(IsTemporary, bool());
+  MOCK_METHOD1(SetIsTemporary, void(bool));
+  MOCK_METHOD1(SetOpened, void(bool));
+  MOCK_CONST_METHOD0(GetOpened, bool());
+  MOCK_CONST_METHOD0(GetLastModifiedTime, const std::string&());
+  MOCK_CONST_METHOD0(GetETag, const std::string&());
+  MOCK_CONST_METHOD0(GetLastReason, DownloadInterruptReason());
+  MOCK_CONST_METHOD0(GetPersistentStoreInfo,
+                     content::DownloadPersistentStoreInfo());
+  MOCK_CONST_METHOD0(GetBrowserContext, content::BrowserContext*());
+  MOCK_CONST_METHOD0(GetWebContents, content::WebContents*());
+  MOCK_CONST_METHOD0(GetFileNameToReportUser, FilePath());
+  MOCK_METHOD1(SetDisplayName, void(const FilePath&));
+  MOCK_CONST_METHOD0(GetUserVerifiedFilePath, FilePath());
+  MOCK_METHOD1(OffThreadCancel, void(DownloadFileManager* file_manager));
+  MOCK_CONST_METHOD1(DebugString, std::string(bool));
+  MOCK_METHOD0(MockDownloadOpenForTesting, void());
+  MOCK_METHOD1(GetExternalData, ExternalData*(const void*));
+  MOCK_CONST_METHOD1(GetExternalData, const ExternalData*(const void*));
+  MOCK_METHOD2(SetExternalData, void(const void*, ExternalData*));
+};
+
 class MockDownloadManagerDelegate : public content::DownloadManagerDelegate {
  public:
   MockDownloadManagerDelegate();
@@ -143,30 +256,30 @@ class MockDownloadItemFactory
   // functionality if it's ever needed by consumers.
 
   // Returns NULL if no item of that id is present.
-  content::MockDownloadItem* GetItem(int id);
+  MockDownloadItemImpl* GetItem(int id);
 
   // Remove and return an item made by the factory.
   // Generally used during teardown.
-  content::MockDownloadItem* PopItem();
+  MockDownloadItemImpl* PopItem();
 
   // Should be called when the item of this id is removed so that
   // we don't keep dangling pointers.
   void RemoveItem(int id);
 
   // Overridden methods from DownloadItemFactory.
-  virtual content::DownloadItem* CreatePersistedItem(
-      DownloadItemImpl::Delegate* delegate,
+  virtual DownloadItemImpl* CreatePersistedItem(
+      DownloadItemImplDelegate* delegate,
       content::DownloadId download_id,
       const content::DownloadPersistentStoreInfo& info,
       const net::BoundNetLog& bound_net_log) OVERRIDE;
-  virtual content::DownloadItem* CreateActiveItem(
-      DownloadItemImpl::Delegate* delegate,
+  virtual DownloadItemImpl* CreateActiveItem(
+      DownloadItemImplDelegate* delegate,
       const DownloadCreateInfo& info,
       scoped_ptr<DownloadRequestHandleInterface> request_handle,
       bool is_otr,
       const net::BoundNetLog& bound_net_log) OVERRIDE;
-  virtual content::DownloadItem* CreateSavePageItem(
-      DownloadItemImpl::Delegate* delegate,
+  virtual DownloadItemImpl* CreateSavePageItem(
+      DownloadItemImplDelegate* delegate,
       const FilePath& path,
       const GURL& url,
       bool is_otr,
@@ -175,7 +288,8 @@ class MockDownloadItemFactory
       const net::BoundNetLog& bound_net_log) OVERRIDE;
 
  private:
-  std::map<int32, content::MockDownloadItem*> items_;
+  std::map<int32, MockDownloadItemImpl*> items_;
+  DownloadItemImplDelegate item_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(MockDownloadItemFactory);
 };
@@ -184,19 +298,19 @@ MockDownloadItemFactory::MockDownloadItemFactory() {}
 
 MockDownloadItemFactory::~MockDownloadItemFactory() {}
 
-content::MockDownloadItem* MockDownloadItemFactory::GetItem(int id) {
+MockDownloadItemImpl* MockDownloadItemFactory::GetItem(int id) {
   if (items_.find(id) == items_.end())
     return NULL;
   return items_[id];
 }
 
-content::MockDownloadItem* MockDownloadItemFactory::PopItem() {
+MockDownloadItemImpl* MockDownloadItemFactory::PopItem() {
   if (items_.empty())
     return NULL;
 
-  std::map<int32, content::MockDownloadItem*>::iterator first_item
+  std::map<int32, MockDownloadItemImpl*>::iterator first_item
       = items_.begin();
-  content::MockDownloadItem* result = first_item->second;
+  MockDownloadItemImpl* result = first_item->second;
   items_.erase(first_item);
   return result;
 }
@@ -206,16 +320,16 @@ void MockDownloadItemFactory::RemoveItem(int id) {
   items_.erase(id);
 }
 
-content::DownloadItem* MockDownloadItemFactory::CreatePersistedItem(
-    DownloadItemImpl::Delegate* delegate,
+DownloadItemImpl* MockDownloadItemFactory::CreatePersistedItem(
+    DownloadItemImplDelegate* 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());
 
-  content::MockDownloadItem* result =
-      new StrictMock<content::MockDownloadItem>;
+  MockDownloadItemImpl* result =
+      new StrictMock<MockDownloadItemImpl>(&item_delegate_);
   EXPECT_CALL(*result, GetId())
       .WillRepeatedly(Return(local_id));
   items_[local_id] = result;
@@ -223,8 +337,8 @@ content::DownloadItem* MockDownloadItemFactory::CreatePersistedItem(
   return result;
 }
 
-content::DownloadItem* MockDownloadItemFactory::CreateActiveItem(
-    DownloadItemImpl::Delegate* delegate,
+DownloadItemImpl* MockDownloadItemFactory::CreateActiveItem(
+    DownloadItemImplDelegate* delegate,
     const DownloadCreateInfo& info,
     scoped_ptr<DownloadRequestHandleInterface> request_handle,
     bool is_otr,
@@ -232,8 +346,8 @@ content::DownloadItem* MockDownloadItemFactory::CreateActiveItem(
   int local_id = info.download_id.local();
   DCHECK(items_.find(local_id) == items_.end());
 
-  content::MockDownloadItem* result =
-      new StrictMock<content::MockDownloadItem>;
+  MockDownloadItemImpl* result =
+      new StrictMock<MockDownloadItemImpl>(&item_delegate_);
   EXPECT_CALL(*result, GetId())
       .WillRepeatedly(Return(local_id));
   items_[local_id] = result;
@@ -241,8 +355,8 @@ content::DownloadItem* MockDownloadItemFactory::CreateActiveItem(
   return result;
 }
 
-content::DownloadItem* MockDownloadItemFactory::CreateSavePageItem(
-    DownloadItemImpl::Delegate* delegate,
+DownloadItemImpl* MockDownloadItemFactory::CreateSavePageItem(
+    DownloadItemImplDelegate* delegate,
     const FilePath& path,
     const GURL& url,
     bool is_otr,
@@ -252,8 +366,8 @@ content::DownloadItem* MockDownloadItemFactory::CreateSavePageItem(
   int local_id = download_id.local();
   DCHECK(items_.find(local_id) == items_.end());
 
-  content::MockDownloadItem* result =
-      new StrictMock<content::MockDownloadItem>;
+  MockDownloadItemImpl* result =
+      new StrictMock<MockDownloadItemImpl>(&item_delegate_);
   EXPECT_CALL(*result, GetId())
       .WillRepeatedly(Return(local_id));
   items_[local_id] = result;
@@ -325,7 +439,7 @@ class DownloadManagerTest : public testing::Test {
   }
 
   virtual void TearDown() {
-    while (content::MockDownloadItem*
+    while (MockDownloadItemImpl*
            item = mock_download_item_factory_->PopItem()) {
       EXPECT_CALL(*item, GetSafetyState())
           .WillOnce(Return(content::DownloadItem::SAFE));
@@ -344,7 +458,7 @@ class DownloadManagerTest : public testing::Test {
   }
 
   // Returns download id.
-  content::MockDownloadItem& AddItemToManager() {
+  MockDownloadItemImpl& AddItemToManager() {
     DownloadCreateInfo info;
 
     static const char* kDownloadIdDomain = "Test download id domain";
@@ -358,15 +472,15 @@ class DownloadManagerTest : public testing::Test {
     download_manager_->CreateDownloadItem(&info);
 
     DCHECK(mock_download_item_factory_->GetItem(id));
-    content::MockDownloadItem& item(*mock_download_item_factory_->GetItem(id));
+    MockDownloadItemImpl& item(*mock_download_item_factory_->GetItem(id));
     ON_CALL(item, GetId())
         .WillByDefault(Return(id));
 
     return item;
   }
 
-  content::MockDownloadItem& GetMockDownloadItem(int id) {
-    content::MockDownloadItem* itemp = mock_download_item_factory_->GetItem(id);
+  MockDownloadItemImpl& GetMockDownloadItem(int id) {
+    MockDownloadItemImpl* itemp = mock_download_item_factory_->GetItem(id);
 
     DCHECK(itemp);
     return *itemp;
@@ -386,11 +500,11 @@ class DownloadManagerTest : public testing::Test {
   }
 
   // Probe at private internals.
-  DownloadItem* GetActiveDownloadItem(int32 id) {
-    return download_manager_->GetActiveDownload(id);
+  void DownloadStopped(DownloadItemImpl* item) {
+    download_manager_->DownloadStopped(item);
   }
 
-  void AddItemToHistory(content::MockDownloadItem& item, int64 db_handle) {
+  void AddItemToHistory(MockDownloadItemImpl& 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())
@@ -442,7 +556,7 @@ TEST_F(DownloadManagerTest, StartDownload) {
   scoped_ptr<content::ByteStreamReader> stream;
   int32 local_id(5);                    // Random value
 
-  EXPECT_FALSE(GetActiveDownloadItem(local_id));
+  EXPECT_FALSE(download_manager_->GetActiveDownloadItem(local_id));
 
   EXPECT_CALL(GetMockDownloadManagerDelegate(), GetNextId())
       .WillOnce(Return(content::DownloadId(this, local_id)));
@@ -453,17 +567,17 @@ TEST_F(DownloadManagerTest, StartDownload) {
       download_manager_.get(), true, _, _));
 
   download_manager_->StartDownload(info.Pass(), stream.Pass());
-  EXPECT_TRUE(GetActiveDownloadItem(local_id));
+  EXPECT_TRUE(download_manager_->GetActiveDownloadItem(local_id));
 }
 
 // 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());
+  MockDownloadItemImpl& item(AddItemToManager());
   int download_id = item.GetId();
 
   // Confirm we're internally consistent.
-  EXPECT_EQ(&item, GetActiveDownloadItem(download_id));
+  EXPECT_EQ(&item, download_manager_->GetActiveDownloadItem(download_id));
 
   ScopedTempDir download_dir;
   ASSERT_TRUE(download_dir.CreateUniqueTempDir());
@@ -484,7 +598,7 @@ TEST_F(DownloadManagerTest, 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());
+  MockDownloadItemImpl& item(AddItemToManager());
 
   ScopedTempDir download_dir;
   ASSERT_TRUE(download_dir.CreateUniqueTempDir());
@@ -510,7 +624,7 @@ TEST_F(DownloadManagerTest, OnTargetPathAvailable) {
 // to the DownloadItem?
 TEST_F(DownloadManagerTest, OnDownloadInterrupted) {
   // Put a mock we have a handle to on the download manager.
-  content::MockDownloadItem& item(AddItemToManager());
+  MockDownloadItemImpl& item(AddItemToManager());
   int download_id = item.GetId();
 
   int64 size = 0xdeadbeef;
@@ -522,14 +636,14 @@ TEST_F(DownloadManagerTest, OnDownloadInterrupted) {
   EXPECT_CALL(item, Interrupt(reason));
   download_manager_->OnDownloadInterrupted(
       download_id, size, hash_state, reason);
-  EXPECT_EQ(&item, GetActiveDownloadItem(download_id));
+  EXPECT_EQ(&item, download_manager_->GetActiveDownloadItem(download_id));
 }
 
 // Does DownloadStopped remove Download from appropriate queues?
 // This test tests non-persisted downloads.
 TEST_F(DownloadManagerTest, OnDownloadStopped_NonPersisted) {
   // Put a mock we have a handle to on the download manager.
-  content::MockDownloadItem& item(AddItemToManager());
+  MockDownloadItemImpl& item(AddItemToManager());
 
   EXPECT_CALL(item, IsPersisted())
       .WillRepeatedly(Return(false));
@@ -539,18 +653,18 @@ TEST_F(DownloadManagerTest, OnDownloadStopped_NonPersisted) {
       .WillRepeatedly(Return(DownloadItem::kUninitializedHandle));
 
   EXPECT_CALL(item, OffThreadCancel(&GetMockDownloadFileManager()));
-  download_manager_->DownloadStopped(&item);
+  DownloadStopped(&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 :-{.
+  // active list by calling download_manager_->GetActiveDownloadItem(id).
+  // Currently, the item is left on the active list for rendez-vous with
+  // the history system :-{.
 }
 
 // Does DownloadStopped remove Download from appropriate queues?
 // This test tests persisted downloads.
 TEST_F(DownloadManagerTest, OnDownloadStopped_Persisted) {
   // Put a mock we have a handle to on the download manager.
-  content::MockDownloadItem& item(AddItemToManager());
+  MockDownloadItemImpl& item(AddItemToManager());
   int download_id = item.GetId();
   int64 db_handle = 0x7;
   EXPECT_CALL(item, GetExternalData(_))
@@ -567,6 +681,6 @@ TEST_F(DownloadManagerTest, OnDownloadStopped_Persisted) {
       .WillRepeatedly(Return(db_handle));
 
   EXPECT_CALL(item, OffThreadCancel(&GetMockDownloadFileManager()));
-  download_manager_->DownloadStopped(&item);
-  EXPECT_EQ(NULL, GetActiveDownloadItem(download_id));
+  DownloadStopped(&item);
+  EXPECT_EQ(NULL, download_manager_->GetActiveDownloadItem(download_id));
 }
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index d138602..675529b 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -20,6 +20,7 @@
 #include "base/utf_string_conversions.h"
 #include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_item_impl.h"
+#include "content/browser/download/download_manager_impl.h"
 #include "content/browser/download/download_stats.h"
 #include "content/browser/download/save_file.h"
 #include "content/browser/download/save_file_manager.h"
@@ -32,7 +33,6 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/download_manager.h"
 #include "content/public/browser/download_manager_delegate.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/resource_context.h"
@@ -264,8 +264,9 @@ void SavePackage::InternalInit() {
   file_manager_ = rdh->save_file_manager();
   DCHECK(file_manager_);
 
-  download_manager_ = BrowserContext::GetDownloadManager(
-      web_contents()->GetBrowserContext());
+  download_manager_ = static_cast<DownloadManagerImpl*>(
+      BrowserContext::GetDownloadManager(
+          web_contents()->GetBrowserContext()));
   DCHECK(download_manager_);
 
   download_stats::RecordSavePackageEvent(download_stats::SAVE_PACKAGE_STARTED);
diff --git a/content/browser/download/save_package.h b/content/browser/download/save_package.h
index f8c8905..7f2bebb 100644
--- a/content/browser/download/save_package.h
+++ b/content/browser/download/save_package.h
@@ -24,6 +24,8 @@
 #include "content/public/common/referrer.h"
 #include "googleurl/src/gurl.h"
 
+class DownloadItemImpl;
+class DownloadManagerImpl;
 class GURL;
 class SaveFileManager;
 class SaveItem;
@@ -31,7 +33,6 @@ class SavePackage;
 struct SaveFileCreateInfo;
 
 namespace content {
-class DownloadManager;
 class WebContents;
 }
 
@@ -264,8 +265,8 @@ class CONTENT_EXPORT SavePackage
   SaveFileManager* file_manager_;
 
   // DownloadManager owns the DownloadItem and handles history and UI.
-  content::DownloadManager* download_manager_;
-  content::DownloadItem* download_;
+  DownloadManagerImpl* download_manager_;
+  DownloadItemImpl* download_;
 
   // The URL of the page the user wants to save.
   GURL page_url_;