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

R=phajdan.jr@chromium.org
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

10 files changed, 65 insertions, 110 deletions

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 @@ IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, AccessKeys) {
 }
 
 // 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 @@ IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, DISABLED_ReservedAccelerators) {
 
   // 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 9602831..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 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, ReloadAppAfterCrash) {
 
   // 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 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, MAYBE_PluginLoadUnload) {
   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 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, MAYBE_PluginLoadUnload) {
 
   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 @@ IN_PROC_BROWSER_TEST_F(IsolatedAppApiTest, MAYBE_CookieIsolation) {
   // 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..9666478 100644
--- a/chrome/browser/sessions/session_restore_browsertest.cc
+++ b/chrome/browser/sessions/session_restore_browsertest.cc
@@ -121,11 +121,18 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, RestoreIndividualTabFromWindow) {
 
   // 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 02a7560..085e079 100644
--- a/chrome/browser/ui/browser_navigator_browsertest_chromeos.cc
+++ b/chrome/browser/ui/browser_navigator_browsertest_chromeos.cc
@@ -63,11 +63,9 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_LargePopup) {
   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 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_LargePopupFromPopup) {
   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);
-
-  // Wait for page to load.
-  ui_test_utils::WaitForNavigationInCurrentTab(p2.browser);
+  ui_test_utils::NavigateToURL(&p2);
 
-  // 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 e05b49f..84feac8 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 @@ IN_PROC_BROWSER_TEST_F(FindInPageTest, FocusRestoreOnTabSwitch) {
   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 5140b22..d087e68 100644
--- a/chrome/test/base/ui_test_utils.cc
+++ b/chrome/test/base/ui_test_utils.cc
@@ -252,27 +252,6 @@ bool GetCurrentTabTitle(const Browser* browser, string16* title) {
   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 @@ void WaitForNavigations(NavigationController* controller,
   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 @@ void WindowedNotificationObserver::Wait() {
   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 201f82f..07a4311 100644
--- a/chrome/test/base/ui_test_utils.h
+++ b/chrome/test/base/ui_test_utils.h
@@ -88,33 +88,17 @@ void RunAllPendingInMessageLoop();
 // 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 @@ class WindowedNotificationObserver : public NotificationObserver {
   // 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 e5c5a2e..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 @@ class SpeechInputBrowserTest : public InProcessBrowserTest {
     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());
   }