diff options
author | sail@chromium.org <sail@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-17 20:05:48 +0000 |
---|---|---|
committer | sail@chromium.org <sail@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-17 20:05:48 +0000 |
commit | b6a4ac2bccf88f27be2c2017dddeddf2aac1ff3b (patch) | |
tree | 774e9be2f55920890b1322ad40c2ad9dc1b480f4 /chrome/browser | |
parent | fd31c2215a5e81b62653e989ba811185d00a0694 (diff) | |
download | chromium_src-b6a4ac2bccf88f27be2c2017dddeddf2aac1ff3b.zip chromium_src-b6a4ac2bccf88f27be2c2017dddeddf2aac1ff3b.tar.gz chromium_src-b6a4ac2bccf88f27be2c2017dddeddf2aac1ff3b.tar.bz2 |
Revert "Fix warning prompting on closing a window that will cancel downloads."
This reverts commit 340c91ba33fcdb81885b353635c16b9ff91d1f37.
Revert "Disable BrowserCloseTest.DownloadsCloseCheck_3. It fails on ASAN Tests (2) and Linux x64."
This reverts commit 06b7c55aadc63d0d23cd4248203d6f9c80895210.
Revert "touchui: Fix browser_tests compile."
This reverts commit dc0f37f22f819d92c36534199922b606891c1c4a.
Revert "Disable BrowserCloseTest.DownloadsCloseCheck_1 on Mac."
This reverts commit 353f4150ab6ec68a7c9d9e9d9afecbf2e110de52.
BUG=100566
TEST=
TBR=rdsmith
Review URL: http://codereview.chromium.org/8316016
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@105887 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r-- | chrome/browser/download/download_browsertest.cc | 448 | ||||
-rw-r--r-- | chrome/browser/download/download_service.cc | 23 | ||||
-rw-r--r-- | chrome/browser/download/download_service.h | 6 | ||||
-rw-r--r-- | chrome/browser/download/download_test_observer.cc | 297 | ||||
-rw-r--r-- | chrome/browser/download/download_test_observer.h | 181 | ||||
-rw-r--r-- | chrome/browser/ui/browser.cc | 162 | ||||
-rw-r--r-- | chrome/browser/ui/browser.h | 27 | ||||
-rw-r--r-- | chrome/browser/ui/browser_close_browsertest.cc | 546 | ||||
-rw-r--r-- | chrome/browser/ui/browser_list.cc | 21 | ||||
-rw-r--r-- | chrome/browser/ui/gtk/download/download_in_progress_dialog_gtk.cc | 15 | ||||
-rw-r--r-- | chrome/browser/ui/views/download/download_in_progress_dialog_view.cc | 15 |
11 files changed, 513 insertions, 1228 deletions
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc index 4d86222..054feae 100644 --- a/chrome/browser/download/download_browsertest.cc +++ b/chrome/browser/download/download_browsertest.cc @@ -20,7 +20,6 @@ #include "chrome/browser/download/download_service.h" #include "chrome/browser/download/download_service_factory.h" #include "chrome/browser/download/download_shelf.h" -#include "chrome/browser/download/download_test_observer.h" #include "chrome/browser/download/download_util.h" #include "chrome/browser/extensions/extension_install_ui.h" #include "chrome/browser/extensions/extension_service.h" @@ -83,6 +82,359 @@ void DenyDangerousDownload(scoped_refptr<DownloadManager> download_manager, download->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD); } +// Construction of this class defines a system state, based on some number +// of downloads being seen in a particular state + other events that +// may occur in the download system. That state will be recorded if it +// occurs at any point after construction. When that state occurs, the class +// is considered finished. Callers may either probe for the finished state, or +// wait on it. +// +// TODO(rdsmith): Detect manager going down, remove pointer to +// DownloadManager, transition to finished. (For right now we +// just use a scoped_refptr<> to keep it around, but that may cause +// timeouts on waiting if a DownloadManager::Shutdown() occurs which +// cancels our in-progress downloads.) +class DownloadsObserver : public DownloadManager::Observer, + public DownloadItem::Observer { + public: + // Create an object that will be considered finished when |wait_count| + // download items have entered state |download_finished_state|. + // If |finish_on_select_file| is true, the object will also be + // considered finished if the DownloadManager raises a + // SelectFileDialogDisplayed() notification. + + // TODO(rdsmith): Consider rewriting the interface to take a list of events + // to treat as completion events. + DownloadsObserver(DownloadManager* download_manager, + size_t wait_count, + DownloadItem::DownloadState download_finished_state, + bool finish_on_select_file, + DangerousDownloadAction dangerous_download_action) + : download_manager_(download_manager), + wait_count_(wait_count), + finished_downloads_at_construction_(0), + waiting_(false), + download_finished_state_(download_finished_state), + finish_on_select_file_(finish_on_select_file), + select_file_dialog_seen_(false), + dangerous_download_action_(dangerous_download_action) { + download_manager_->AddObserver(this); // Will call initial ModelChanged(). + finished_downloads_at_construction_ = finished_downloads_.size(); + EXPECT_NE(DownloadItem::REMOVING, download_finished_state) + << "Waiting for REMOVING is not supported. Try COMPLETE."; + } + + ~DownloadsObserver() { + std::set<DownloadItem*>::iterator it = downloads_observed_.begin(); + for (; it != downloads_observed_.end(); ++it) + (*it)->RemoveObserver(this); + + download_manager_->RemoveObserver(this); + } + + // State accessors. + bool select_file_dialog_seen() { return select_file_dialog_seen_; } + + // Wait for whatever state was specified in the constructor. + void WaitForFinished() { + if (!IsFinished()) { + waiting_ = true; + ui_test_utils::RunMessageLoop(); + waiting_ = false; + } + } + + // Return true if everything's happened that we're configured for. + bool IsFinished() { + if (finished_downloads_.size() - finished_downloads_at_construction_ + >= wait_count_) + return true; + return (finish_on_select_file_ && select_file_dialog_seen_); + } + + // DownloadItem::Observer + virtual void OnDownloadUpdated(DownloadItem* download) { + // The REMOVING state indicates that the download is being destroyed. + // Stop observing. Do not do anything with it, as it is about to be gone. + if (download->state() == DownloadItem::REMOVING) { + std::set<DownloadItem*>::iterator it = downloads_observed_.find(download); + ASSERT_TRUE(it != downloads_observed_.end()); + downloads_observed_.erase(it); + download->RemoveObserver(this); + return; + } + + // Real UI code gets the user's response after returning from the observer. + if (download->safety_state() == DownloadItem::DANGEROUS && + !ContainsKey(dangerous_downloads_seen_, download->id())) { + dangerous_downloads_seen_.insert(download->id()); + + // Calling DangerousDownloadValidated() at this point will + // cause the download to be completed twice. Do what the real UI + // code does: make the call as a delayed task. + switch (dangerous_download_action_) { + case ON_DANGEROUS_DOWNLOAD_ACCEPT: + // Fake user click on "Accept". Delay the actual click, as the + // real UI would. + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&AcceptDangerousDownload, download_manager_, + download->id())); + break; + + case ON_DANGEROUS_DOWNLOAD_DENY: + // Fake a user click on "Deny". Delay the actual click, as the + // real UI would. + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&DenyDangerousDownload, download_manager_, + download->id())); + break; + + case ON_DANGEROUS_DOWNLOAD_FAIL: + ADD_FAILURE() << "Unexpected dangerous download item."; + break; + + default: + NOTREACHED(); + } + } + + if (download->state() == download_finished_state_) + DownloadInFinalState(download); + } + + virtual void OnDownloadOpened(DownloadItem* download) {} + + // DownloadManager::Observer + virtual void ModelChanged() { + // Regenerate DownloadItem observers. If there are any download items + // in our final state, note them in |finished_downloads_| + // (done by |OnDownloadUpdated()|). + std::vector<DownloadItem*> downloads; + download_manager_->GetAllDownloads(FilePath(), &downloads); + + std::vector<DownloadItem*>::iterator it = downloads.begin(); + for (; it != downloads.end(); ++it) { + OnDownloadUpdated(*it); // Safe to call multiple times; checks state. + + std::set<DownloadItem*>::const_iterator + finished_it(finished_downloads_.find(*it)); + std::set<DownloadItem*>::iterator + observed_it(downloads_observed_.find(*it)); + + // If it isn't finished and we're aren't observing it, start. + if (finished_it == finished_downloads_.end() && + observed_it == downloads_observed_.end()) { + (*it)->AddObserver(this); + downloads_observed_.insert(*it); + continue; + } + + // If it is finished and we are observing it, stop. + if (finished_it != finished_downloads_.end() && + observed_it != downloads_observed_.end()) { + (*it)->RemoveObserver(this); + downloads_observed_.erase(observed_it); + continue; + } + } + } + + virtual void SelectFileDialogDisplayed(int32 /* id */) { + select_file_dialog_seen_ = true; + SignalIfFinished(); + } + + virtual size_t NumDangerousDownloadsSeen() const { + return dangerous_downloads_seen_.size(); + } + + private: + // Called when we know that a download item is in a final state. + // Note that this is not the same as it first transitioning in to the + // final state; multiple notifications may occur once the item is in + // that state. So we keep our own track of transitions into final. + void DownloadInFinalState(DownloadItem* download) { + if (finished_downloads_.find(download) != finished_downloads_.end()) { + // We've already seen terminal state on this download. + return; + } + + // Record the transition. + finished_downloads_.insert(download); + + SignalIfFinished(); + } + + void SignalIfFinished() { + if (waiting_ && IsFinished()) + MessageLoopForUI::current()->Quit(); + } + + // The observed download manager. + scoped_refptr<DownloadManager> download_manager_; + + // The set of DownloadItem's that have transitioned to their finished state + // since construction of this object. When the size of this array + // reaches wait_count_, we're done. + std::set<DownloadItem*> finished_downloads_; + + // The set of DownloadItem's we are currently observing. Generally there + // won't be any overlap with the above; once we see the final state + // on a DownloadItem, we'll stop observing it. + std::set<DownloadItem*> downloads_observed_; + + // The number of downloads to wait on completing. + size_t wait_count_; + + // The number of downloads entered in final state in initial + // ModelChanged(). We use |finished_downloads_| to track the incoming + // transitions to final state we should ignore, and to track the + // number of final state transitions that occurred between + // construction and return from wait. But some downloads may be in our + // final state (and thus be entered into |finished_downloads_|) when we + // construct this class. We don't want to count those in our transition + // to finished. + int finished_downloads_at_construction_; + + // Whether an internal message loop has been started and must be quit upon + // all downloads completing. + bool waiting_; + + // The state on which to consider the DownloadItem finished. + DownloadItem::DownloadState download_finished_state_; + + // True if we should transition the DownloadsObserver to finished if + // the select file dialog comes up. + bool finish_on_select_file_; + + // True if we've seen the select file dialog. + bool select_file_dialog_seen_; + + // Action to take if a dangerous download is encountered. + DangerousDownloadAction dangerous_download_action_; + + // Holds the download ids which were dangerous. + std::set<int32> dangerous_downloads_seen_; + + DISALLOW_COPY_AND_ASSIGN(DownloadsObserver); +}; + +// WaitForFlush() returns after: +// * There are no IN_PROGRESS download items remaining on the +// DownloadManager. +// * There have been two round trip messages through the file and +// IO threads. +// This almost certainly means that a Download cancel has propagated through +// the system. +class DownloadsFlushObserver + : public DownloadManager::Observer, + public DownloadItem::Observer, + public base::RefCountedThreadSafe<DownloadsFlushObserver> { + public: + explicit DownloadsFlushObserver(DownloadManager* download_manager) + : download_manager_(download_manager), + waiting_for_zero_inprogress_(true) {} + + void WaitForFlush() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + download_manager_->AddObserver(this); + ui_test_utils::RunMessageLoop(); + } + + // DownloadsManager observer methods. + virtual void ModelChanged() { + // Model has changed, so there may be more DownloadItems to observe. + CheckDownloadsInProgress(true); + } + + // DownloadItem observer methods. + virtual void OnDownloadUpdated(DownloadItem* download) { + // No change in DownloadItem set on manager. + CheckDownloadsInProgress(false); + } + virtual void OnDownloadOpened(DownloadItem* download) {} + + protected: + friend class base::RefCountedThreadSafe<DownloadsFlushObserver>; + + virtual ~DownloadsFlushObserver() { + download_manager_->RemoveObserver(this); + for (std::set<DownloadItem*>::iterator it = downloads_observed_.begin(); + it != downloads_observed_.end(); ++it) { + (*it)->RemoveObserver(this); + } + } + + private: + // If we're waiting for that flush point, check the number + // of downloads in the IN_PROGRESS state and take appropriate + // action. If requested, also observes all downloads while iterating. + void CheckDownloadsInProgress(bool observe_downloads) { + if (waiting_for_zero_inprogress_) { + int count = 0; + + std::vector<DownloadItem*> downloads; + download_manager_->SearchDownloads(string16(), &downloads); + std::vector<DownloadItem*>::iterator it = downloads.begin(); + for (; it != downloads.end(); ++it) { + if ((*it)->state() == DownloadItem::IN_PROGRESS) + count++; + if (observe_downloads) { + if (downloads_observed_.find(*it) == downloads_observed_.end()) { + (*it)->AddObserver(this); + } + // Download items are forever, and we don't want to make + // assumptions about future state transitions, so once we + // start observing them, we don't stop until destruction. + } + } + + if (count == 0) { + waiting_for_zero_inprogress_ = false; + // Stop observing DownloadItems. We maintain the observation + // of DownloadManager so that we don't have to independently track + // whether we are observing it for conditional destruction. + for (std::set<DownloadItem*>::iterator it = downloads_observed_.begin(); + it != downloads_observed_.end(); ++it) { + (*it)->RemoveObserver(this); + } + downloads_observed_.clear(); + + // Trigger next step. We need to go past the IO thread twice, as + // there's a self-task posting in the IO thread cancel path. + BrowserThread::PostTask( + BrowserThread::FILE, FROM_HERE, + base::Bind(&DownloadsFlushObserver::PingFileThread, this, 2)); + } + } + } + + void PingFileThread(int cycle) { + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&DownloadsFlushObserver::PingIOThread, this, cycle)); + } + + void PingIOThread(int cycle) { + if (--cycle) { + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&DownloadsFlushObserver::PingFileThread, this, cycle)); + } else { + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, new MessageLoop::QuitTask()); + } + } + + DownloadManager* download_manager_; + std::set<DownloadItem*> downloads_observed_; + bool waiting_for_zero_inprogress_; + + DISALLOW_COPY_AND_ASSIGN(DownloadsFlushObserver); +}; + // Collect the information from FILE and IO threads needed for the Cancel // Test, specifically the number of outstanding requests on the // ResourceDispatcherHost and the number of pending downloads on the @@ -365,39 +717,39 @@ class DownloadTest : public InProcessBrowserTest { return GetDownloadPrefs(browser)->download_path(); } - // Create a DownloadTestObserver that will wait for the + // Create a DownloadsObserver that will wait for the // specified number of downloads to finish. - DownloadTestObserver* CreateWaiter(Browser* browser, int num_downloads) { + DownloadsObserver* CreateWaiter(Browser* browser, int num_downloads) { DownloadManager* download_manager = DownloadManagerForBrowser(browser); - return new DownloadTestObserver( + return new DownloadsObserver( download_manager, num_downloads, DownloadItem::COMPLETE, // Really done - true, // Bail on select file - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL); + true, // Bail on select file + ON_DANGEROUS_DOWNLOAD_FAIL); } - // Create a DownloadTestObserver that will wait for the + // Create a DownloadsObserver that will wait for the // specified number of downloads to start. - DownloadTestObserver* CreateInProgressWaiter(Browser* browser, + DownloadsObserver* CreateInProgressWaiter(Browser* browser, int num_downloads) { DownloadManager* download_manager = DownloadManagerForBrowser(browser); - return new DownloadTestObserver( + return new DownloadsObserver( download_manager, num_downloads, DownloadItem::IN_PROGRESS, // Has started true, // Bail on select file - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL); + ON_DANGEROUS_DOWNLOAD_FAIL); } - // Create a DownloadTestObserver that will wait for the + // Create a DownloadsObserver that will wait for the // specified number of downloads to finish, or for // a dangerous download warning to be shown. - DownloadTestObserver* DangerousInstallWaiter( + DownloadsObserver* DangerousInstallWaiter( Browser* browser, int num_downloads, DownloadItem::DownloadState final_state, - DownloadTestObserver::DangerousDownloadAction dangerous_download_action) { + DangerousDownloadAction dangerous_download_action) { DownloadManager* download_manager = DownloadManagerForBrowser(browser); - return new DownloadTestObserver( + return new DownloadsObserver( download_manager, num_downloads, final_state, true, // Bail on select file @@ -419,7 +771,7 @@ class DownloadTest : public InProcessBrowserTest { SelectExpectation expectation, int browser_test_flags) { // Setup notification, navigate, and block. - scoped_ptr<DownloadTestObserver> observer(CreateWaiter(browser, 1)); + scoped_ptr<DownloadsObserver> observer(CreateWaiter(browser, 1)); // This call will block until the condition specified by // |browser_test_flags|, but will not wait for the download to finish. ui_test_utils::NavigateToURLWithDisposition(browser, @@ -510,7 +862,7 @@ class DownloadTest : public InProcessBrowserTest { // Download a partial web page in a background tab and wait. // The mock system will not complete until it gets a special URL. - scoped_ptr<DownloadTestObserver> observer(CreateWaiter(browser, 1)); + scoped_ptr<DownloadsObserver> observer(CreateWaiter(browser, 1)); ui_test_utils::NavigateToURL(browser, url); // TODO(ahendrickson): check download status text before downloading. @@ -685,13 +1037,13 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadMimeTypeSelect) { // Download the file and wait. We expect the Select File dialog to appear // due to the MIME type, but we still wait until the download completes. - scoped_ptr<DownloadTestObserver> observer( - new DownloadTestObserver( + scoped_ptr<DownloadsObserver> observer( + new DownloadsObserver( DownloadManagerForBrowser(browser()), 1, DownloadItem::COMPLETE, // Really done false, // Continue on select file. - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); + ON_DANGEROUS_DOWNLOAD_FAIL)); ui_test_utils::NavigateToURLWithDisposition( browser(), url, CURRENT_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); @@ -1109,11 +1461,11 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadCancelled) { EXPECT_EQ(1, browser()->tab_count()); // TODO(rdsmith): Fragile code warning! The code below relies on the - // DownloadTestObserver only finishing when the new download has reached + // DownloadsObserver only finishing when the new download has reached // the state of being entered into the history and being user-visible // (that's what's required for the Remove to be valid and for the // download shelf to be visible). By the pure semantics of - // DownloadTestObserver, that's not guaranteed; DownloadItems are created + // DownloadsObserver, that's not guaranteed; DownloadItems are created // in the IN_PROGRESS state and made known to the DownloadManager // immediately, so any ModelChanged event on the DownloadManager after // navigation would allow the observer to return. However, the only @@ -1126,7 +1478,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadCancelled) { // Create a download, wait until it's started, and confirm // we're in the expected state. - scoped_ptr<DownloadTestObserver> observer( + scoped_ptr<DownloadsObserver> observer( CreateInProgressWaiter(browser(), 1)); ui_test_utils::NavigateToURL( browser(), GURL(URLRequestSlowDownloadJob::kUnknownSizeUrl)); @@ -1141,9 +1493,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadCancelled) { // Cancel the download and wait for download system quiesce. downloads[0]->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD); - scoped_refptr<DownloadTestFlushObserver> flush_observer( - new DownloadTestFlushObserver( - DownloadManagerForBrowser(browser()))); + scoped_refptr<DownloadsFlushObserver> flush_observer( + new DownloadsFlushObserver(DownloadManagerForBrowser(browser()))); flush_observer->WaitForFlush(); // Get the important info from other threads and check it. @@ -1257,7 +1608,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, AnchorDownloadTag) { // Create a download, wait until it's complete, and confirm // we're in the expected state. - scoped_ptr<DownloadTestObserver> observer(CreateWaiter(browser(), 1)); + scoped_ptr<DownloadsObserver> observer(CreateWaiter(browser(), 1)); ui_test_utils::NavigateToURL(browser(), url); observer->WaitForFinished(); @@ -1303,10 +1654,11 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxDenyInstall) { ASSERT_TRUE(InitialSetup(false)); GURL extension_url(URLRequestMockHTTPJob::GetMockUrl(kGoodCrxPath)); - scoped_ptr<DownloadTestObserver> observer( - DangerousInstallWaiter( - browser(), 1, DownloadItem::CANCELLED, - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_DENY)); + scoped_ptr<DownloadsObserver> observer( + DangerousInstallWaiter(browser(), + 1, + DownloadItem::CANCELLED, + ON_DANGEROUS_DOWNLOAD_DENY)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); @@ -1332,10 +1684,11 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxInstallDenysPermissions) { download_crx_util::SetMockInstallUIForTesting( new MockAbortExtensionInstallUI()); - scoped_ptr<DownloadTestObserver> observer( - DangerousInstallWaiter( - browser(), 1, DownloadItem::COMPLETE, - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT)); + scoped_ptr<DownloadsObserver> observer( + DangerousInstallWaiter(browser(), + 1, + DownloadItem::COMPLETE, + ON_DANGEROUS_DOWNLOAD_ACCEPT)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); @@ -1361,10 +1714,11 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxInstallAcceptPermissions) { download_crx_util::SetMockInstallUIForTesting( new MockAutoConfirmExtensionInstallUI(browser()->profile())); - scoped_ptr<DownloadTestObserver> observer( - DangerousInstallWaiter( - browser(), 1, DownloadItem::COMPLETE, - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT)); + scoped_ptr<DownloadsObserver> observer( + DangerousInstallWaiter(browser(), + 1, + DownloadItem::COMPLETE, + ON_DANGEROUS_DOWNLOAD_ACCEPT)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); @@ -1391,10 +1745,11 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxInvalid) { download_crx_util::SetMockInstallUIForTesting( new MockAutoConfirmExtensionInstallUI(browser()->profile())); - scoped_ptr<DownloadTestObserver> observer( - DangerousInstallWaiter( - browser(), 1, DownloadItem::COMPLETE, - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT)); + scoped_ptr<DownloadsObserver> observer( + DangerousInstallWaiter(browser(), + 1, + DownloadItem::COMPLETE, + ON_DANGEROUS_DOWNLOAD_ACCEPT)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); @@ -1415,10 +1770,11 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxLargeTheme) { download_crx_util::SetMockInstallUIForTesting( new MockAutoConfirmExtensionInstallUI(browser()->profile())); - scoped_ptr<DownloadTestObserver> observer( - DangerousInstallWaiter( - browser(), 1, DownloadItem::COMPLETE, - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT)); + scoped_ptr<DownloadsObserver> observer( + DangerousInstallWaiter(browser(), + 1, + DownloadItem::COMPLETE, + ON_DANGEROUS_DOWNLOAD_ACCEPT)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); diff --git a/chrome/browser/download/download_service.cc b/chrome/browser/download/download_service.cc index 6b5fcb2..659af4f 100644 --- a/chrome/browser/download/download_service.cc +++ b/chrome/browser/download/download_service.cc @@ -6,9 +6,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/download/chrome_download_manager_delegate.h" -#include "chrome/browser/download/download_service_factory.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/profiles/profile_manager.h" #include "content/browser/download/download_manager.h" DownloadService::DownloadService(Profile* profile) @@ -36,27 +34,6 @@ bool DownloadService::HasCreatedDownloadManager() { return download_manager_created_; } -int DownloadService::DownloadCount() const { - return download_manager_created_ ? manager_->in_progress_count() : 0; -} - -// static -int DownloadService::DownloadCountAllProfiles() { - std::vector<Profile*> profiles( - g_browser_process->profile_manager()->GetLoadedProfiles()); - - int count = 0; - for (std::vector<Profile*>::iterator it = profiles.begin(); - it < profiles.end(); ++it) { - count += DownloadServiceFactory::GetForProfile(*it)->DownloadCount(); - if ((*it)->HasOffTheRecordProfile()) - count += DownloadServiceFactory::GetForProfile( - (*it)->GetOffTheRecordProfile())->DownloadCount(); - } - - return count; -} - void DownloadService::SetDownloadManagerDelegateForTesting( ChromeDownloadManagerDelegate* new_delegate) { // Guarantee everything is properly initialized. diff --git a/chrome/browser/download/download_service.h b/chrome/browser/download/download_service.h index 2cc6570..4d754b0 100644 --- a/chrome/browser/download/download_service.h +++ b/chrome/browser/download/download_service.h @@ -29,12 +29,6 @@ class DownloadService : public ProfileKeyedService { // Has a download manager been created? (By calling above function.) bool HasCreatedDownloadManager(); - // Number of downloads associated with this instance of the service. - int DownloadCount() const; - - // Number of downloads associated with all profiles. - static int DownloadCountAllProfiles(); - // Sets the DownloadManagerDelegate associated with this object and // its DownloadManager. Takes ownership of |delegate|, and destroys // the previous delegate. For testing. diff --git a/chrome/browser/download/download_test_observer.cc b/chrome/browser/download/download_test_observer.cc deleted file mode 100644 index 1d0c5b5..0000000 --- a/chrome/browser/download/download_test_observer.cc +++ /dev/null @@ -1,297 +0,0 @@ -// Copyright (c) 2011 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 <vector> - -#include "base/logging.h" -#include "base/message_loop.h" -#include "base/stl_util.h" -#include "base/task.h" -#include "chrome/browser/download/download_test_observer.h" -#include "chrome/test/base/ui_test_utils.h" -#include "content/browser/browser_thread.h" - -// These functions take scoped_refptr's to DownloadManager because they -// are posted to message queues, and hence may execute arbitrarily after -// their actual posting. Once posted, there is no connection between -// these routines and the DownloadTestObserver class from which they came, -// so the DownloadTestObserver's reference to the DownloadManager cannot -// be counted on to keep the DownloadManager around. - -// Fake user click on "Accept". -void AcceptDangerousDownload(scoped_refptr<DownloadManager> download_manager, - int32 download_id) { - DownloadItem* download = download_manager->GetDownloadItem(download_id); - download->DangerousDownloadValidated(); -} - -// Fake user click on "Deny". -void DenyDangerousDownload(scoped_refptr<DownloadManager> download_manager, - int32 download_id) { - DownloadItem* download = download_manager->GetDownloadItem(download_id); - ASSERT_TRUE(download->IsPartialDownload()); - download->Cancel(true); - download->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD); -} - -DownloadTestObserver::DownloadTestObserver( - DownloadManager* download_manager, - size_t wait_count, - DownloadItem::DownloadState download_finished_state, - bool finish_on_select_file, - DangerousDownloadAction dangerous_download_action) - : download_manager_(download_manager), - wait_count_(wait_count), - finished_downloads_at_construction_(0), - waiting_(false), - download_finished_state_(download_finished_state), - finish_on_select_file_(finish_on_select_file), - select_file_dialog_seen_(false), - dangerous_download_action_(dangerous_download_action) { - download_manager_->AddObserver(this); // Will call initial ModelChanged(). - finished_downloads_at_construction_ = finished_downloads_.size(); - EXPECT_NE(DownloadItem::REMOVING, download_finished_state) - << "Waiting for REMOVING is not supported. Try COMPLETE."; - } - -DownloadTestObserver::~DownloadTestObserver() { - for (DownloadSet::iterator it = downloads_observed_.begin(); - it != downloads_observed_.end(); ++it) - (*it)->RemoveObserver(this); - - download_manager_->RemoveObserver(this); -} - -void DownloadTestObserver::WaitForFinished() { - if (!IsFinished()) { - waiting_ = true; - ui_test_utils::RunMessageLoop(); - waiting_ = false; - } -} - -bool DownloadTestObserver::IsFinished() const { - if (finished_downloads_.size() - finished_downloads_at_construction_ >= - wait_count_) - return true; - return (finish_on_select_file_ && select_file_dialog_seen_); -} - -void DownloadTestObserver::OnDownloadUpdated(DownloadItem* download) { - // The REMOVING state indicates that the download is being destroyed. - // Stop observing. Do not do anything with it, as it is about to be gone. - if (download->state() == DownloadItem::REMOVING) { - DownloadSet::iterator it = downloads_observed_.find(download); - ASSERT_TRUE(it != downloads_observed_.end()); - downloads_observed_.erase(it); - download->RemoveObserver(this); - return; - } - - // Real UI code gets the user's response after returning from the observer. - if (download->safety_state() == DownloadItem::DANGEROUS && - !ContainsKey(dangerous_downloads_seen_, download->id())) { - dangerous_downloads_seen_.insert(download->id()); - - // Calling DangerousDownloadValidated() at this point will - // cause the download to be completed twice. Do what the real UI - // code does: make the call as a delayed task. - switch (dangerous_download_action_) { - case ON_DANGEROUS_DOWNLOAD_ACCEPT: - // Fake user click on "Accept". Delay the actual click, as the - // real UI would. - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - NewRunnableFunction( - &AcceptDangerousDownload, - download_manager_, - download->id())); - break; - - case ON_DANGEROUS_DOWNLOAD_DENY: - // Fake a user click on "Deny". Delay the actual click, as the - // real UI would. - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - NewRunnableFunction( - &DenyDangerousDownload, - download_manager_, - download->id())); - break; - - case ON_DANGEROUS_DOWNLOAD_FAIL: - ADD_FAILURE() << "Unexpected dangerous download item."; - break; - - default: - NOTREACHED(); - } - } - - if (download->state() == download_finished_state_) { - DownloadInFinalState(download); - } -} - -void DownloadTestObserver::ModelChanged() { - // Regenerate DownloadItem observers. If there are any download items - // in our final state, note them in |finished_downloads_| - // (done by |OnDownloadUpdated()|). - std::vector<DownloadItem*> downloads; - download_manager_->GetAllDownloads(FilePath(), &downloads); - - for (std::vector<DownloadItem*>::iterator it = downloads.begin(); - it != downloads.end(); ++it) { - OnDownloadUpdated(*it); // Safe to call multiple times; checks state. - - DownloadSet::const_iterator finished_it(finished_downloads_.find(*it)); - DownloadSet::iterator observed_it(downloads_observed_.find(*it)); - - // If it isn't finished and we're aren't observing it, start. - if (finished_it == finished_downloads_.end() && - observed_it == downloads_observed_.end()) { - (*it)->AddObserver(this); - downloads_observed_.insert(*it); - continue; - } - - // If it is finished and we are observing it, stop. - if (finished_it != finished_downloads_.end() && - observed_it != downloads_observed_.end()) { - (*it)->RemoveObserver(this); - downloads_observed_.erase(observed_it); - continue; - } - } -} - -void DownloadTestObserver::SelectFileDialogDisplayed(int32 /* id */) { - select_file_dialog_seen_ = true; - SignalIfFinished(); -} - -size_t DownloadTestObserver::NumDangerousDownloadsSeen() const { - return dangerous_downloads_seen_.size(); -} - -void DownloadTestObserver::DownloadInFinalState(DownloadItem* download) { - if (finished_downloads_.find(download) != finished_downloads_.end()) { - // We've already seen terminal state on this download. - return; - } - - // Record the transition. - finished_downloads_.insert(download); - - SignalIfFinished(); -} - -void DownloadTestObserver::SignalIfFinished() { - if (waiting_ && IsFinished()) - MessageLoopForUI::current()->Quit(); -} - -DownloadTestFlushObserver::DownloadTestFlushObserver( - DownloadManager* download_manager) - : download_manager_(download_manager), - waiting_for_zero_inprogress_(true) {} - -void DownloadTestFlushObserver::WaitForFlush() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - download_manager_->AddObserver(this); - ui_test_utils::RunMessageLoop(); -} - -void DownloadTestFlushObserver::ModelChanged() { - // Model has changed, so there may be more DownloadItems to observe. - CheckDownloadsInProgress(true); -} - -void DownloadTestFlushObserver::OnDownloadUpdated(DownloadItem* download) { - // The REMOVING state indicates that the download is being destroyed. - // Stop observing. Do not do anything with it, as it is about to be gone. - if (download->state() == DownloadItem::REMOVING) { - DownloadSet::iterator it = downloads_observed_.find(download); - ASSERT_TRUE(it != downloads_observed_.end()); - downloads_observed_.erase(it); - download->RemoveObserver(this); - return; - } - - // No change in DownloadItem set on manager. - CheckDownloadsInProgress(false); -} - -DownloadTestFlushObserver::~DownloadTestFlushObserver() { - download_manager_->RemoveObserver(this); - for (DownloadSet::iterator it = downloads_observed_.begin(); - it != downloads_observed_.end(); ++it) { - (*it)->RemoveObserver(this); - } -} - -// If we're waiting for that flush point, check the number -// of downloads in the IN_PROGRESS state and take appropriate -// action. If requested, also observes all downloads while iterating. -void DownloadTestFlushObserver::CheckDownloadsInProgress( - bool observe_downloads) { - if (waiting_for_zero_inprogress_) { - int count = 0; - - std::vector<DownloadItem*> downloads; - download_manager_->SearchDownloads(string16(), &downloads); - for (std::vector<DownloadItem*>::iterator it = downloads.begin(); - it != downloads.end(); ++it) { - if ((*it)->state() == DownloadItem::IN_PROGRESS) - count++; - if (observe_downloads) { - if (downloads_observed_.find(*it) == downloads_observed_.end()) { - (*it)->AddObserver(this); - downloads_observed_.insert(*it); - } - // Download items are forever, and we don't want to make - // assumptions about future state transitions, so once we - // start observing them, we don't stop until destruction. - } - } - - if (count == 0) { - waiting_for_zero_inprogress_ = false; - // Stop observing DownloadItems. We maintain the observation - // of DownloadManager so that we don't have to independently track - // whether we are observing it for conditional destruction. - for (DownloadSet::iterator it = downloads_observed_.begin(); - it != downloads_observed_.end(); ++it) { - (*it)->RemoveObserver(this); - } - downloads_observed_.clear(); - - // Trigger next step. We need to go past the IO thread twice, as - // there's a self-task posting in the IO thread cancel path. - BrowserThread::PostTask( - BrowserThread::FILE, FROM_HERE, - NewRunnableMethod(this, - &DownloadTestFlushObserver::PingFileThread, 2)); - } - } -} - -void DownloadTestFlushObserver::PingFileThread(int cycle) { - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - NewRunnableMethod(this, &DownloadTestFlushObserver::PingIOThread, - cycle)); -} - -void DownloadTestFlushObserver::PingIOThread(int cycle) { - if (--cycle) { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - NewRunnableMethod(this, &DownloadTestFlushObserver::PingFileThread, - cycle)); - } else { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, new MessageLoop::QuitTask()); - } -} diff --git a/chrome/browser/download/download_test_observer.h b/chrome/browser/download/download_test_observer.h deleted file mode 100644 index 01c03c6..0000000 --- a/chrome/browser/download/download_test_observer.h +++ /dev/null @@ -1,181 +0,0 @@ -// Copyright (c) 2011 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 CHROME_BROWSER_DOWNLOAD_DOWNLOAD_TEST_OBSERVER_H_ -#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_TEST_OBSERVER_H_ -#pragma once - -#include <set> - -#include "base/basictypes.h" -#include "base/memory/ref_counted.h" -#include "content/browser/download/download_item.h" -#include "content/browser/download/download_manager.h" - -// Construction of this class defines a system state, based on some number -// of downloads being seen in a particular state + other events that -// may occur in the download system. That state will be recorded if it -// occurs at any point after construction. When that state occurs, the class -// is considered finished. Callers may either probe for the finished state, or -// wait on it. -// -// TODO(rdsmith): Detect manager going down, remove pointer to -// DownloadManager, transition to finished. (For right now we -// just use a scoped_refptr<> to keep it around, but that may cause -// timeouts on waiting if a DownloadManager::Shutdown() occurs which -// cancels our in-progress downloads.) -class DownloadTestObserver : public DownloadManager::Observer, - public DownloadItem::Observer { - public: - // Action an observer should take if a dangerous download is encountered. - enum DangerousDownloadAction { - ON_DANGEROUS_DOWNLOAD_ACCEPT, // Accept the download - ON_DANGEROUS_DOWNLOAD_DENY, // Deny the download - ON_DANGEROUS_DOWNLOAD_FAIL // Fail if a dangerous download is seen - }; - - // Create an object that will be considered finished when |wait_count| - // download items have entered state |download_finished_state|. - // If |finish_on_select_file| is true, the object will also be - // considered finished if the DownloadManager raises a - // SelectFileDialogDisplayed() notification. - - // TODO(rdsmith): Consider rewriting the interface to take a list of events - // to treat as completion events. - DownloadTestObserver(DownloadManager* download_manager, - size_t wait_count, - DownloadItem::DownloadState download_finished_state, - bool finish_on_select_file, - DangerousDownloadAction dangerous_download_action); - - virtual ~DownloadTestObserver(); - - // State accessors. - bool select_file_dialog_seen() const { return select_file_dialog_seen_; } - - // Wait for whatever state was specified in the constructor. - void WaitForFinished(); - - // Return true if everything's happened that we're configured for. - bool IsFinished() const; - - // DownloadItem::Observer - virtual void OnDownloadUpdated(DownloadItem* download) OVERRIDE; - virtual void OnDownloadOpened(DownloadItem* download) OVERRIDE {} - - // DownloadManager::Observer - virtual void ModelChanged() OVERRIDE; - - virtual void SelectFileDialogDisplayed(int32 id) OVERRIDE; - - size_t NumDangerousDownloadsSeen() const; - - private: - typedef std::set<DownloadItem*> DownloadSet; - - // Called when we know that a download item is in a final state. - // Note that this is not the same as it first transitioning in to the - // final state; multiple notifications may occur once the item is in - // that state. So we keep our own track of transitions into final. - void DownloadInFinalState(DownloadItem* download); - - void SignalIfFinished(); - - // The observed download manager. - scoped_refptr<DownloadManager> download_manager_; - - // The set of DownloadItem's that have transitioned to their finished state - // since construction of this object. When the size of this array - // reaches wait_count_, we're done. - DownloadSet finished_downloads_; - - // The set of DownloadItem's we are currently observing. Generally there - // won't be any overlap with the above; once we see the final state - // on a DownloadItem, we'll stop observing it. - DownloadSet downloads_observed_; - - // The number of downloads to wait on completing. - size_t wait_count_; - - // The number of downloads entered in final state in initial - // ModelChanged(). We use |finished_downloads_| to track the incoming - // transitions to final state we should ignore, and to track the - // number of final state transitions that occurred between - // construction and return from wait. But some downloads may be in our - // final state (and thus be entered into |finished_downloads_|) when we - // construct this class. We don't want to count those in our transition - // to finished. - int finished_downloads_at_construction_; - - // Whether an internal message loop has been started and must be quit upon - // all downloads completing. - bool waiting_; - - // The state on which to consider the DownloadItem finished. - DownloadItem::DownloadState download_finished_state_; - - // True if we should transition the DownloadTestObserver to finished if - // the select file dialog comes up. - bool finish_on_select_file_; - - // True if we've seen the select file dialog. - bool select_file_dialog_seen_; - - // Action to take if a dangerous download is encountered. - DangerousDownloadAction dangerous_download_action_; - - // Holds the download ids which were dangerous. - std::set<int32> dangerous_downloads_seen_; - - DISALLOW_COPY_AND_ASSIGN(DownloadTestObserver); -}; - -// The WaitForFlush() method on this class returns after: -// * There are no IN_PROGRESS download items remaining on the -// DownloadManager. -// * There have been two round trip messages through the file and -// IO threads. -// This almost certainly means that a Download cancel has propagated through -// the system. -class DownloadTestFlushObserver - : public DownloadManager::Observer, - public DownloadItem::Observer, - public base::RefCountedThreadSafe<DownloadTestFlushObserver> { - public: - explicit DownloadTestFlushObserver(DownloadManager* download_manager); - - void WaitForFlush(); - - // DownloadsManager observer methods. - virtual void ModelChanged() OVERRIDE; - - // DownloadItem observer methods. - virtual void OnDownloadUpdated(DownloadItem* download) OVERRIDE; - virtual void OnDownloadOpened(DownloadItem* download) OVERRIDE {} - - protected: - friend class base::RefCountedThreadSafe<DownloadTestFlushObserver>; - - virtual ~DownloadTestFlushObserver(); - - private: - typedef std::set<DownloadItem*> DownloadSet; - - // If we're waiting for that flush point, check the number - // of downloads in the IN_PROGRESS state and take appropriate - // action. If requested, also observes all downloads while iterating. - void CheckDownloadsInProgress(bool observe_downloads); - - void PingFileThread(int cycle); - - void PingIOThread(int cycle); - - DownloadManager* download_manager_; - DownloadSet downloads_observed_; - bool waiting_for_zero_inprogress_; - - DISALLOW_COPY_AND_ASSIGN(DownloadTestFlushObserver); -}; - -#endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_TEST_OBSERVER_H_ diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index e2ed3b4..4e3b9f3 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -19,8 +19,8 @@ #include "base/metrics/histogram.h" #include "base/path_service.h" #include "base/string_number_conversions.h" -#include "base/string_util.h" #include "base/stringprintf.h" +#include "base/string_util.h" #include "base/threading/thread.h" #include "base/threading/thread_restrictions.h" #include "base/time.h" @@ -75,7 +75,6 @@ #include "chrome/browser/printing/print_preview_tab_controller.h" #include "chrome/browser/printing/print_view_manager.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/sessions/restore_tab_helper.h" #include "chrome/browser/sessions/session_service.h" #include "chrome/browser/sessions/session_service_factory.h" @@ -1046,64 +1045,6 @@ void Browser::InProgressDownloadResponse(bool cancel_downloads) { ShowDownloadsTab(); } -Browser::DownloadClosePreventionType Browser::OkToCloseWithInProgressDownloads( - int* num_downloads_blocking) const { - DCHECK(num_downloads_blocking); - *num_downloads_blocking = 0; - - if (is_attempting_to_close_browser_) - return DOWNLOAD_CLOSE_OK; - - // If we're not running a full browser process with a profile manager - // (testing), it's ok to close the browser. - if (!g_browser_process->profile_manager()) - return DOWNLOAD_CLOSE_OK; - - int total_download_count = DownloadService::DownloadCountAllProfiles(); - if (total_download_count == 0) - return DOWNLOAD_CLOSE_OK; // No downloads; can definitely close. - - // Figure out how many windows are open total, and associated with this - // profile, that are relevant for the ok-to-close decision. - int profile_window_count = 0; - int total_window_count = 0; - for (BrowserList::const_iterator iter = BrowserList::begin(); - iter != BrowserList::end(); ++iter) { - // Don't count this browser window or any other in the process of closing. - Browser* const browser = *iter; - // Check is_attempting_to_close_browser_ as window closing may be - // delayed, and windows that are in the process of closing don't - // count against our totals. - if (browser == this || browser->is_attempting_to_close_browser_) - continue; - - if ((*iter)->profile() == profile()) - profile_window_count++; - total_window_count++; - } - - // If there aren't any other windows, we're at browser shutdown, - // which would cancel all current downloads. - if (total_window_count == 0) { - *num_downloads_blocking = total_download_count; - return DOWNLOAD_CLOSE_BROWSER_SHUTDOWN; - } - - // If there aren't any other windows on our profile, and we're an incognito - // profile, and there are downloads associated with that profile, - // those downloads would be cancelled by our window (-> profile) close. - DownloadService* download_service = - DownloadServiceFactory::GetForProfile(profile()); - if (profile_window_count == 0 && download_service->DownloadCount() > 0 && - profile()->IsOffTheRecord()) { - *num_downloads_blocking = download_service->DownloadCount(); - return DOWNLOAD_CLOSE_LAST_WINDOW_IN_INCOGNITO_PROFILE; - } - - // Those are the only conditions under which we will block shutdown. - return DOWNLOAD_CLOSE_OK; -} - //////////////////////////////////////////////////////////////////////////////// // Browser, TabStripModel pass-thrus: @@ -4997,19 +4938,100 @@ void Browser::ClearUnloadState(TabContents* tab, bool process_now) { /////////////////////////////////////////////////////////////////////////////// // Browser, In-progress download termination handling (private): +void Browser::CheckDownloadsInProgress(bool* normal_downloads_are_present, + bool* incognito_downloads_are_present) { + *normal_downloads_are_present = false; + *incognito_downloads_are_present = false; + + // If there are no download in-progress, our job is done. + DownloadManager* download_manager = NULL; + DownloadService* download_service = + DownloadServiceFactory::GetForProfile(profile()); + // But first we need to check for the existence of the download manager, as + // GetDownloadManager() will unnecessarily try to create one if it does not + // exist. + if (download_service->HasCreatedDownloadManager()) + download_manager = download_service->GetDownloadManager(); + if (profile()->IsOffTheRecord()) { + // Browser is incognito and so download_manager if present is for incognito + // downloads. + *incognito_downloads_are_present = + (download_manager && download_manager->in_progress_count() != 0); + // Check original profile. + DownloadService* download_service = DownloadServiceFactory::GetForProfile( + profile()->GetOriginalProfile()); + if (download_service->HasCreatedDownloadManager()) + download_manager = download_service->GetDownloadManager(); + } + + *normal_downloads_are_present = + (download_manager && download_manager->in_progress_count() != 0); +} + bool Browser::CanCloseWithInProgressDownloads() { - // If we've prompted, we need to hear from the user before we - // can close. - if (cancel_download_confirmation_state_ != NOT_PROMPTED) - return cancel_download_confirmation_state_ != WAITING_FOR_RESPONSE; - - int num_downloads_blocking; - if (DOWNLOAD_CLOSE_OK == - OkToCloseWithInProgressDownloads(&num_downloads_blocking)) + if (cancel_download_confirmation_state_ != NOT_PROMPTED) { + if (cancel_download_confirmation_state_ == WAITING_FOR_RESPONSE) { + // We need to hear from the user before we can close. + return false; + } + // RESPONSE_RECEIVED case, the user decided to go along with the closing. + return true; + } + // Indicated that normal (non-incognito) downloads are pending. + bool normal_downloads_are_present = false; + bool incognito_downloads_are_present = false; + CheckDownloadsInProgress(&normal_downloads_are_present, + &incognito_downloads_are_present); + if (!normal_downloads_are_present && !incognito_downloads_are_present) + return true; + + if (is_attempting_to_close_browser_) + return true; + + if ((!normal_downloads_are_present && !profile()->IsOffTheRecord()) || + (!incognito_downloads_are_present && profile()->IsOffTheRecord())) + return true; + + // Let's figure out if we are the last window for our profile. + // Note that we cannot just use BrowserList::GetBrowserCount as browser + // windows closing is delayed and the returned count might include windows + // that are being closed. + // The browser allowed to be closed only if: + // 1. It is a regular browser and there are no regular downloads present or + // this is not the last regular browser window. + // 2. It is an incognito browser and there are no incognito downloads present + // or this is not the last incognito browser window. + int count = 0; + for (BrowserList::const_iterator iter = BrowserList::begin(); + iter != BrowserList::end(); ++iter) { + // Don't count this browser window or any other in the process of closing. + // Only consider tabbed browser windows, not popups. + Browser* const browser = *iter; + if (browser == this + || browser->is_attempting_to_close_browser_ + || !browser->is_type_tabbed()) + continue; + + // Verify that this is not the last non-incognito or incognito browser, + // depending on the pending downloads. + if (normal_downloads_are_present && !profile()->IsOffTheRecord() && + browser->profile()->IsOffTheRecord()) + continue; + if (incognito_downloads_are_present && profile()->IsOffTheRecord() && + !browser->profile()->IsOffTheRecord()) + continue; + + // We test the original profile, because an incognito browser window keeps + // the original profile alive (and its DownloadManager). + // We also need to test explicitly the profile directly so that 2 incognito + // profiles count as a match. + if ((*iter)->profile() == profile() || + (*iter)->profile()->GetOriginalProfile() == profile()) + count++; + } + if (count > 0) return true; - // Closing this window will kill some downloads; prompt to make sure - // that's ok. cancel_download_confirmation_state_ = WAITING_FOR_RESPONSE; window_->ConfirmBrowserCloseWithPendingDownloads(); diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h index e383062..26a9ad4 100644 --- a/chrome/browser/ui/browser.h +++ b/chrome/browser/ui/browser.h @@ -109,21 +109,6 @@ class Browser : public TabHandlerDelegate, FEATURE_DOWNLOADSHELF = 128 }; - // The context for a download blocked notification from - // OkToCloseWithInProgressDownloads. - enum DownloadClosePreventionType { - // Browser close is not blocked by download state. - DOWNLOAD_CLOSE_OK, - - // The browser is shutting down and there are active downloads - // that would be cancelled. - DOWNLOAD_CLOSE_BROWSER_SHUTDOWN, - - // There are active downloads associated with this incognito profile - // that would be canceled. - DOWNLOAD_CLOSE_LAST_WINDOW_IN_INCOGNITO_PROFILE, - }; - struct CreateParams { CreateParams(Type type, Profile* profile); @@ -356,20 +341,16 @@ class Browser : public TabHandlerDelegate, // In-progress download termination handling ///////////////////////////////// + // Are normal and/or incognito downloads in progress? + void CheckDownloadsInProgress(bool* normal_downloads, + bool* incognito_downloads); + // Called when the user has decided whether to proceed or not with the browser // closure. |cancel_downloads| is true if the downloads should be canceled // and the browser closed, false if the browser should stay open and the // downloads running. void InProgressDownloadResponse(bool cancel_downloads); - // Indicates whether or not this browser window can be closed, or - // would be blocked by in-progress downloads. - // If executing downloads would be cancelled by this window close, - // then |*num_downloads_blocking| is updated with how many downloads - // would be canceled if the close continued. - DownloadClosePreventionType OkToCloseWithInProgressDownloads( - int* num_downloads_blocking) const; - // TabStripModel pass-thrus ///////////////////////////////////////////////// TabStripModel* tabstrip_model() const { diff --git a/chrome/browser/ui/browser_close_browsertest.cc b/chrome/browser/ui/browser_close_browsertest.cc deleted file mode 100644 index 019db26..0000000 --- a/chrome/browser/ui/browser_close_browsertest.cc +++ /dev/null @@ -1,546 +0,0 @@ -// Copyright (c) 2011 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/command_line.h" -#include "base/logging.h" -#include "base/path_service.h" -#include "base/stringprintf.h" -#include "chrome/browser/browser_process.h" -#include "chrome/browser/download/download_service.h" -#include "chrome/browser/download/download_service_factory.h" -#include "chrome/browser/download/download_test_observer.h" -#include "chrome/browser/net/url_request_mock_util.h" -#include "chrome/browser/prefs/pref_service.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/profiles/profile_manager.h" -#include "chrome/browser/ui/browser.h" -#include "chrome/browser/ui/browser_window.h" -#include "chrome/browser/ui/webui/active_downloads_ui.h" -#include "chrome/common/chrome_paths.h" -#include "chrome/common/pref_names.h" -#include "chrome/common/url_constants.h" -#include "chrome/test/base/in_process_browser_test.h" -#include "chrome/test/base/ui_test_utils.h" -#include "content/browser/download/download_item.h" -#include "content/browser/net/url_request_slow_download_job.h" -#include "content/browser/tab_contents/tab_contents.h" -#include "content/public/common/page_transition_types.h" - -class BrowserCloseTest : public InProcessBrowserTest { - public: - // Structure defining test cases for DownloadsCloseCheck. - struct DownloadsCloseCheckCase { - std::string DebugString() const; - - // Input - struct { - struct { - int windows; - int downloads; - } regular; - struct { - int windows; - int downloads; - } incognito; - } profile_a; - - struct { - struct { - int windows; - int downloads; - } regular; - struct { - int windows; - int downloads; - } incognito; - } profile_b; - - // We always probe a window in profile A. - enum { REGULAR = 0, INCOGNITO = 1 } window_to_probe; - - // Output - Browser::DownloadClosePreventionType type; - - // Unchecked if type == DOWNLOAD_CLOSE_OK. - int num_blocking; - }; - - protected: - virtual void SetUpOnMainThread() OVERRIDE { - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&chrome_browser_net::SetUrlRequestMocksEnabled, true)); - } - - // Create a second profile to work within multi-profile. - Profile* CreateSecondProfile() { - FilePath user_data_dir; - PathService::Get(chrome::DIR_USER_DATA, &user_data_dir); - - if (!second_profile_data_dir_.CreateUniqueTempDirUnderPath(user_data_dir)) - return NULL; - - return g_browser_process->profile_manager()->GetProfile( - second_profile_data_dir_.path()); - } - - // Create |num_downloads| number of downloads that are stalled - // (will quickly get to a place where the server won't - // provide any more data) so that we can test closing the - // browser with active downloads. - void CreateStalledDownloads(Browser* browser, int num_downloads) { - GURL url(URLRequestSlowDownloadJob::kKnownSizeUrl); - - if (num_downloads == 0) - return; - - // Setup an observer waiting for the given number of downloads - // to get to IN_PROGRESS. - DownloadManager* download_manager = - browser->profile()->GetDownloadManager(); - scoped_ptr<DownloadTestObserver> observer( - new DownloadTestObserver( - download_manager, num_downloads, - DownloadItem::IN_PROGRESS, - true, // Bail on select file. - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); - - // Set of that number of downloads. - while (num_downloads--) - ui_test_utils::NavigateToURLWithDisposition( - browser, url, NEW_BACKGROUND_TAB, - ui_test_utils::BROWSER_TEST_NONE); - - // Wait for them. - observer->WaitForFinished(); - } - - // All all downloads created in CreateStalledDownloads() to - // complete, and block in this routine until they do complete. - void CompleteAllDownloads(Browser* browser) { - GURL finish_url(URLRequestSlowDownloadJob::kFinishDownloadUrl); - ui_test_utils::NavigateToURL(browser, finish_url); - - // Go through and, for every single profile, wait until there are - // no active downloads on that download manager. - std::vector<Profile*> profiles( - g_browser_process->profile_manager()->GetLoadedProfiles()); - for (std::vector<Profile*>::const_iterator pit = profiles.begin(); - pit != profiles.end(); ++pit) { - DownloadService* download_service = - DownloadServiceFactory::GetForProfile(*pit); - if (download_service->HasCreatedDownloadManager()) { - DownloadManager *mgr = download_service->GetDownloadManager(); - scoped_refptr<DownloadTestFlushObserver> observer( - new DownloadTestFlushObserver(mgr)); - observer->WaitForFlush(); - } - if ((*pit)->HasOffTheRecordProfile()) { - DownloadService* incognito_download_service = - DownloadServiceFactory::GetForProfile( - (*pit)->GetOffTheRecordProfile()); - if (incognito_download_service->HasCreatedDownloadManager()) { - DownloadManager *mgr = - incognito_download_service->GetDownloadManager(); - scoped_refptr<DownloadTestFlushObserver> observer( - new DownloadTestFlushObserver(mgr)); - observer->WaitForFlush(); - } - } - } - } - - // Create a Browser (with associated window) on the specified profile. - Browser* CreateBrowserOnProfile(Profile* profile) { - Browser* new_browser = Browser::Create(profile); - new_browser->AddSelectedTabWithURL(GURL(chrome::kAboutBlankURL), - content::PAGE_TRANSITION_START_PAGE); - ui_test_utils::WaitForNavigation( - &new_browser->GetSelectedTabContents()->controller()); - new_browser->window()->Show(); - return new_browser; - } - - // Adjust the number of browsers and associated windows up or down - // to |num_windows|. This routine assumes that there is only a single - // browser associated with the profile on entry. |*base_browser| contains - // this browser, and the profile is derived from that browser. On output, - // if |*base_browser| was destroyed (because |num_windows == 0|), NULL - // is assigned to that memory location. - bool AdjustBrowsersOnProfile(Browser** base_browser, int num_windows) { - int num_downloads_blocking; - if (num_windows == 0) { - if (Browser::DOWNLOAD_CLOSE_OK != - (*base_browser)->OkToCloseWithInProgressDownloads( - &num_downloads_blocking)) - return false; - (*base_browser)->window()->Close(); - *base_browser = 0; - return true; - } - - // num_windows > 0 - Profile* profile((*base_browser)->profile()); - for (int w = 1; w < num_windows; ++w) { - CreateBrowserOnProfile(profile); - } - return true; - } - - int TotalUnclosedBrowsers() { - int count = 0; - for (BrowserList::const_iterator iter = BrowserList::begin(); - iter != BrowserList::end(); ++iter) - if (!(*iter)->IsAttemptingToCloseBrowser()) { - count++; - } - return count; - } - - // Note that this is invalid to call if TotalUnclosedBrowsers() == 0. - Browser* FirstUnclosedBrowser() { - for (BrowserList::const_iterator iter = BrowserList::begin(); - iter != BrowserList::end(); ++iter) - if (!(*iter)->IsAttemptingToCloseBrowser()) - return (*iter); - return NULL; - } - - bool SetupForDownloadCloseCheck() { - first_profile_ = browser()->profile(); - - bool result = first_profile_data_dir_.CreateUniqueTempDir(); - EXPECT_TRUE(result); - if (!result) return false; - first_profile_->GetPrefs()->SetFilePath( - prefs::kDownloadDefaultDirectory, - first_profile_data_dir_.path()); - - second_profile_ = CreateSecondProfile(); - EXPECT_TRUE(second_profile_); - if (!second_profile_) return false; - - return true; - } - - // Test a specific DownloadsCloseCheckCase. Returns false if - // an assertion has failed and the test should be aborted. - bool ExecuteDownloadCloseCheckCase(size_t i) { - const DownloadsCloseCheckCase& check_case(download_close_check_cases[i]); - - // Test invariant: so that we don't actually try and close the browser, - // we always enter the function with a single browser window open on the - // main profile. That means we need to exit the function the same way. - // So we setup everything except for the |first_profile_| regular, and then - // flip the bit on the main window. - // Note that this means that browser() is unreliable in the context - // of this function or its callers; we'll be killing that main window - // and recreating it fairly frequently. - int unclosed_browsers = TotalUnclosedBrowsers(); - EXPECT_EQ(1, unclosed_browsers); - if (1 != unclosed_browsers) - return false; - - Browser* entry_browser = FirstUnclosedBrowser(); - EXPECT_EQ(first_profile_, entry_browser->profile()) - << "Case" << i - << ": " << check_case.DebugString(); - if (first_profile_ != entry_browser->profile()) - return false; - int total_download_count = DownloadService::DownloadCountAllProfiles(); - EXPECT_EQ(0, total_download_count) - << "Case " << i - << ": " << check_case.DebugString(); - if (0 != total_download_count) - return false; - - Profile* first_profile_incognito = first_profile_->GetOffTheRecordProfile(); - Profile* second_profile_incognito = - second_profile_->GetOffTheRecordProfile(); - - // For simplicty of coding, we create a window on each profile so that - // we can easily create downloads, then we destroy or create windows - // as necessary. - Browser* browser_a_regular(CreateBrowserOnProfile(first_profile_)); - Browser* browser_a_incognito( - CreateBrowserOnProfile(first_profile_incognito)); - Browser* browser_b_regular(CreateBrowserOnProfile(second_profile_)); - Browser* browser_b_incognito( - CreateBrowserOnProfile(second_profile_incognito)); - - // Kill our entry browser. - entry_browser->window()->Close(); - entry_browser = NULL; - - // Create all downloads needed. - CreateStalledDownloads( - browser_a_regular, check_case.profile_a.regular.downloads); - CreateStalledDownloads( - browser_a_incognito, check_case.profile_a.incognito.downloads); - CreateStalledDownloads( - browser_b_regular, check_case.profile_b.regular.downloads); - CreateStalledDownloads( - browser_b_incognito, check_case.profile_b.incognito.downloads); - - // Adjust the windows - Browser** browsers[] = { - &browser_a_regular, &browser_a_incognito, - &browser_b_regular, &browser_b_incognito - }; - int window_counts[] = { - check_case.profile_a.regular.windows, - check_case.profile_a.incognito.windows, - check_case.profile_b.regular.windows, - check_case.profile_b.incognito.windows, - }; - for (size_t j = 0; j < arraysize(browsers); ++j) { - bool result = AdjustBrowsersOnProfile(browsers[j], window_counts[j]); - EXPECT_TRUE(result); - if (!result) - return false; - } - ui_test_utils::RunAllPendingInMessageLoop(); - -#if defined(OS_CHROMEOS) - // Get rid of downloads panel on ChromeOS - Browser* panel = NULL; -#if defined(TOUCH_UI) - ActiveDownloadsUI::GetPopup(&panel); -#else - panel = ActiveDownloadsUI::GetPopup(); -#endif - if (panel) - panel->CloseWindow(); - ui_test_utils::RunAllPendingInMessageLoop(); -#endif - - // All that work, for this one little test. - EXPECT_TRUE((check_case.window_to_probe == - DownloadsCloseCheckCase::REGULAR) || - (check_case.window_to_probe == - DownloadsCloseCheckCase::INCOGNITO)); - if (!((check_case.window_to_probe == - DownloadsCloseCheckCase::REGULAR) || - (check_case.window_to_probe == - DownloadsCloseCheckCase::INCOGNITO))) - return false; - - int num_downloads_blocking; - Browser* browser_to_probe = - (check_case.window_to_probe == DownloadsCloseCheckCase::REGULAR ? - browser_a_regular : - browser_a_incognito); - Browser::DownloadClosePreventionType type = - browser_to_probe->OkToCloseWithInProgressDownloads( - &num_downloads_blocking); - EXPECT_EQ(check_case.type, type) << "Case " << i - << ": " << check_case.DebugString(); - if (type != Browser::DOWNLOAD_CLOSE_OK) - EXPECT_EQ(check_case.num_blocking, num_downloads_blocking) - << "Case " << i - << ": " << check_case.DebugString(); - - // Release all the downloads. - CompleteAllDownloads(browser_to_probe); - - // Create a new main window and kill everything else. - entry_browser = CreateBrowserOnProfile(first_profile_); - for (BrowserList::const_iterator bit = BrowserList::begin(); - bit != BrowserList::end(); ++bit) { - if ((*bit) != entry_browser) { - EXPECT_TRUE((*bit)->window()); - if (!(*bit)->window()) - return false; - (*bit)->window()->Close(); - } - } - ui_test_utils::RunAllPendingInMessageLoop(); - - return true; - } - - static const DownloadsCloseCheckCase download_close_check_cases[]; - - // DownloadCloseCheck variables. - Profile* first_profile_; - Profile* second_profile_; - - ScopedTempDir first_profile_data_dir_; - ScopedTempDir second_profile_data_dir_; -}; - -const BrowserCloseTest::DownloadsCloseCheckCase -BrowserCloseTest::download_close_check_cases[] = { - // Top level nesting is {profile_a, profile_b} - // Second level nesting is {regular, incognito - // Third level (inner) nesting is {windows, downloads} - - // Last window (incognito) triggers browser close warning. - {{{0, 0}, {1, 1}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO, - Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN, 1}, - - // Last incognito window triggers incognito close warning. - {{{1, 0}, {1, 1}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO, - Browser::DOWNLOAD_CLOSE_LAST_WINDOW_IN_INCOGNITO_PROFILE, 1}, - - // Last incognito window with no downloads triggers no warning. - {{{0, 0}, {1, 0}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO, - Browser::DOWNLOAD_CLOSE_OK}, - - // Last incognito window with window+download on another incognito profile - // triggers no warning. - {{{0, 0}, {1, 0}}, {{0, 0}, {1, 1}}, - BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO, - Browser::DOWNLOAD_CLOSE_OK}, - - // Non-last incognito window triggers no warning. - {{{0, 0}, {2, 1}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO, - Browser::DOWNLOAD_CLOSE_OK}, - - // Non-last regular window triggers no warning. - {{{2, 1}, {0, 0}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::REGULAR, - Browser::DOWNLOAD_CLOSE_OK}, - - // Last regular window triggers browser close. - {{{1, 1}, {0, 0}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::REGULAR, - Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN, 1}, - - // Last regular window triggers browser close for download on different - // profile. - {{{1, 0}, {0, 0}}, {{0, 1}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::REGULAR, - Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN, 1}, - - // Last regular window triggers no warning if incognito - // active (http://crbug.com/61257). - {{{1, 0}, {1, 1}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::REGULAR, - Browser::DOWNLOAD_CLOSE_OK}, - - // Last regular window triggers no warning if other profile window active. - {{{1, 1}, {0, 0}}, {{1, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::REGULAR, - Browser::DOWNLOAD_CLOSE_OK}, - - // Last regular window triggers no warning if other incognito window - // active. - {{{1, 0}, {0, 0}}, {{0, 0}, {1, 1}}, - BrowserCloseTest::DownloadsCloseCheckCase::REGULAR, - Browser::DOWNLOAD_CLOSE_OK}, - - // Last regular window triggers no warning if incognito active. - {{{1, 1}, {1, 0}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::REGULAR, - Browser::DOWNLOAD_CLOSE_OK}, - - // Test plural for regular. - {{{1, 2}, {0, 0}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::REGULAR, - Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN, 2}, - - // Test plural for incognito. - {{{1, 0}, {1, 2}}, {{0, 0}, {0, 0}}, - BrowserCloseTest::DownloadsCloseCheckCase::INCOGNITO, - Browser::DOWNLOAD_CLOSE_LAST_WINDOW_IN_INCOGNITO_PROFILE, 2}, -}; - -std::string BrowserCloseTest::DownloadsCloseCheckCase::DebugString() const { - std::string result; - result += "{"; - if (profile_a.regular.windows || profile_a.regular.downloads) - result += base::StringPrintf("Regular profile A: (%d w, %d d), ", - profile_a.regular.windows, - profile_a.regular.downloads); - if (profile_a.incognito.windows || profile_a.incognito.downloads) - result += base::StringPrintf("Incognito profile A: (%d w, %d d), ", - profile_a.incognito.windows, - profile_a.incognito.downloads); - if (profile_b.regular.windows || profile_b.regular.downloads) - result += base::StringPrintf("Regular profile B: (%d w, %d d), ", - profile_b.regular.windows, - profile_b.regular.downloads); - if (profile_b.incognito.windows || profile_b.incognito.downloads) - result += base::StringPrintf("Incognito profile B: (%d w, %d d), ", - profile_b.incognito.windows, - profile_b.incognito.downloads); - result += (window_to_probe == REGULAR ? "Probe regular" : - window_to_probe == INCOGNITO ? "Probe incognito" : - "Probe unknown"); - result += "} -> "; - if (type == Browser::DOWNLOAD_CLOSE_OK) { - result += "No warning"; - } else { - result += base::StringPrintf( - "%s (%d downloads) warning", - (type == Browser::DOWNLOAD_CLOSE_BROWSER_SHUTDOWN ? "Browser shutdown" : - type == Browser::DOWNLOAD_CLOSE_LAST_WINDOW_IN_INCOGNITO_PROFILE ? - "Incognito close" : "Unknown"), - num_blocking); - } - return result; -} - -// The following test is split into three chunks to reduce the chance -// of hitting the 25s timeout. - -IN_PROC_BROWSER_TEST_F(BrowserCloseTest, DownloadsCloseCheck_0) { - ASSERT_TRUE(SetupForDownloadCloseCheck()); - for (size_t i = 0; i < arraysize(download_close_check_cases) / 6; ++i) { - ExecuteDownloadCloseCheckCase(i); - } -} - -// This test is disabled on Mac, see http://crbug.com/100566 -#if defined(OS_MACOSX) -#define MAYBE_DownloadsCloseCheck_1 DISABLED_DownloadsCloseCheck_1 -#else -#define MAYBE_DownloadsCloseCheck_1 DownloadsCloseCheck_1 -#endif - -IN_PROC_BROWSER_TEST_F(BrowserCloseTest, MAYBE_DownloadsCloseCheck_1) { - ASSERT_TRUE(SetupForDownloadCloseCheck()); - for (size_t i = arraysize(download_close_check_cases) / 6; - i < 2 * arraysize(download_close_check_cases) / 6; ++i) { - ExecuteDownloadCloseCheckCase(i); - } -} - -IN_PROC_BROWSER_TEST_F(BrowserCloseTest, DownloadsCloseCheck_2) { - ASSERT_TRUE(SetupForDownloadCloseCheck()); - for (size_t i = 2 * arraysize(download_close_check_cases) / 6; - i < 3 * arraysize(download_close_check_cases) / 6; ++i) { - ExecuteDownloadCloseCheckCase(i); - } -} - -// This test is disabled, see http://crbug.com/100517 -IN_PROC_BROWSER_TEST_F(BrowserCloseTest, DISABLED_DownloadsCloseCheck_3) { - ASSERT_TRUE(SetupForDownloadCloseCheck()); - for (size_t i = 3 * arraysize(download_close_check_cases) / 6; - i < 4 * arraysize(download_close_check_cases) / 6; ++i) { - ExecuteDownloadCloseCheckCase(i); - } -} - -IN_PROC_BROWSER_TEST_F(BrowserCloseTest, DownloadsCloseCheck_4) { - ASSERT_TRUE(SetupForDownloadCloseCheck()); - for (size_t i = 4 * arraysize(download_close_check_cases) / 6; - i < 5 * arraysize(download_close_check_cases) / 6; ++i) { - ExecuteDownloadCloseCheckCase(i); - } -} - -IN_PROC_BROWSER_TEST_F(BrowserCloseTest, DownloadsCloseCheck_5) { - ASSERT_TRUE(SetupForDownloadCloseCheck()); - for (size_t i = 5 * arraysize(download_close_check_cases) / 6; - i < 6 * arraysize(download_close_check_cases) / 6; ++i) { - ExecuteDownloadCloseCheckCase(i); - } -} diff --git a/chrome/browser/ui/browser_list.cc b/chrome/browser/ui/browser_list.cc index faa1356..be11579 100644 --- a/chrome/browser/ui/browser_list.cc +++ b/chrome/browser/ui/browser_list.cc @@ -10,7 +10,6 @@ #include "build/build_config.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_shutdown.h" -#include "chrome/browser/download/download_service.h" #include "chrome/browser/metrics/thread_watcher.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/printing/background_printing_manager.h" @@ -188,17 +187,15 @@ printing::BackgroundPrintingManager* GetBackgroundPrintingManager() { // This currently checks if there is pending download, or if it needs to // handle unload handler. bool AreAllBrowsersCloseable() { - BrowserList::const_iterator browser_it = BrowserList::begin(); - if (browser_it == BrowserList::end()) - return true; - - // If there are any downloads active, all browsers are not closeable. - if (DownloadService::DownloadCountAllProfiles() > 0) - return false; - - // Check TabsNeedBeforeUnloadFired(). - for (; browser_it != BrowserList::end(); ++browser_it) { - if ((*browser_it)->TabsNeedBeforeUnloadFired()) + for (BrowserList::const_iterator i = BrowserList::begin(); + i != BrowserList::end(); ++i) { + bool normal_downloads_are_present = false; + bool incognito_downloads_are_present = false; + (*i)->CheckDownloadsInProgress(&normal_downloads_are_present, + &incognito_downloads_are_present); + if (normal_downloads_are_present || + incognito_downloads_are_present || + (*i)->TabsNeedBeforeUnloadFired()) return false; } return true; diff --git a/chrome/browser/ui/gtk/download/download_in_progress_dialog_gtk.cc b/chrome/browser/ui/gtk/download/download_in_progress_dialog_gtk.cc index 43d4600..289b585 100644 --- a/chrome/browser/ui/gtk/download/download_in_progress_dialog_gtk.cc +++ b/chrome/browser/ui/gtk/download/download_in_progress_dialog_gtk.cc @@ -21,18 +21,9 @@ DownloadInProgressDialogGtk::DownloadInProgressDialogGtk(Browser* browser) : browser_(browser) { - int download_count; - Browser::DownloadClosePreventionType type = - browser_->OkToCloseWithInProgressDownloads(&download_count); - - // This dialog should have been created within the same thread invocation - // as the original test that lead to us, so it should always not be ok - // to close. - DCHECK_NE(Browser::DOWNLOAD_CLOSE_OK, type); - - // TODO(rdsmith): This dialog should be different depending on whether we're - // closing the last incognito window of a profile or doing browser shutdown. - // See http://crbug.com/88421. + int download_count = + DownloadServiceFactory::GetForProfile( + browser->profile())->GetDownloadManager()->in_progress_count(); std::string warning_text; std::string explanation_text; diff --git a/chrome/browser/ui/views/download/download_in_progress_dialog_view.cc b/chrome/browser/ui/views/download/download_in_progress_dialog_view.cc index 933460a..5fe1b6c 100644 --- a/chrome/browser/ui/views/download/download_in_progress_dialog_view.cc +++ b/chrome/browser/ui/views/download/download_in_progress_dialog_view.cc @@ -27,18 +27,9 @@ DownloadInProgressDialogView::DownloadInProgressDialogView(Browser* browser) : browser_(browser), product_name_(l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)) { - int download_count; - Browser::DownloadClosePreventionType type = - browser_->OkToCloseWithInProgressDownloads(&download_count); - - // This dialog should have been created within the same thread invocation - // as the original test that lead to us, so it should always not be ok - // to close. - DCHECK_NE(Browser::DOWNLOAD_CLOSE_OK, type); - - // TODO(rdsmith): This dialog should be different depending on whether we're - // closing the last incognito window of a profile or doing browser shutdown. - // See http://crbug.com/88421. + int download_count = + DownloadServiceFactory::GetForProfile( + browser->profile())->GetDownloadManager()->in_progress_count(); string16 warning_text; string16 explanation_text; |