diff options
21 files changed, 57 insertions, 105 deletions
diff --git a/android_webview/browser/aw_content_browser_client.cc b/android_webview/browser/aw_content_browser_client.cc index 58942e2..5a53dd1 100644 --- a/android_webview/browser/aw_content_browser_client.cc +++ b/android_webview/browser/aw_content_browser_client.cc @@ -196,14 +196,6 @@ AwContentBrowserClient::CreateQuotaPermissionContext() { return new AwQuotaPermissionContext; } -void AwContentBrowserClient::OpenItem(const FilePath& path) { - NOTREACHED() << "Android WebView does not use chromium downloads"; -} - -void AwContentBrowserClient::ShowItemInFolder(const FilePath& path) { - NOTREACHED() << "Android WebView does not use chromium downloads"; -} - void AwContentBrowserClient::AllowCertificateError( int render_process_id, int render_view_id, diff --git a/android_webview/browser/aw_content_browser_client.h b/android_webview/browser/aw_content_browser_client.h index 8d00a43..41bed68 100644 --- a/android_webview/browser/aw_content_browser_client.h +++ b/android_webview/browser/aw_content_browser_client.h @@ -72,8 +72,6 @@ class AwContentBrowserClient : public content::ContentBrowserClient { const std::vector<std::pair<int, int> >& render_views) OVERRIDE; virtual content::QuotaPermissionContext* CreateQuotaPermissionContext() OVERRIDE; - virtual void OpenItem(const FilePath& path) OVERRIDE; - virtual void ShowItemInFolder(const FilePath& path) OVERRIDE; virtual void AllowCertificateError( int render_process_id, int render_view_id, diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index a430cc9..d27c0b2 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -1193,14 +1193,6 @@ ChromeContentBrowserClient::CreateQuotaPermissionContext() { return new ChromeQuotaPermissionContext(); } -void ChromeContentBrowserClient::OpenItem(const FilePath& path) { - platform_util::OpenItem(path); -} - -void ChromeContentBrowserClient::ShowItemInFolder(const FilePath& path) { - platform_util::ShowItemInFolder(path); -} - void ChromeContentBrowserClient::AllowCertificateError( int render_process_id, int render_view_id, diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h index 136ddab..129a5d6 100644 --- a/chrome/browser/chrome_content_browser_client.h +++ b/chrome/browser/chrome_content_browser_client.h @@ -126,8 +126,6 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient { const GURL& url, content::ResourceContext* context) OVERRIDE; virtual content::QuotaPermissionContext* CreateQuotaPermissionContext() OVERRIDE; - virtual void OpenItem(const FilePath& path) OVERRIDE; - virtual void ShowItemInFolder(const FilePath& path) OVERRIDE; virtual void AllowCertificateError( int render_process_id, int render_view_id, diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc index 1c9dd5f..90770c2 100644 --- a/chrome/browser/download/chrome_download_manager_delegate.cc +++ b/chrome/browser/download/chrome_download_manager_delegate.cc @@ -33,6 +33,7 @@ #include "chrome/browser/history/history.h" #include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/intents/web_intents_util.h" +#include "chrome/browser/platform_util.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/safe_browsing/safe_browsing_service.h" @@ -523,6 +524,15 @@ void ChromeDownloadManagerDelegate::ChooseSavePath( #endif } +void ChromeDownloadManagerDelegate::OpenDownload(DownloadItem* download) { + platform_util::OpenItem(download->GetFullPath()); +} + +void ChromeDownloadManagerDelegate::ShowDownloadInShell( + DownloadItem* download) { + platform_util::ShowItemInFolder(download->GetFullPath()); +} + void ChromeDownloadManagerDelegate::ClearLastDownloadPath() { last_download_path_.clear(); } diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h index 9224165..c10cb1d 100644 --- a/chrome/browser/download/chrome_download_manager_delegate.h +++ b/chrome/browser/download/chrome_download_manager_delegate.h @@ -86,6 +86,8 @@ class ChromeDownloadManagerDelegate const FilePath::StringType& default_extension, bool can_save_as_complete, const content::SavePackagePathPickedCallback& callback) OVERRIDE; + virtual void OpenDownload(content::DownloadItem* download) OVERRIDE; + virtual void ShowDownloadInShell(content::DownloadItem* download) OVERRIDE; // Clears the last directory chosen by the user in response to a file chooser // prompt. Called when clearing recent history. diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc index 66830ad..c2a7678 100644 --- a/chrome/browser/download/download_browsertest.cc +++ b/chrome/browser/download/download_browsertest.cc @@ -167,30 +167,6 @@ void SetHiddenDownloadCallback(DownloadItem* item, net::Error error) { } // namespace -// While an object of this class exists, it will mock out download -// opening for all downloads created on the specified download manager. -class MockDownloadOpeningObserver : public DownloadManager::Observer { - public: - explicit MockDownloadOpeningObserver(DownloadManager* manager) - : download_manager_(manager) { - download_manager_->AddObserver(this); - } - - ~MockDownloadOpeningObserver() { - download_manager_->RemoveObserver(this); - } - - virtual void OnDownloadCreated( - DownloadManager* manager, DownloadItem* item) OVERRIDE { - item->MockDownloadOpenForTesting(); - } - - private: - DownloadManager* download_manager_; - - DISALLOW_COPY_AND_ASSIGN(MockDownloadOpeningObserver); -}; - class HistoryObserver : public DownloadHistory::Observer { public: explicit HistoryObserver(Profile* profile) @@ -1487,8 +1463,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, AutoOpen) { ASSERT_TRUE( GetDownloadPrefs(browser())->EnableAutoOpenBasedOnExtension(file)); - // Mock out external opening on all downloads until end of test. - MockDownloadOpeningObserver observer(DownloadManagerForBrowser(browser())); + DownloadManagerForBrowser(browser())->MockDownloadOpenForTesting(); DownloadAndWait(browser(), url); diff --git a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc index 451bb6a..2c818f2 100644 --- a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc +++ b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc @@ -813,30 +813,6 @@ const char HTML5FileWriter::kHTML5FileWritten[] = "html5_file_written"; const char HTML5FileWriter::kURLRequestContextToreDown[] = "url_request_context_tore_down"; -// While an object of this class exists, it will mock out download -// opening for all downloads created on the specified download manager. -class MockDownloadOpeningObserver : public DownloadManager::Observer { - public: - explicit MockDownloadOpeningObserver(DownloadManager* manager) - : download_manager_(manager) { - download_manager_->AddObserver(this); - } - - ~MockDownloadOpeningObserver() { - download_manager_->RemoveObserver(this); - } - - virtual void OnDownloadCreated( - DownloadManager* manager, DownloadItem* item) OVERRIDE { - item->MockDownloadOpenForTesting(); - } - - private: - DownloadManager* download_manager_; - - DISALLOW_COPY_AND_ASSIGN(MockDownloadOpeningObserver); -}; - } // namespace IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, @@ -846,7 +822,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest, new DownloadsOpenFunction(), "[-42]").c_str()); - MockDownloadOpeningObserver mock_open_observer(GetCurrentManager()); + GetCurrentManager()->MockDownloadOpenForTesting(); DownloadItem* download_item = CreateSlowTestDownload(); ASSERT_TRUE(download_item); EXPECT_FALSE(download_item->GetOpened()); diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc index 6a6acba..5f87609 100644 --- a/content/browser/download/download_item_impl.cc +++ b/content/browser/download/download_item_impl.cc @@ -149,7 +149,6 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate, is_temporary_(false), all_data_saved_(false), opened_(opened), - open_enabled_(true), delegate_delayed_complete_(false), bound_net_log_(bound_net_log), ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) { @@ -198,7 +197,6 @@ DownloadItemImpl::DownloadItemImpl( is_temporary_(!info.save_info->file_path.empty()), all_data_saved_(false), opened_(false), - open_enabled_(true), delegate_delayed_complete_(false), bound_net_log_(bound_net_log), ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) { @@ -251,7 +249,6 @@ DownloadItemImpl::DownloadItemImpl(DownloadItemImplDelegate* delegate, is_temporary_(false), all_data_saved_(false), opened_(false), - open_enabled_(true), delegate_delayed_complete_(false), bound_net_log_(bound_net_log), ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) { @@ -437,20 +434,13 @@ void DownloadItemImpl::OpenDownload() { RecordOpen(GetEndTime(), !GetOpened()); opened_ = true; FOR_EACH_OBSERVER(Observer, observers_, OnDownloadOpened(this)); - delegate_->DownloadOpened(this); - - // For testing: If download opening is disabled on this item, - // make the rest of the routine a no-op. - if (!open_enabled_) - return; - - GetContentClient()->browser()->OpenItem(GetFullPath()); + delegate_->OpenDownload(this); } void DownloadItemImpl::ShowDownloadInShell() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - GetContentClient()->browser()->ShowItemInFolder(GetFullPath()); + delegate_->ShowDownloadInShell(this); } int32 DownloadItemImpl::GetId() const { @@ -797,10 +787,6 @@ std::string DownloadItemImpl::DebugString(bool verbose) const { return description; } -void DownloadItemImpl::MockDownloadOpenForTesting() { - open_enabled_ = false; -} - DownloadItemImpl::ResumeMode DownloadItemImpl::GetResumeMode() const { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); if (!IsInterrupted()) diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h index b152074..e320096 100644 --- a/content/browser/download/download_item_impl.h +++ b/content/browser/download/download_item_impl.h @@ -152,7 +152,6 @@ class CONTENT_EXPORT DownloadItemImpl virtual void SetOpened(bool opened) OVERRIDE; virtual void SetDisplayName(const FilePath& name) OVERRIDE; virtual std::string DebugString(bool verbose) const OVERRIDE; - virtual void MockDownloadOpenForTesting() OVERRIDE; // All remaining public interfaces virtual to allow for DownloadItemImpl // mocks. @@ -466,9 +465,6 @@ class CONTENT_EXPORT DownloadItemImpl // be treated as though the user opened it. bool opened_; - // Do we actually open downloads when requested? For testing purposes only. - bool open_enabled_; - // Did the delegate delay calling Complete on this download? bool delegate_delayed_complete_; diff --git a/content/browser/download/download_item_impl_delegate.cc b/content/browser/download/download_item_impl_delegate.cc index 932bcb0..02175ca 100644 --- a/content/browser/download/download_item_impl_delegate.cc +++ b/content/browser/download/download_item_impl_delegate.cc @@ -65,7 +65,10 @@ BrowserContext* DownloadItemImplDelegate::GetBrowserContext() const { void DownloadItemImplDelegate::UpdatePersistence(DownloadItemImpl* download) {} -void DownloadItemImplDelegate::DownloadOpened(DownloadItemImpl* download) {} +void DownloadItemImplDelegate::OpenDownload(DownloadItemImpl* download) {} + +void DownloadItemImplDelegate::ShowDownloadInShell(DownloadItemImpl* download) { +} void DownloadItemImplDelegate::DownloadRemoved(DownloadItemImpl* download) {} diff --git a/content/browser/download/download_item_impl_delegate.h b/content/browser/download/download_item_impl_delegate.h index a64c2e6..b6eebac 100644 --- a/content/browser/download/download_item_impl_delegate.h +++ b/content/browser/download/download_item_impl_delegate.h @@ -77,9 +77,14 @@ class CONTENT_EXPORT DownloadItemImplDelegate { // Update the persistent store with our information. virtual void UpdatePersistence(DownloadItemImpl* download); + // Opens the file associated with this download. + virtual void OpenDownload(DownloadItemImpl* download); + + // Shows the download via the OS shell. + virtual void ShowDownloadInShell(DownloadItemImpl* download); + // Handle any delegate portions of a state change operation on the // DownloadItem. - virtual void DownloadOpened(DownloadItemImpl* download); virtual void DownloadRemoved(DownloadItemImpl* download); // Show the download in the browser. diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc index 4b40c27..35b43cf 100644 --- a/content/browser/download/download_manager_impl.cc +++ b/content/browser/download/download_manager_impl.cc @@ -223,7 +223,8 @@ DownloadManagerImpl::DownloadManagerImpl( shutdown_needed_(false), browser_context_(NULL), delegate_(NULL), - net_log_(net_log) { + net_log_(net_log), + open_enabled_(true) { } DownloadManagerImpl::~DownloadManagerImpl() { @@ -659,7 +660,11 @@ void DownloadManagerImpl::GetAllDownloads(DownloadVector* downloads) { } } -void DownloadManagerImpl::DownloadOpened(DownloadItemImpl* download) { +void DownloadManagerImpl::MockDownloadOpenForTesting() { + open_enabled_ = false; +} + +void DownloadManagerImpl::OpenDownload(DownloadItemImpl* download) { int num_unopened = 0; for (DownloadMap::iterator it = downloads_.begin(); it != downloads_.end(); ++it) { @@ -669,6 +674,14 @@ void DownloadManagerImpl::DownloadOpened(DownloadItemImpl* download) { ++num_unopened; } RecordOpensOutstanding(num_unopened); + + if (delegate_ && open_enabled_) // Some tests disable OpenDownload(). + delegate_->OpenDownload(download); +} + +void DownloadManagerImpl::ShowDownloadInShell(DownloadItemImpl* download) { + if (delegate_) + delegate_->ShowDownloadInShell(download); } } // namespace content diff --git a/content/browser/download/download_manager_impl.h b/content/browser/download/download_manager_impl.h index d5a3c9d..de7ad5d 100644 --- a/content/browser/download/download_manager_impl.h +++ b/content/browser/download/download_manager_impl.h @@ -78,6 +78,7 @@ class CONTENT_EXPORT DownloadManagerImpl : public DownloadManager, virtual BrowserContext* GetBrowserContext() const OVERRIDE; virtual void CheckForHistoryFilesRemoval() OVERRIDE; virtual DownloadItem* GetDownload(int id) OVERRIDE; + virtual void MockDownloadOpenForTesting() OVERRIDE; // For testing; specifically, accessed from TestFileErrorInjector. void SetDownloadItemFactoryForTesting( @@ -133,7 +134,8 @@ class CONTENT_EXPORT DownloadManagerImpl : public DownloadManager, virtual void ResumeInterruptedDownload( scoped_ptr<content::DownloadUrlParameters> params, content::DownloadId id) OVERRIDE; - virtual void DownloadOpened(DownloadItemImpl* download) OVERRIDE; + virtual void OpenDownload(DownloadItemImpl* download) OVERRIDE; + virtual void ShowDownloadInShell(DownloadItemImpl* download) OVERRIDE; virtual void DownloadRemoved(DownloadItemImpl* download) OVERRIDE; virtual void ShowDownloadInBrowser(DownloadItemImpl* download) OVERRIDE; @@ -165,6 +167,9 @@ class CONTENT_EXPORT DownloadManagerImpl : public DownloadManager, net::NetLog* net_log_; + // Do we actually open downloads when requested? For testing purposes only. + bool open_enabled_; + DISALLOW_COPY_AND_ASSIGN(DownloadManagerImpl); }; diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc index b1c5ac6..754fdc1 100644 --- a/content/browser/download/download_manager_impl_unittest.cc +++ b/content/browser/download/download_manager_impl_unittest.cc @@ -164,7 +164,6 @@ class MockDownloadItemImpl : public DownloadItemImpl { MOCK_CONST_METHOD0(GetFileNameToReportUser, FilePath()); MOCK_METHOD1(SetDisplayName, void(const FilePath&)); MOCK_CONST_METHOD0(GetUserVerifiedFilePath, FilePath()); - MOCK_METHOD0(MockDownloadOpenForTesting, void()); // May be called when vlog is on. virtual std::string DebugString(bool verbose) const OVERRIDE { return ""; } }; diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h index 68ca0ae..2e3b3c7 100644 --- a/content/public/browser/content_browser_client.h +++ b/content/public/browser/content_browser_client.h @@ -287,12 +287,6 @@ class CONTENT_EXPORT ContentBrowserClient { // Create and return a new quota permission context. virtual QuotaPermissionContext* CreateQuotaPermissionContext(); - // Open the given file in the desktop's default manner. - virtual void OpenItem(const FilePath& path) {} - - // Show the given file in a file manager. If possible, select the file. - virtual void ShowItemInFolder(const FilePath& path) {} - // Informs the embedder that a certificate error has occured. If // |overridable| is true and if |strict_enforcement| is false, the user // can ignore the error and continue. The embedder can call the callback diff --git a/content/public/browser/download_item.h b/content/public/browser/download_item.h index 5d0db88..2d4c5a8 100644 --- a/content/public/browser/download_item.h +++ b/content/public/browser/download_item.h @@ -318,7 +318,6 @@ class CONTENT_EXPORT DownloadItem : public base::SupportsUserData { // Debug/testing ------------------------------------------------------------- virtual std::string DebugString(bool verbose) const = 0; - virtual void MockDownloadOpenForTesting() = 0; }; } // namespace content diff --git a/content/public/browser/download_manager.h b/content/public/browser/download_manager.h index fa98dd9..14dfac4 100644 --- a/content/public/browser/download_manager.h +++ b/content/public/browser/download_manager.h @@ -166,6 +166,9 @@ class CONTENT_EXPORT DownloadManager // it is or state it's in. virtual DownloadItem* GetDownload(int id) = 0; + // Prohibits OpenDownload() from actually opening downloads for testing. + virtual void MockDownloadOpenForTesting() = 0; + protected: virtual ~DownloadManager() {} diff --git a/content/public/browser/download_manager_delegate.h b/content/public/browser/download_manager_delegate.h index 5333938..add75c4 100644 --- a/content/public/browser/download_manager_delegate.h +++ b/content/public/browser/download_manager_delegate.h @@ -117,6 +117,12 @@ class CONTENT_EXPORT DownloadManagerDelegate { const SavePackagePathPickedCallback& callback) { } + // Opens the file associated with this download. + virtual void OpenDownload(DownloadItem* download) {} + + // Shows the download via the OS shell. + virtual void ShowDownloadInShell(DownloadItem* download) {} + protected: virtual ~DownloadManagerDelegate(); }; diff --git a/content/public/test/mock_download_item.h b/content/public/test/mock_download_item.h index 5a6688c..08f4dc3 100644 --- a/content/public/test/mock_download_item.h +++ b/content/public/test/mock_download_item.h @@ -90,7 +90,6 @@ class MockDownloadItem : public DownloadItem { MOCK_METHOD1(SetOpened, void(bool)); MOCK_METHOD1(SetDisplayName, void(const FilePath&)); MOCK_CONST_METHOD1(DebugString, std::string(bool)); - MOCK_METHOD0(MockDownloadOpenForTesting, void()); }; } // namespace content diff --git a/content/public/test/mock_download_manager.h b/content/public/test/mock_download_manager.h index 82c2858..ef47a3f 100644 --- a/content/public/test/mock_download_manager.h +++ b/content/public/test/mock_download_manager.h @@ -68,6 +68,7 @@ class MockDownloadManager : public DownloadManager { MOCK_METHOD1(SavePageDownloadFinished, void(DownloadItem* download)); MOCK_METHOD1(GetActiveDownloadItem, DownloadItem*(int id)); MOCK_METHOD1(GetActiveDownload, DownloadItem*(int32 download_id)); + MOCK_METHOD0(MockDownloadOpenForTesting, void()); protected: virtual ~MockDownloadManager(); |