diff options
Diffstat (limited to 'content/browser/download')
| -rw-r--r-- | content/browser/download/download_item_factory.h | 18 | ||||
| -rw-r--r-- | content/browser/download/download_item_impl.cc | 25 | ||||
| -rw-r--r-- | content/browser/download/download_item_impl.h | 125 | ||||
| -rw-r--r-- | content/browser/download/download_item_impl_delegate.cc | 62 | ||||
| -rw-r--r-- | content/browser/download/download_item_impl_delegate.h | 70 | ||||
| -rw-r--r-- | content/browser/download/download_item_impl_unittest.cc | 78 | ||||
| -rw-r--r-- | content/browser/download/download_manager_impl.cc | 146 | ||||
| -rw-r--r-- | content/browser/download/download_manager_impl.h | 90 | ||||
| -rw-r--r-- | content/browser/download/download_manager_impl_unittest.cc | 210 | ||||
| -rw-r--r-- | content/browser/download/save_package.cc | 7 | ||||
| -rw-r--r-- | content/browser/download/save_package.h | 7 |
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_; |