Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(130)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/download/download_browsertest.cc

Issue 9568003: Fixed issue with DownloadTestObserver. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fixed CLANG issue. Created 8 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/browser/download/download_extension_test.cc » ('j') | chrome/browser/download/download_test_observer.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/download/download_browsertest.cc
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
index 5874cce0183e983a3661edf6a6d6a4e237c14c6d..1e1a3687bdb1d49d16ce449122e0cec08262cb62 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -389,41 +389,37 @@ 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) {
+ DownloadTestObserverTerminal* 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);
+ DownloadTestObserverTerminal::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,
+ DownloadTestObserverInProgress* CreateInProgressWaiter(Browser* browser,
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);
}
- // 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(
+ DownloadTestObserverTerminal* DangerousInstallWaiter(
Browser* browser,
int num_downloads,
- DownloadItem::DownloadState final_state,
- DownloadTestObserver::DangerousDownloadAction dangerous_download_action) {
+ DownloadTestObserverTerminal::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);
}
@@ -443,7 +439,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<DownloadTestObserverTerminal> 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,
@@ -542,7 +538,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<DownloadTestObserverTerminal> observer(CreateWaiter(browser, 1));
ui_test_utils::NavigateToURL(browser, url);
// TODO(ahendrickson): check download status text before downloading.
@@ -693,15 +689,12 @@ class DownloadTest : public InProcessBrowserTest {
ASSERT_TRUE(url.is_valid());
DownloadManager* download_manager = DownloadManagerForBrowser(browser());
- scoped_ptr<DownloadTestObserver> observer(
- new DownloadTestObserver(
+ scoped_ptr<DownloadTestObserverTerminal> observer(
+ new DownloadTestObserverTerminal(
download_manager,
1,
Randy Smith (Not in Mondays) 2012/03/07 21:39:16 Shouldn't all the observers where the state has be
ahendrickson 2012/03/08 17:36:07 Done for all the non-IN_PROGRESS observers.
- download_info.reason == DOWNLOAD_INTERRUPT_REASON_NONE ?
- DownloadItem::COMPLETE : // Really done
- DownloadItem::INTERRUPTED,
true, // Bail on select file
- DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
+ DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_FAIL));
if (download_info.download_method == DOWNLOAD_DIRECT) {
// Go directly to download.
@@ -798,13 +791,12 @@ 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<DownloadTestObserverTerminal> observer(
+ new DownloadTestObserverTerminal(
DownloadManagerForBrowser(browser()),
1,
- DownloadItem::COMPLETE, // Really done
false, // Continue on select file.
- DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
+ DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_FAIL));
ui_test_utils::NavigateToURLWithDisposition(
browser(), url, CURRENT_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
@@ -1286,7 +1278,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, MultiDownload) {
// Create a download, wait until it's started, and confirm
// we're in the expected state.
- scoped_ptr<DownloadTestObserver> observer1(
+ scoped_ptr<DownloadTestObserverInProgress> observer1(
CreateInProgressWaiter(browser(), 1));
ui_test_utils::NavigateToURL(
browser(), GURL(URLRequestSlowDownloadJob::kUnknownSizeUrl));
@@ -1321,7 +1313,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, MultiDownload) {
// Allow the first request to finish. We do this by loading a third URL
// in a separate tab.
- scoped_ptr<DownloadTestObserver> observer2(CreateWaiter(browser(), 1));
+ scoped_ptr<DownloadTestObserverTerminal>
+ observer2(CreateWaiter(browser(), 1));
GURL finish_url(URLRequestSlowDownloadJob::kFinishDownloadUrl);
ui_test_utils::NavigateToURLWithDisposition(
browser(),
@@ -1356,12 +1349,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
@@ -1373,7 +1366,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<DownloadTestObserverInProgress> observer(
CreateInProgressWaiter(browser(), 1));
ui_test_utils::NavigateToURL(
browser(), GURL(URLRequestSlowDownloadJob::kUnknownSizeUrl));
@@ -1504,7 +1497,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<DownloadTestObserverTerminal> observer(CreateWaiter(browser(), 1));
ui_test_utils::NavigateToURL(browser(), url);
observer->WaitForFinished();
@@ -1550,10 +1543,10 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxDenyInstall) {
ASSERT_TRUE(InitialSetup(false));
GURL extension_url(URLRequestMockHTTPJob::GetMockUrl(kGoodCrxPath));
- scoped_ptr<DownloadTestObserver> observer(
+ scoped_ptr<DownloadTestObserverTerminal> observer(
DangerousInstallWaiter(
- browser(), 1, DownloadItem::CANCELLED,
- DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_DENY));
+ browser(), 1,
+ DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_DENY));
ui_test_utils::NavigateToURL(browser(), extension_url);
observer->WaitForFinished();
@@ -1579,10 +1572,10 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxInstallDenysPermissions) {
download_crx_util::SetMockInstallUIForTesting(
new MockAbortExtensionInstallUI());
- scoped_ptr<DownloadTestObserver> observer(
+ scoped_ptr<DownloadTestObserverTerminal> observer(
DangerousInstallWaiter(
- browser(), 1, DownloadItem::COMPLETE,
- DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT));
+ browser(), 1,
+ DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_ACCEPT));
ui_test_utils::NavigateToURL(browser(), extension_url);
observer->WaitForFinished();
@@ -1608,10 +1601,10 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxInstallAcceptPermissions) {
download_crx_util::SetMockInstallUIForTesting(
new MockAutoConfirmExtensionInstallUI(browser()->profile()));
- scoped_ptr<DownloadTestObserver> observer(
+ scoped_ptr<DownloadTestObserverTerminal> observer(
DangerousInstallWaiter(
- browser(), 1, DownloadItem::COMPLETE,
- DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT));
+ browser(), 1,
+ DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_ACCEPT));
ui_test_utils::NavigateToURL(browser(), extension_url);
observer->WaitForFinished();
@@ -1638,10 +1631,10 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxInvalid) {
download_crx_util::SetMockInstallUIForTesting(
new MockAutoConfirmExtensionInstallUI(browser()->profile()));
- scoped_ptr<DownloadTestObserver> observer(
+ scoped_ptr<DownloadTestObserverTerminal> observer(
DangerousInstallWaiter(
- browser(), 1, DownloadItem::COMPLETE,
- DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT));
+ browser(), 1,
+ DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_ACCEPT));
ui_test_utils::NavigateToURL(browser(), extension_url);
observer->WaitForFinished();
@@ -1662,10 +1655,10 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, CrxLargeTheme) {
download_crx_util::SetMockInstallUIForTesting(
new MockAutoConfirmExtensionInstallUI(browser()->profile()));
- scoped_ptr<DownloadTestObserver> observer(
+ scoped_ptr<DownloadTestObserverTerminal> observer(
DangerousInstallWaiter(
- browser(), 1, DownloadItem::COMPLETE,
- DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT));
+ browser(), 1,
+ DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_ACCEPT));
ui_test_utils::NavigateToURL(browser(), extension_url);
observer->WaitForFinished();
@@ -1824,12 +1817,11 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadUrl) {
WebContents* web_contents = browser()->GetSelectedWebContents();
ASSERT_TRUE(web_contents);
- DownloadTestObserver* observer(
- new DownloadTestObserver(
- DownloadManagerForBrowser(browser()), 1,
- DownloadItem::COMPLETE, // Really done
- false, // Ignore select file.
- DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
+ DownloadTestObserverTerminal* observer(
+ new DownloadTestObserverTerminal(
+ DownloadManagerForBrowser(browser()), 1,
+ false, // Ignore select file.
+ DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_FAIL));
DownloadSaveInfo save_info;
save_info.prompt_for_save_location = true;
DownloadManagerForBrowser(browser())->DownloadUrl(
@@ -1858,7 +1850,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadUrlToPath) {
DownloadSaveInfo save_info;
save_info.file_path = target_file_full_path;
- DownloadTestObserver* observer(CreateWaiter(browser(), 1));
+ DownloadTestObserverTerminal* observer(CreateWaiter(browser(), 1));
DownloadManagerForBrowser(browser())->DownloadUrl(
url, GURL(""), "", false, -1, save_info, web_contents);
observer->WaitForFinished();
@@ -1893,10 +1885,10 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaGet) {
// is not bypassed then this will fail since the server is no longer
// reachable.
ASSERT_TRUE(test_server()->Stop());
- scoped_ptr<DownloadTestObserver> waiter(
- new DownloadTestObserver(
- DownloadManagerForBrowser(browser()), 1, DownloadItem::COMPLETE,
- false, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
+ scoped_ptr<DownloadTestObserverTerminal> waiter(
+ new DownloadTestObserverTerminal(
+ DownloadManagerForBrowser(browser()), 1,
+ false, DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_FAIL));
browser()->SavePage();
waiter->WaitForFinished();
@@ -1905,12 +1897,13 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaGet) {
EXPECT_TRUE(waiter->select_file_dialog_seen());
ASSERT_EQ(1u, download_items.size());
ASSERT_EQ(url, download_items[0]->GetOriginalUrl());
+ EXPECT_EQ(DownloadItem::COMPLETE, download_items[0]->GetState());
// Try to download it via a context menu.
- scoped_ptr<DownloadTestObserver> waiter_context_menu(
- new DownloadTestObserver(
- DownloadManagerForBrowser(browser()), 1, DownloadItem::COMPLETE,
- false, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
+ scoped_ptr<DownloadTestObserverTerminal> waiter_context_menu(
+ new DownloadTestObserverTerminal(
+ DownloadManagerForBrowser(browser()), 1,
+ false, DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_FAIL));
content::ContextMenuParams context_menu_params;
context_menu_params.media_type = WebKit::WebContextMenuData::MediaTypeImage;
context_menu_params.src_url = url;
@@ -1925,9 +1918,11 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaGet) {
download_items.clear();
GetDownloads(browser(), &download_items);
EXPECT_TRUE(waiter_context_menu->select_file_dialog_seen());
+ EXPECT_EQ(DownloadItem::COMPLETE, download_items[0]->GetState());
ASSERT_EQ(2u, download_items.size());
ASSERT_EQ(url, download_items[0]->GetOriginalUrl());
ASSERT_EQ(url, download_items[1]->GetOriginalUrl());
+ EXPECT_EQ(DownloadItem::COMPLETE, download_items[1]->GetState());
}
IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaPost) {
@@ -1968,10 +1963,10 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaPost) {
// reachable. This will also fail if it tries to be retrieved via "GET"
// rather than "POST".
ASSERT_TRUE(test_server()->Stop());
- scoped_ptr<DownloadTestObserver> waiter(
- new DownloadTestObserver(
- DownloadManagerForBrowser(browser()), 1, DownloadItem::COMPLETE,
- false, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
+ scoped_ptr<DownloadTestObserverTerminal> waiter(
+ new DownloadTestObserverTerminal(
+ DownloadManagerForBrowser(browser()), 1,
+ false, DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_FAIL));
browser()->SavePage();
waiter->WaitForFinished();
@@ -1982,10 +1977,10 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SavePageNonHTMLViaPost) {
ASSERT_EQ(jpeg_url, download_items[0]->GetOriginalUrl());
// Try to download it via a context menu.
- scoped_ptr<DownloadTestObserver> waiter_context_menu(
- new DownloadTestObserver(
- DownloadManagerForBrowser(browser()), 1, DownloadItem::COMPLETE,
- false, DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
+ scoped_ptr<DownloadTestObserverTerminal> waiter_context_menu(
+ new DownloadTestObserverTerminal(
+ DownloadManagerForBrowser(browser()), 1,
+ false, DownloadTestObserverTerminal::ON_DANGEROUS_DOWNLOAD_FAIL));
content::ContextMenuParams context_menu_params;
context_menu_params.media_type = WebKit::WebContextMenuData::MediaTypeImage;
context_menu_params.src_url = jpeg_url;
« no previous file with comments | « no previous file | chrome/browser/download/download_extension_test.cc » ('j') | chrome/browser/download/download_test_observer.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698