Remove a couple racy functions from ui_test_utils.
Un-disable the ReservedAccelerators test. Now seems to pass linux debug.
Get rid of usage of WaitForNavigationInCurrentTab
[email protected]
BUG=69475
TEST=browser_tests.*,interactive_ui_tests.*,ui_tests.*
Review URL: http://codereview.chromium.org/7693013
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98093 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/browser_keyevents_browsertest.cc b/chrome/browser/browser_keyevents_browsertest.cc
index 2feeb61..0e8bb09 100644
--- a/chrome/browser/browser_keyevents_browsertest.cc
+++ b/chrome/browser/browser_keyevents_browsertest.cc
@@ -650,7 +650,7 @@
}
// Disabled, http://crbug.com/69475.
-IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, DISABLED_ReservedAccelerators) {
+IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, ReservedAccelerators) {
ASSERT_TRUE(test_server()->Start());
ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
@@ -680,10 +680,7 @@
// Press Ctrl/Cmd+T, which will open a new tab. It cannot be suppressed.
EXPECT_NO_FATAL_FAILURE(TestKeyEvent(0, kTestCtrlOrCmdT));
-
- ASSERT_NO_FATAL_FAILURE(
- wait_for_new_tab.WaitFor(Source<TabContentsWrapper>(
- browser()->GetTabContentsWrapperAt(1))));
+ wait_for_new_tab.Wait();
int result_length;
ASSERT_NO_FATAL_FAILURE(GetResultLength(0, &result_length));
diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index 960283157..1dcc950 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -10,6 +10,7 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/test/base/ui_test_utils.h"
@@ -417,8 +418,12 @@
// Crash the tab and reload it, chrome.app.isInstalled should still be true.
ui_test_utils::CrashTab(browser()->GetSelectedTabContents());
+ ui_test_utils::WindowedNotificationObserver observer(
+ content::NOTIFICATION_LOAD_STOP,
+ Source<NavigationController>(
+ &browser()->GetSelectedTabContentsWrapper()->controller()));
browser()->Reload(CURRENT_TAB);
- ASSERT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(browser()));
+ observer.Wait();
ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
contents->render_view_host(), L"",
L"window.domAutomationController.send(chrome.app.isInstalled)",
diff --git a/chrome/browser/extensions/extension_browsertests_misc.cc b/chrome/browser/extensions/extension_browsertests_misc.cc
index 22d3927..8a9e996 100644
--- a/chrome/browser/extensions/extension_browsertests_misc.cc
+++ b/chrome/browser/extensions/extension_browsertests_misc.cc
@@ -724,8 +724,14 @@
ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
tab->render_view_host(), L"", L"testPluginWorks()", &result));
EXPECT_FALSE(result);
- browser()->Reload(CURRENT_TAB);
- ui_test_utils::WaitForNavigationInCurrentTab(browser());
+ {
+ ui_test_utils::WindowedNotificationObserver observer(
+ content::NOTIFICATION_LOAD_STOP,
+ Source<NavigationController>(
+ &browser()->GetSelectedTabContentsWrapper()->controller()));
+ browser()->Reload(CURRENT_TAB);
+ observer.Wait();
+ }
ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
tab->render_view_host(), L"", L"testPluginWorks()", &result));
EXPECT_TRUE(result);
@@ -744,8 +750,14 @@
ASSERT_TRUE(LoadExtension(extension_dir));
EXPECT_EQ(size_before + 1, service->extensions()->size());
- browser()->Reload(CURRENT_TAB);
- ui_test_utils::WaitForNavigationInCurrentTab(browser());
+ {
+ ui_test_utils::WindowedNotificationObserver observer(
+ content::NOTIFICATION_LOAD_STOP,
+ Source<NavigationController>(
+ &browser()->GetSelectedTabContentsWrapper()->controller()));
+ browser()->Reload(CURRENT_TAB);
+ observer.Wait();
+ }
ASSERT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
tab->render_view_host(), L"", L"testPluginWorks()", &result));
EXPECT_TRUE(result);
diff --git a/chrome/browser/extensions/isolated_app_apitest.cc b/chrome/browser/extensions/isolated_app_apitest.cc
index 4ae8476..0571246 100644
--- a/chrome/browser/extensions/isolated_app_apitest.cc
+++ b/chrome/browser/extensions/isolated_app_apitest.cc
@@ -9,6 +9,7 @@
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/browser/renderer_host/browser_render_process_host.h"
@@ -115,8 +116,12 @@
// Check that isolation persists even if the tab crashes and is reloaded.
browser()->SelectNumberedTab(1);
ui_test_utils::CrashTab(tab1);
+ ui_test_utils::WindowedNotificationObserver observer(
+ content::NOTIFICATION_LOAD_STOP,
+ Source<NavigationController>(
+ &browser()->GetSelectedTabContentsWrapper()->controller()));
browser()->Reload(CURRENT_TAB);
- ASSERT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(browser()));
+ observer.Wait();
EXPECT_TRUE(HasCookie(tab1, "app1=3"));
EXPECT_FALSE(HasCookie(tab1, "app2"));
EXPECT_FALSE(HasCookie(tab1, "normalPage"));
diff --git a/chrome/browser/sessions/session_restore_browsertest.cc b/chrome/browser/sessions/session_restore_browsertest.cc
index cd4ffd8..96664786 100644
--- a/chrome/browser/sessions/session_restore_browsertest.cc
+++ b/chrome/browser/sessions/session_restore_browsertest.cc
@@ -121,11 +121,18 @@
// Add and navigate three tabs.
ui_test_utils::NavigateToURL(browser(), url1);
- browser()->AddSelectedTabWithURL(url2, PageTransition::LINK);
- ui_test_utils::WaitForNavigationInCurrentTab(browser());
-
- browser()->AddSelectedTabWithURL(url3, PageTransition::LINK);
- ui_test_utils::WaitForNavigationInCurrentTab(browser());
+ {
+ ui_test_utils::WindowedNotificationObserver observer(
+ content::NOTIFICATION_LOAD_STOP, NotificationService::AllSources());
+ browser()->AddSelectedTabWithURL(url2, PageTransition::LINK);
+ observer.Wait();
+ }
+ {
+ ui_test_utils::WindowedNotificationObserver observer(
+ content::NOTIFICATION_LOAD_STOP, NotificationService::AllSources());
+ browser()->AddSelectedTabWithURL(url3, PageTransition::LINK);
+ observer.Wait();
+ }
TabRestoreService* service =
TabRestoreServiceFactory::GetForProfile(browser()->profile());
diff --git a/chrome/browser/ui/browser_navigator_browsertest_chromeos.cc b/chrome/browser/ui/browser_navigator_browsertest_chromeos.cc
index 02a75606..085e079 100644
--- a/chrome/browser/ui/browser_navigator_browsertest_chromeos.cc
+++ b/chrome/browser/ui/browser_navigator_browsertest_chromeos.cc
@@ -63,11 +63,9 @@
browser::NavigateParams p(MakeNavigateParams());
p.disposition = NEW_POPUP;
p.window_bounds = gfx::Rect(0, 0, 10000, 10000);
- browser::Navigate(&p);
- // Wait for page to load.
- ui_test_utils::WaitForNavigationInCurrentTab(p.browser);
+ ui_test_utils::NavigateToURL(&p);
- // Navigate() should have opened a new tab.
+ // NavigateToURL() should have opened a new tab.
EXPECT_EQ(browser(), p.browser);
// We should have one window with two tabs.
@@ -83,21 +81,15 @@
browser::NavigateParams p1(MakeNavigateParams());
p1.disposition = NEW_POPUP;
p1.window_bounds = gfx::Rect(0, 0, 200, 200);
- browser::Navigate(&p1);
-
- // Wait for page to load.
- ui_test_utils::WaitForNavigationInCurrentTab(p1.browser);
+ ui_test_utils::NavigateToURL(&p1);
// Open a large popup from the popup.
browser::NavigateParams p2(MakeNavigateParams(p1.browser));
p2.disposition = NEW_POPUP;
p2.window_bounds = gfx::Rect(0, 0, 10000, 10000);
- browser::Navigate(&p2);
+ ui_test_utils::NavigateToURL(&p2);
- // Wait for page to load.
- ui_test_utils::WaitForNavigationInCurrentTab(p2.browser);
-
- // Navigate() should have opened a new tab in the primary browser.
+ // NavigateToURL() should have opened a new tab in the primary browser.
EXPECT_EQ(browser(), p2.browser);
// We should have two windows. browser() should have two tabs.
diff --git a/chrome/browser/ui/views/find_bar_host_interactive_uitest.cc b/chrome/browser/ui/views/find_bar_host_interactive_uitest.cc
index e05b49fb..84feac80 100644
--- a/chrome/browser/ui/views/find_bar_host_interactive_uitest.cc
+++ b/chrome/browser/ui/views/find_bar_host_interactive_uitest.cc
@@ -162,8 +162,10 @@
EXPECT_TRUE(ASCIIToUTF16("a") == find_bar->GetFindSelectedText());
// Open another tab (tab B).
+ ui_test_utils::WindowedNotificationObserver observer(
+ content::NOTIFICATION_LOAD_STOP, NotificationService::AllSources());
browser()->AddSelectedTabWithURL(url, PageTransition::TYPED);
- ASSERT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(browser()));
+ observer.Wait();
// Make sure Find box is open.
browser()->Find();
diff --git a/chrome/test/base/ui_test_utils.cc b/chrome/test/base/ui_test_utils.cc
index 5140b22a..d087e68 100644
--- a/chrome/test/base/ui_test_utils.cc
+++ b/chrome/test/base/ui_test_utils.cc
@@ -252,27 +252,6 @@
return true;
}
-bool WaitForNavigationInCurrentTab(Browser* browser) {
- TabContents* tab_contents = browser->GetSelectedTabContents();
- if (!tab_contents)
- return false;
- WaitForNavigation(&tab_contents->controller());
- return true;
-}
-
-bool WaitForNavigationsInCurrentTab(Browser* browser,
- int number_of_navigations) {
- TabContents* tab_contents = browser->GetSelectedTabContents();
- if (!tab_contents)
- return false;
- WaitForNavigations(&tab_contents->controller(), number_of_navigations);
- return true;
-}
-
-void WaitForNavigation(NavigationController* controller) {
- WaitForNavigations(controller, 1);
-}
-
void WaitForNavigations(NavigationController* controller,
int number_of_navigations) {
TestNavigationObserver observer(
@@ -280,6 +259,10 @@
observer.WaitForObservation();
}
+void WaitForNavigation(NavigationController* controller) {
+ WaitForNavigations(controller, 1);
+}
+
void WaitForNewTab(Browser* browser) {
TestNotificationObserver observer;
RegisterAndWait(&observer, content::NOTIFICATION_TAB_ADDED,
@@ -813,20 +796,6 @@
ui_test_utils::RunMessageLoop();
}
-void WindowedNotificationObserver::WaitFor(const NotificationSource& source) {
- if (waiting_for_ != NotificationService::AllSources()) {
- LOG(FATAL) << "WaitFor called when already waiting on a specific source."
- << "Use Wait in this case.";
- }
-
- waiting_for_ = source;
- if (sources_seen_.count(waiting_for_.map_key()) > 0)
- return;
-
- running_ = true;
- ui_test_utils::RunMessageLoop();
-}
-
void WindowedNotificationObserver::Observe(int type,
const NotificationSource& source,
const NotificationDetails& details) {
diff --git a/chrome/test/base/ui_test_utils.h b/chrome/test/base/ui_test_utils.h
index 201f82f7..07a4311 100644
--- a/chrome/test/base/ui_test_utils.h
+++ b/chrome/test/base/ui_test_utils.h
@@ -88,33 +88,17 @@
// Puts the current tab title in |title|. Returns true on success.
bool GetCurrentTabTitle(const Browser* browser, string16* title);
-// Waits for the current tab to complete the navigation. Returns true on
-// success. TODO(gbillock): remove this race hazard.
-// Use WindowedNotificationObserver instead.
-bool WaitForNavigationInCurrentTab(Browser* browser);
-
-// Waits for the current tab to complete the specified number of navigations.
-// Returns true on success. TODO(gbillock): remove this race hazard.
-// Use WindowedNotificationObserver instead.
-bool WaitForNavigationsInCurrentTab(Browser* browser,
- int number_of_navigations);
-
// Waits for |controller| to complete a navigation. This blocks until
// the navigation finishes. TODO(gbillock): remove this race hazard.
// Use WindowedNotificationObserver instead.
void WaitForNavigation(NavigationController* controller);
-// Waits for |controller| to complete a navigation. This blocks until
-// the specified number of navigations complete. TODO(gbillock): remove this
-// race hazard.
-// Use WindowedNotificationObserver instead.
-void WaitForNavigations(NavigationController* controller,
- int number_of_navigations);
-
-// Waits for a new tab to be added to |browser|.
+// Waits for a new tab to be added to |browser|. TODO(gbillock): remove this
+// race hazard. Use WindowedNotificationObserver instead.
void WaitForNewTab(Browser* browser);
-// Waits for a |browser_action| to be updated.
+// Waits for a |browser_action| to be updated. TODO(gbillock): remove this race
+// hazard. Use WindowedNotificationObserver instead.
void WaitForBrowserActionUpdated(ExtensionAction* browser_action);
// Waits for a load stop for the specified |tab|'s controller, if the tab is
@@ -416,26 +400,6 @@
// returns immediately.
void Wait();
- // WaitFor waits until the given notification type is received from the
- // given object. If the notification was emitted between the construction of
- // this object and this call then it returns immediately.
- //
- // Use this variant when you supply AllSources to the constructor but want
- // to wait for notification from a specific observer.
- //
- // Beware that this is inheriently plagued by ABA issues. Consider:
- // WindowedNotificationObserver is created, listening for notifications from
- // all sources
- // Object A is created with address x and fires a notification
- // Object A is freed
- // Object B is created with the same address
- // WaitFor is called with the address of B
- //
- // In this case, WaitFor will return immediately because of the
- // notification from A (because they shared an address), despite being
- // different objects.
- void WaitFor(const NotificationSource& source);
-
// NotificationObserver:
virtual void Observe(int type,
const NotificationSource& source,
diff --git a/content/browser/speech/speech_input_browsertest.cc b/content/browser/speech/speech_input_browsertest.cc
index e5c5a2e2..4110a81 100644
--- a/content/browser/speech/speech_input_browsertest.cc
+++ b/content/browser/speech/speech_input_browsertest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
@@ -141,19 +141,21 @@
mouse_event.y = 0;
mouse_event.clickCount = 1;
TabContents* tab_contents = browser()->GetSelectedTabContents();
+ ui_test_utils::WindowedNotificationObserver observer(
+ content::NOTIFICATION_LOAD_STOP,
+ Source<NavigationController>(&tab_contents->controller()));
tab_contents->render_view_host()->ForwardMouseEvent(mouse_event);
mouse_event.type = WebKit::WebInputEvent::MouseUp;
tab_contents->render_view_host()->ForwardMouseEvent(mouse_event);
+ observer.Wait();
}
void RunSpeechInputTest(const FilePath::CharType* filename) {
- LoadAndStartSpeechInputTest(filename);
-
// The fake speech input manager would receive the speech input
// request and return the test string as recognition result. The test page
// then sets the URL fragment as 'pass' if it received the expected string.
- TabContents* tab_contents = browser()->GetSelectedTabContents();
- ui_test_utils::WaitForNavigations(&tab_contents->controller(), 1);
+ LoadAndStartSpeechInputTest(filename);
+
EXPECT_EQ("pass", browser()->GetSelectedTabContents()->GetURL().ref());
}
