diff options
| -rw-r--r-- | chrome/browser/download/download_browsertest.cc | 132 | ||||
| -rw-r--r-- | chrome/browser/download/download_extension_test.cc | 25 | ||||
| -rw-r--r-- | chrome/browser/download/download_test_observer.cc | 85 | ||||
| -rw-r--r-- | chrome/browser/download/download_test_observer.h | 88 | ||||
| -rw-r--r-- | chrome/browser/ui/browser_close_browsertest.cc | 11 |
5 files changed, 256 insertions, 85 deletions
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc index 448ef24..51e541c 100644 --- a/chrome/browser/download/download_browsertest.cc +++ b/chrome/browser/download/download_browsertest.cc @@ -335,45 +335,56 @@ class DownloadTest : public InProcessBrowserTest { return GetDownloadPrefs(browser)->download_path(); } - // Create a DownloadTestObserver that will wait for the + // Create a DownloadTestObserverTerminal that will wait for the // specified number of downloads to finish. DownloadTestObserver* CreateWaiter(Browser* browser, int num_downloads) { DownloadManager* download_manager = DownloadManagerForBrowser(browser); - return new DownloadTestObserver( + return new DownloadTestObserverTerminal( download_manager, num_downloads, - DownloadItem::COMPLETE, // Really done true, // Bail on select file DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL); } - // Create a DownloadTestObserver that will wait for the + // Create a DownloadTestObserverInProgress that will wait for the // specified number of downloads to start. DownloadTestObserver* CreateInProgressWaiter(Browser* browser, - int num_downloads) { + int num_downloads) { DownloadManager* download_manager = DownloadManagerForBrowser(browser); - return new DownloadTestObserver( - download_manager, num_downloads, - DownloadItem::IN_PROGRESS, // Has started - true, // Bail on select file - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL); + return new DownloadTestObserverInProgress( + download_manager, num_downloads, true); // Bail on select file. } - // Create a DownloadTestObserver that will wait for the + // Create a DownloadTestObserverTerminal that will wait for the // specified number of downloads to finish, or for // a dangerous download warning to be shown. DownloadTestObserver* DangerousInstallWaiter( Browser* browser, int num_downloads, - DownloadItem::DownloadState final_state, DownloadTestObserver::DangerousDownloadAction dangerous_download_action) { DownloadManager* download_manager = DownloadManagerForBrowser(browser); - return new DownloadTestObserver( + return new DownloadTestObserverTerminal( download_manager, num_downloads, - final_state, true, // Bail on select file dangerous_download_action); } + void CheckDownloadStatesForBrowser(Browser* browser, + size_t num, + DownloadItem::DownloadState state) { + std::vector<DownloadItem*> download_items; + GetDownloads(browser, &download_items); + + EXPECT_EQ(num, download_items.size()); + + for (size_t i = 0; i < download_items.size(); ++i) { + EXPECT_EQ(state, download_items[i]->GetState()) << " Item " << i; + } + } + + void CheckDownloadStates(size_t num, DownloadItem::DownloadState state) { + CheckDownloadStatesForBrowser(browser(), num, state); + } + // Download |url|, then wait for the download to finish. // |disposition| indicates where the navigation occurs (current tab, new // foreground tab, etc). @@ -398,6 +409,7 @@ class DownloadTest : public InProcessBrowserTest { browser_test_flags); // Waits for the download to complete. observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); // If specified, check the state of the select file dialog. if (expectation != EXPECT_NOTHING) { @@ -510,6 +522,8 @@ class DownloadTest : public InProcessBrowserTest { NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStatesForBrowser(browser, 1, DownloadItem::COMPLETE); EXPECT_EQ(2, browser->tab_count()); @@ -640,12 +654,9 @@ class DownloadTest : public InProcessBrowserTest { DownloadManager* download_manager = DownloadManagerForBrowser(browser()); scoped_ptr<DownloadTestObserver> observer( - new DownloadTestObserver( + new DownloadTestObserverTerminal( download_manager, 1, - download_info.reason == content::DOWNLOAD_INTERRUPT_REASON_NONE ? - DownloadItem::COMPLETE : // Really done - DownloadItem::INTERRUPTED, true, // Bail on select file DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); @@ -665,8 +676,14 @@ class DownloadTest : public InProcessBrowserTest { 1); } - if (download_info.show_download_item) + if (download_info.show_download_item) { observer->WaitForFinished(); + DownloadItem::DownloadState final_state = + (download_info.reason == content::DOWNLOAD_INTERRUPT_REASON_NONE) ? + DownloadItem::COMPLETE : + DownloadItem::INTERRUPTED; + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(final_state)); + } // Validate that the correct file was downloaded. download_items.clear(); @@ -745,16 +762,17 @@ 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( + new DownloadTestObserverTerminal( DownloadManagerForBrowser(browser()), 1, - DownloadItem::COMPLETE, // Really done false, // Continue on select file. DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); ui_test_utils::NavigateToURLWithDisposition( browser(), url, CURRENT_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(1, DownloadItem::COMPLETE); EXPECT_TRUE(observer->select_file_dialog_seen()); // Check state. @@ -1272,6 +1290,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, MultiDownload) { NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); observer2->WaitForFinished(); // Wait for the third request. + EXPECT_EQ(1u, observer2->NumDownloadsSeenInState(DownloadItem::COMPLETE)); // Get the important info from other threads and check it. EXPECT_TRUE(DownloadManager::EnsureNoPendingDownloadsForTesting()); @@ -1297,12 +1316,12 @@ 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 - // 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 - // in the IN_PROGRESS state and made known to the DownloadManager + // DownloadTestObserverInProgress 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 + // DownloadTestObserverInProgress, 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 // ModelChanged() event the code will currently fire is in @@ -1445,6 +1464,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, AnchorDownloadTag) { scoped_ptr<DownloadTestObserver> observer(CreateWaiter(browser(), 1)); ui_test_utils::NavigateToURL(browser(), url); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(1, DownloadItem::COMPLETE); // Confirm the downloaded data exists. FilePath downloaded_file = GetDownloadDirectory(browser()); @@ -1490,11 +1511,12 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxDenyInstall) { scoped_ptr<DownloadTestObserver> observer( DangerousInstallWaiter( - browser(), 1, DownloadItem::CANCELLED, + browser(), 1, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_DENY)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::CANCELLED)); EXPECT_EQ(1u, observer->NumDangerousDownloadsSeen()); // Download shelf should close. Download panel stays open on ChromeOS. @@ -1519,11 +1541,13 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxInstallDenysPermissions) { scoped_ptr<DownloadTestObserver> observer( DangerousInstallWaiter( - browser(), 1, DownloadItem::COMPLETE, + browser(), 1, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(1, DownloadItem::COMPLETE); EXPECT_EQ(1u, observer->NumDangerousDownloadsSeen()); // Download shelf should close. Download panel stays open on ChromeOS. @@ -1548,11 +1572,13 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxInstallAcceptPermissions) { scoped_ptr<DownloadTestObserver> observer( DangerousInstallWaiter( - browser(), 1, DownloadItem::COMPLETE, + browser(), 1, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(1, DownloadItem::COMPLETE); EXPECT_EQ(1u, observer->NumDangerousDownloadsSeen()); // Download shelf should close. Download panel stays open on ChromeOS. @@ -1578,11 +1604,13 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxInvalid) { scoped_ptr<DownloadTestObserver> observer( DangerousInstallWaiter( - browser(), 1, DownloadItem::COMPLETE, + browser(), 1, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(1, DownloadItem::COMPLETE); // Check that the extension was not installed. ExtensionService* extension_service = @@ -1602,11 +1630,13 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxLargeTheme) { scoped_ptr<DownloadTestObserver> observer( DangerousInstallWaiter( - browser(), 1, DownloadItem::COMPLETE, + browser(), 1, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT)); ui_test_utils::NavigateToURL(browser(), extension_url); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(1, DownloadItem::COMPLETE); EXPECT_EQ(1u, observer->NumDangerousDownloadsSeen()); // Download shelf should close. Download panel stays open on ChromeOS. @@ -1763,16 +1793,17 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadUrl) { ASSERT_TRUE(web_contents); DownloadTestObserver* observer( - new DownloadTestObserver( - DownloadManagerForBrowser(browser()), 1, - DownloadItem::COMPLETE, // Really done - false, // Ignore select file. - DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); + new DownloadTestObserverTerminal( + DownloadManagerForBrowser(browser()), 1, + false, // Ignore select file. + DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); DownloadSaveInfo save_info; save_info.prompt_for_save_location = true; DownloadManagerForBrowser(browser())->DownloadUrl( url, GURL(""), "", false, -1, save_info, web_contents); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(1, DownloadItem::COMPLETE); EXPECT_TRUE(observer->select_file_dialog_seen()); // Check state. @@ -1800,6 +1831,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadUrlToPath) { DownloadManagerForBrowser(browser())->DownloadUrl( url, GURL(""), "", false, -1, save_info, web_contents); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); // Check state. EXPECT_EQ(1, browser()->tab_count()); @@ -1832,11 +1864,13 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaGet) { // reachable. ASSERT_TRUE(test_server()->Stop()); scoped_ptr<DownloadTestObserver> waiter( - new DownloadTestObserver( - DownloadManagerForBrowser(browser()), 1, DownloadItem::COMPLETE, + new DownloadTestObserverTerminal( + DownloadManagerForBrowser(browser()), 1, false, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); browser()->SavePage(); waiter->WaitForFinished(); + EXPECT_EQ(1u, waiter->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(1, DownloadItem::COMPLETE); // Validate that the correct file was downloaded. GetDownloads(browser(), &download_items); @@ -1846,8 +1880,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaGet) { // Try to download it via a context menu. scoped_ptr<DownloadTestObserver> waiter_context_menu( - new DownloadTestObserver( - DownloadManagerForBrowser(browser()), 1, DownloadItem::COMPLETE, + new DownloadTestObserverTerminal( + DownloadManagerForBrowser(browser()), 1, false, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); content::ContextMenuParams context_menu_params; context_menu_params.media_type = WebKit::WebContextMenuData::MediaTypeImage; @@ -1858,6 +1892,9 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaGet) { menu.Init(); menu.ExecuteCommand(IDC_CONTENT_CONTEXT_SAVEIMAGEAS); waiter_context_menu->WaitForFinished(); + EXPECT_EQ( + 1u, waiter_context_menu->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(2, DownloadItem::COMPLETE); // Validate that the correct file was downloaded via the context menu. download_items.clear(); @@ -1907,11 +1944,13 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaPost) { // rather than "POST". ASSERT_TRUE(test_server()->Stop()); scoped_ptr<DownloadTestObserver> waiter( - new DownloadTestObserver( - DownloadManagerForBrowser(browser()), 1, DownloadItem::COMPLETE, + new DownloadTestObserverTerminal( + DownloadManagerForBrowser(browser()), 1, false, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); browser()->SavePage(); waiter->WaitForFinished(); + EXPECT_EQ(1u, waiter->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(1, DownloadItem::COMPLETE); // Validate that the correct file was downloaded. GetDownloads(browser(), &download_items); @@ -1921,8 +1960,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaPost) { // Try to download it via a context menu. scoped_ptr<DownloadTestObserver> waiter_context_menu( - new DownloadTestObserver( - DownloadManagerForBrowser(browser()), 1, DownloadItem::COMPLETE, + new DownloadTestObserverTerminal( + DownloadManagerForBrowser(browser()), 1, false, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL)); content::ContextMenuParams context_menu_params; context_menu_params.media_type = WebKit::WebContextMenuData::MediaTypeImage; @@ -1932,6 +1971,9 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaPost) { menu.Init(); menu.ExecuteCommand(IDC_CONTENT_CONTEXT_SAVEIMAGEAS); waiter_context_menu->WaitForFinished(); + EXPECT_EQ( + 1u, waiter_context_menu->NumDownloadsSeenInState(DownloadItem::COMPLETE)); + CheckDownloadStates(2, DownloadItem::COMPLETE); // Validate that the correct file was downloaded via the context menu. download_items.clear(); diff --git a/chrome/browser/download/download_extension_test.cc b/chrome/browser/download/download_extension_test.cc index d4e9081..7257f1e 100644 --- a/chrome/browser/download/download_extension_test.cc +++ b/chrome/browser/download/download_extension_test.cc @@ -59,12 +59,14 @@ class DownloadExtensionTest : public InProcessBrowserTest { size_t count, DownloadManager::DownloadVector* items) { for (size_t i = 0; i < count; ++i) { scoped_ptr<DownloadTestObserver> observer( - CreateDownloadObserver(1, DownloadItem::IN_PROGRESS)); + CreateInProgressDownloadObserver(1)); GURL slow_download_url(URLRequestSlowDownloadJob::kUnknownSizeUrl); ui_test_utils::NavigateToURLWithDisposition( browser(), slow_download_url, CURRENT_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); observer->WaitForFinished(); + EXPECT_EQ( + 1u, observer->NumDownloadsSeenInState(DownloadItem::IN_PROGRESS)); // We don't expect a select file dialog. CHECK(!observer->select_file_dialog_seen()); } @@ -74,7 +76,7 @@ class DownloadExtensionTest : public InProcessBrowserTest { DownloadItem* CreateSlowTestDownload() { scoped_ptr<DownloadTestObserver> observer( - CreateDownloadObserver(1, DownloadItem::IN_PROGRESS)); + CreateInProgressDownloadObserver(1)); GURL slow_download_url(URLRequestSlowDownloadJob::kUnknownSizeUrl); DownloadManager* manager = GetDownloadManager(); @@ -87,6 +89,7 @@ class DownloadExtensionTest : public InProcessBrowserTest { ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::IN_PROGRESS)); // We don't expect a select file dialog. if (observer->select_file_dialog_seen()) return NULL; @@ -108,22 +111,28 @@ class DownloadExtensionTest : public InProcessBrowserTest { void FinishPendingSlowDownloads() { scoped_ptr<DownloadTestObserver> observer( - CreateDownloadObserver(1, DownloadItem::COMPLETE)); + CreateDownloadObserver(1)); GURL finish_url(URLRequestSlowDownloadJob::kFinishDownloadUrl); ui_test_utils::NavigateToURLWithDisposition( browser(), finish_url, NEW_FOREGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); observer->WaitForFinished(); + EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE)); } - DownloadTestObserver* CreateDownloadObserver( - size_t download_count, - DownloadItem::DownloadState finished_state) { - return new DownloadTestObserver( - GetDownloadManager(), download_count, finished_state, true, + DownloadTestObserver* CreateDownloadObserver(size_t download_count) { + return new DownloadTestObserverTerminal( + GetDownloadManager(), download_count, true, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL); } + DownloadTestObserver* CreateInProgressDownloadObserver( + size_t download_count) { + return new DownloadTestObserverInProgress(GetDownloadManager(), + download_count, + true); + } + bool RunFunction(UIThreadExtensionFunction* function, const std::string& args) { // extension_function_test_utils::RunFunction() does not take diff --git a/chrome/browser/download/download_test_observer.cc b/chrome/browser/download/download_test_observer.cc index 0cbcd0c..9d8ca88 100644 --- a/chrome/browser/download/download_test_observer.cc +++ b/chrome/browser/download/download_test_observer.cc @@ -19,9 +19,9 @@ using content::DownloadManager; // 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. +// 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, @@ -42,21 +42,15 @@ void DenyDangerousDownload(scoped_refptr<DownloadManager> download_manager, 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() { @@ -67,6 +61,12 @@ DownloadTestObserver::~DownloadTestObserver() { download_manager_->RemoveObserver(this); } +void DownloadTestObserver::Init() { + download_manager_->AddObserver(this); // Will call initial ModelChanged(). + finished_downloads_at_construction_ = finished_downloads_.size(); + states_observed_.clear(); +} + void DownloadTestObserver::WaitForFinished() { if (!IsFinished()) { waiting_ = true; @@ -129,9 +129,8 @@ void DownloadTestObserver::OnDownloadUpdated(DownloadItem* download) { } } - if (download->GetState() == download_finished_state_) { + if (IsDownloadInFinalState(download)) DownloadInFinalState(download); - } } void DownloadTestObserver::ModelChanged(DownloadManager* manager) { @@ -182,15 +181,28 @@ size_t DownloadTestObserver::NumDangerousDownloadsSeen() const { return dangerous_downloads_seen_.size(); } +size_t DownloadTestObserver::NumDownloadsSeenInState( + content::DownloadItem::DownloadState state) const { + StateMap::const_iterator it = states_observed_.find(state); + + if (it == states_observed_.end()) + return 0; + + return it->second; +} + void DownloadTestObserver::DownloadInFinalState(DownloadItem* download) { if (finished_downloads_.find(download) != finished_downloads_.end()) { - // We've already seen terminal state on this download. + // We've already seen the final state on this download. return; } // Record the transition. finished_downloads_.insert(download); + // Record the state. + states_observed_[download->GetState()]++; // Initializes to 0 the first time. + SignalIfFinished(); } @@ -199,6 +211,55 @@ void DownloadTestObserver::SignalIfFinished() { MessageLoopForUI::current()->Quit(); } +DownloadTestObserverTerminal::DownloadTestObserverTerminal( + content::DownloadManager* download_manager, + size_t wait_count, + bool finish_on_select_file, + DangerousDownloadAction dangerous_download_action) + : DownloadTestObserver(download_manager, + wait_count, + finish_on_select_file, + dangerous_download_action) { + // You can't rely on overriden virtual functions in a base class constructor; + // the virtual function table hasn't been set up yet. So, we have to do any + // work that depends on those functions in the derived class constructor + // instead. In this case, it's because of |IsDownloadInFinalState()|. + Init(); +} + +DownloadTestObserverTerminal::~DownloadTestObserverTerminal() { +} + + +bool DownloadTestObserverTerminal::IsDownloadInFinalState( + content::DownloadItem* download) { + return (download->GetState() != DownloadItem::IN_PROGRESS); +} + +DownloadTestObserverInProgress::DownloadTestObserverInProgress( + content::DownloadManager* download_manager, + size_t wait_count, + bool finish_on_select_file) + : DownloadTestObserver(download_manager, + wait_count, + finish_on_select_file, + ON_DANGEROUS_DOWNLOAD_ACCEPT) { + // You can't override virtual functions in a base class constructor; the + // virtual function table hasn't been set up yet. So, we have to do any + // work that depends on those functions in the derived class constructor + // instead. In this case, it's because of |IsDownloadInFinalState()|. + Init(); +} + +DownloadTestObserverInProgress::~DownloadTestObserverInProgress() { +} + + +bool DownloadTestObserverInProgress::IsDownloadInFinalState( + content::DownloadItem* download) { + return (download->GetState() == DownloadItem::IN_PROGRESS); +} + DownloadTestFlushObserver::DownloadTestFlushObserver( DownloadManager* download_manager) : download_manager_(download_manager), diff --git a/chrome/browser/download/download_test_observer.h b/chrome/browser/download/download_test_observer.h index 3404f42..1a3a2a0 100644 --- a/chrome/browser/download/download_test_observer.h +++ b/chrome/browser/download/download_test_observer.h @@ -13,12 +13,12 @@ #include "content/public/browser/download_item.h" #include "content/public/browser/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. +// Detects changes to the downloads after construction. +// Finishes when one of the following happens: +// - A specified number of downloads change to a terminal state (defined +// in derived classes). +// - Specific events, such as a select file dialog. +// 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 @@ -36,17 +36,13 @@ class DownloadTestObserver : public content::DownloadManager::Observer, }; // Create an object that will be considered finished when |wait_count| - // download items have entered state |download_finished_state|. + // download items have entered a terminal 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( content::DownloadManager* download_manager, size_t wait_count, - content::DownloadItem::DownloadState download_finished_state, bool finish_on_select_file, DangerousDownloadAction dangerous_download_action); @@ -55,7 +51,7 @@ class DownloadTestObserver : public content::DownloadManager::Observer, // State accessors. bool select_file_dialog_seen() const { return select_file_dialog_seen_; } - // Wait for whatever state was specified in the constructor. + // Wait for the requested number of downloads to enter a terminal state. void WaitForFinished(); // Return true if everything's happened that we're configured for. @@ -73,9 +69,22 @@ class DownloadTestObserver : public content::DownloadManager::Observer, size_t NumDangerousDownloadsSeen() const; + size_t NumDownloadsSeenInState( + content::DownloadItem::DownloadState state) const; + + protected: + // Only to be called by derived classes' constructors. + virtual void Init(); + + // Called to see if a download item is in a final state. + virtual bool IsDownloadInFinalState(content::DownloadItem* download) = 0; + private: typedef std::set<content::DownloadItem*> DownloadSet; + // Maps states to the number of times they have been encountered + typedef std::map<content::DownloadItem::DownloadState, size_t> StateMap; + // 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 @@ -97,6 +106,12 @@ class DownloadTestObserver : public content::DownloadManager::Observer, // on a DownloadItem, we'll stop observing it. DownloadSet downloads_observed_; + // The map of states to the number of times they have been observed since + // we started looking. + // Recorded at the time downloads_observed_ is recorded, but cleared in the + // constructor to exclude pre-existing states. + StateMap states_observed_; + // The number of downloads to wait on completing. size_t wait_count_; @@ -114,9 +129,6 @@ class DownloadTestObserver : public content::DownloadManager::Observer, // all downloads completing. bool waiting_; - // The state on which to consider the DownloadItem finished. - content::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_; @@ -133,6 +145,52 @@ class DownloadTestObserver : public content::DownloadManager::Observer, DISALLOW_COPY_AND_ASSIGN(DownloadTestObserver); }; +class DownloadTestObserverTerminal : public DownloadTestObserver { + public: + // Create an object that will be considered finished when |wait_count| + // download items have entered a terminal state (any but IN_PROGRESS). + // If |finish_on_select_file| is true, the object will also be + // considered finished if the DownloadManager raises a + // SelectFileDialogDisplayed() notification. + DownloadTestObserverTerminal( + content::DownloadManager* download_manager, + size_t wait_count, + bool finish_on_select_file, + DangerousDownloadAction dangerous_download_action); + + virtual ~DownloadTestObserverTerminal(); + + private: + virtual bool IsDownloadInFinalState(content::DownloadItem* download) OVERRIDE; + + DISALLOW_COPY_AND_ASSIGN(DownloadTestObserverTerminal); +}; + +// Detects changes to the downloads after construction. +// Finishes when a specified number of downloads change to the +// IN_PROGRESS state, or a Select File Dialog has appeared. +// Dangerous downloads are accepted. +// Callers may either probe for the finished state, or wait on it. +class DownloadTestObserverInProgress : public DownloadTestObserver { + public: + // Create an object that will be considered finished when |wait_count| + // download items have entered state |IN_PROGRESS|. + // If |finish_on_select_file| is true, the object will also be + // considered finished if the DownloadManager raises a + // SelectFileDialogDisplayed() notification. + DownloadTestObserverInProgress( + content::DownloadManager* download_manager, + size_t wait_count, + bool finish_on_select_file); + + virtual ~DownloadTestObserverInProgress(); + + private: + virtual bool IsDownloadInFinalState(content::DownloadItem* download) OVERRIDE; + + DISALLOW_COPY_AND_ASSIGN(DownloadTestObserverInProgress); +}; + // The WaitForFlush() method on this class returns after: // * There are no IN_PROGRESS download items remaining on the // DownloadManager. diff --git a/chrome/browser/ui/browser_close_browsertest.cc b/chrome/browser/ui/browser_close_browsertest.cc index c0aacc6..5610a4f 100644 --- a/chrome/browser/ui/browser_close_browsertest.cc +++ b/chrome/browser/ui/browser_close_browsertest.cc @@ -115,13 +115,12 @@ class BrowserCloseTest : public InProcessBrowserTest { 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)); + new DownloadTestObserverInProgress(download_manager, + num_downloads, + true)); // Bail on select file. // Set of that number of downloads. + size_t count_downloads = num_downloads; while (num_downloads--) ui_test_utils::NavigateToURLWithDisposition( browser, url, NEW_BACKGROUND_TAB, @@ -129,6 +128,8 @@ class BrowserCloseTest : public InProcessBrowserTest { // Wait for them. observer->WaitForFinished(); + EXPECT_EQ(count_downloads, + observer->NumDownloadsSeenInState(DownloadItem::IN_PROGRESS)); } // All all downloads created in CreateStalledDownloads() to |