Fix up various browser tests not to use WaitForNavigation.

Convenience methods for WindowedNotificationObserver.

R=phajdan.jr@chromium.org
BUG=various
TEST=none


Review URL: http://codereview.chromium.org/7489019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@96593 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/browser_focus_uitest.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
@@ -847,11 +848,23 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, FocusOnReload) {
   ASSERT_TRUE(test_server()->Start());
 
   // Open the new tab, reload.
+  {
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        NotificationService::AllSources());
     browser()->NewTab();
+    observer.Wait();
+  }
   ui_test_utils::RunAllPendingInMessageLoop();
 
+  {
+    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();
+  }
   // Focus should stay on the location bar.
   ASSERT_TRUE(IsViewFocused(VIEW_ID_LOCATION_BAR));
 
@@ -859,8 +872,14 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, FocusOnReload) {
   ui_test_utils::NavigateToURL(browser(), test_server()->GetURL(kSimplePage));
   browser()->FocusLocationBar();
   ASSERT_TRUE(IsViewFocused(VIEW_ID_LOCATION_BAR));
+  {
+    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();
+  }
 
   // Focus should now be on the tab contents.
   browser()->ShowDownloadsTab();
@@ -875,8 +894,14 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, DISABLED_FocusOnReloadCrashedTab) {
   // Open a regular page, crash, reload.
   ui_test_utils::NavigateToURL(browser(), test_server()->GetURL(kSimplePage));
   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();
+  }
 
   // Focus should now be on the tab contents.
   browser()->ShowDownloadsTab();

chrome/browser/crash_recovery_browsertest.cc

@@ -4,6 +4,7 @@
 
 #include "base/file_path.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -15,12 +16,12 @@
 namespace {
 
 void SimulateRendererCrash(Browser* browser) {
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_TAB_CONTENTS_DISCONNECTED,
+      NotificationService::AllSources());
   browser->OpenURL(GURL(chrome::kChromeUICrashURL), GURL(), CURRENT_TAB,
                    PageTransition::TYPED);
-  LOG(ERROR) << "SimulateRendererCrash, before WaitForNotification";
-  ui_test_utils::WaitForNotification(
-      content::NOTIFICATION_TAB_CONTENTS_DISCONNECTED);
-  LOG(ERROR) << "SimulateRendererCrash, after WaitForNotification";
+  observer.Wait();
 }
 
 }  // namespace
@@ -42,10 +43,12 @@ IN_PROC_BROWSER_TEST_F(CrashRecoveryBrowserTest, Reload) {
   ASSERT_TRUE(ui_test_utils::GetCurrentTabTitle(browser(),
                                                 &title_before_crash));
   SimulateRendererCrash(browser());
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP,
+      Source<NavigationController>(
+          &browser()->GetSelectedTabContentsWrapper()->controller()));
   browser()->Reload(CURRENT_TAB);
-  LOG(ERROR) << "Before WaitForNavigationInCurrentTab";
-  ASSERT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(browser()));
-  LOG(ERROR) << "After WaitForNavigationInCurrentTab";
+  observer.Wait();
   ASSERT_TRUE(ui_test_utils::GetCurrentTabTitle(browser(),
                                                 &title_after_crash));
   EXPECT_NE(title_before_crash, title_after_crash);
@@ -69,10 +72,12 @@ IN_PROC_BROWSER_TEST_F(CrashRecoveryBrowserTest, LoadInNewTab) {
   ASSERT_TRUE(ui_test_utils::GetCurrentTabTitle(browser(),
                                                 &title_before_crash));
   SimulateRendererCrash(browser());
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP,
+      Source<NavigationController>(
+          &browser()->GetSelectedTabContentsWrapper()->controller()));
   browser()->Reload(CURRENT_TAB);
-  LOG(ERROR) << "Before WaitForNavigationInCurrentTab";
-  ASSERT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(browser()));
-  LOG(ERROR) << "After WaitForNavigationInCurrentTab";
+  observer.Wait();
   ASSERT_TRUE(ui_test_utils::GetCurrentTabTitle(browser(),
                                                 &title_after_crash));
   EXPECT_EQ(title_before_crash, title_after_crash);

chrome/browser/geolocation/geolocation_browsertest.cc

@@ -317,11 +317,17 @@ class GeolocationBrowserTest : public InProcessBrowserTest {
     size_t state_map_size = settings_state.state_map().size();
     ASSERT_TRUE(infobar_);
     LOG(WARNING) << "will set infobar response";
+    {
+      ui_test_utils::WindowedNotificationObserver observer(
+          content::NOTIFICATION_LOAD_STOP,
+          Source<NavigationController>(&tab_contents_wrapper->controller()));
       if (allowed)
         infobar_->AsConfirmInfoBarDelegate()->Accept();
       else
         infobar_->AsConfirmInfoBarDelegate()->Cancel();
-    WaitForNavigation();
+      observer.Wait();
+    }
+
     tab_contents_wrapper->RemoveInfoBar(infobar_);
     LOG(WARNING) << "infobar response set";
     infobar_ = NULL;

chrome/browser/pdf_browsertest.cc

@@ -336,12 +336,16 @@ IN_PROC_BROWSER_TEST_F(PDFBrowserTest, MAYBE_OnLoadAndReload) {
   GURL url = pdf_test_server()->GetURL("files/onload_reload.html");
   ui_test_utils::NavigateToURL(browser(), url);
 
+  ui_test_utils::WindowedNavigationObserver observer(
+      content::NOTIFICATION_LOAD_STOP,
+      Source<NavigationController>(
+          &browser()->GetSelectedTabContents()->controller()));
   ASSERT_TRUE(ui_test_utils::ExecuteJavaScript(
       browser()->GetSelectedTabContents()->render_view_host(),
       std::wstring(),
       L"reloadPDF();"));
+  observer.Wait();
 
-  ASSERT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(browser()));
   ASSERT_EQ("success", browser()->GetSelectedTabContents()->GetURL().query());
 }
 

chrome/browser/sidebar/sidebar_browsertest.cc

@@ -72,13 +72,11 @@ class SidebarTest : public ExtensionBrowserTest {
 
     SidebarManager* sidebar_manager = SidebarManager::GetInstance();
 
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     sidebar_manager->NavigateSidebar(tab, content_id_, url);
-
-    SidebarContainer* sidebar_container =
-        sidebar_manager->GetSidebarContainerFor(tab, content_id_);
-
-    TabContents* client_contents = sidebar_container->sidebar_contents();
-    ui_test_utils::WaitForNavigation(&client_contents->controller());
+    observer.Wait();
   }
 
   void ShowSidebar(TabContents* tab) {
@@ -199,4 +197,3 @@ IN_PROC_BROWSER_TEST_F(SidebarTest, SidebarNavigate) {
 }
 
 }  // namespace
-

chrome/browser/ssl/ssl_browser_tests.cc

@@ -16,6 +16,7 @@
 #include "content/browser/tab_contents/interstitial_page.h"
 #include "content/browser/tab_contents/navigation_entry.h"
 #include "content/browser/tab_contents/tab_contents.h"
+#include "content/common/notification_service.h"
 #include "net/base/cert_status_flags.h"
 #include "net/test/test_server.h"
 
@@ -116,9 +117,11 @@ class SSLUITest : public InProcessBrowserTest {
   void ProceedThroughInterstitial(TabContents* tab) {
     InterstitialPage* interstitial_page = tab->interstitial_page();
     ASSERT_TRUE(interstitial_page);
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     interstitial_page->Proceed();
-    // Wait for the navigation to be done.
-    ui_test_utils::WaitForNavigation(&(tab->controller()));
+    observer.Wait();
   }
 
   static bool GetFilePathWithHostAndPortReplacement(
@@ -411,8 +414,13 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestHTTPSExpiredCertAndGoForward) {
   ASSERT_TRUE(entry2);
 
   // Now go back so that a page is in the forward history.
+  {
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     tab->controller().GoBack();
-  ui_test_utils::WaitForNavigation(&(tab->controller()));
+    observer.Wait();
+  }
   ASSERT_TRUE(tab->controller().CanGoForward());
   NavigationEntry* entry3 = tab->controller().GetActiveEntry();
   ASSERT_TRUE(entry1 == entry3);
@@ -424,8 +432,13 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestHTTPSExpiredCertAndGoForward) {
                                  true);  // Interstitial showing
 
   // Simulate user clicking and holding on forward button.
+  {
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     tab->controller().GoToOffset(1);
-  ui_test_utils::WaitForNavigation(&(tab->controller()));
+    observer.Wait();
+  }
 
   // We should be showing the second good page.
   EXPECT_FALSE(browser()->GetSelectedTabContents()->interstitial_page());
@@ -597,9 +610,11 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestDisplaysInsecureContentTwoTabs) {
   params.disposition = NEW_FOREGROUND_TAB;
   params.tabstrip_index = 0;
   params.source_contents = tab1;
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP, NotificationService::AllSources());
   browser::Navigate(&params);
   TabContentsWrapper* tab2 = params.target_contents;
-  ui_test_utils::WaitForNavigation(&(tab2->controller()));
+  observer.Wait();
 
   // The new tab has insecure content.
   CheckAuthenticatedState(tab2->tab_contents(), true);
@@ -634,9 +649,11 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestRunsInsecureContentTwoTabs) {
   browser::NavigateParams params(browser(), url, PageTransition::TYPED);
   params.disposition = NEW_FOREGROUND_TAB;
   params.source_contents = tab1;
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP, NotificationService::AllSources());
   browser::Navigate(&params);
   TabContentsWrapper* tab2 = params.target_contents;
-  ui_test_utils::WaitForNavigation(&(tab2->controller()));
+  observer.Wait();
 
   // The new tab has insecure content.
   CheckAuthenticationBrokenState(tab2->tab_contents(), 0, true, false);
@@ -807,9 +824,10 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestCloseTabWithUnsafePopup) {
   // Let's add another tab to make sure the browser does not exit when we close
   // the first tab.
   GURL url = test_server()->GetURL("files/ssl/google.html");
-  TabContentsWrapper* tab2 =
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP, NotificationService::AllSources());
   browser()->AddSelectedTabWithURL(url, PageTransition::TYPED);
-  ui_test_utils::WaitForNavigation(&(tab2->controller()));
+  observer.Wait();
 
   // Close the first tab.
   browser()->CloseTabContents(tab1);
@@ -952,23 +970,33 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestGoodFrameNavigation) {
 
   bool success = false;
   // Now navigate inside the frame.
+  {
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
         tab->render_view_host(), std::wstring(),
         L"window.domAutomationController.send(clickLink('goodHTTPSLink'));",
         &success));
-  EXPECT_TRUE(success);
-  ui_test_utils::WaitForNavigation(&tab->controller());
+    ASSERT_TRUE(success);
+    observer.Wait();
+  }
 
   // We should still be fine.
   CheckAuthenticatedState(tab, false);
 
   // Now let's hit a bad page.
+  {
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
         tab->render_view_host(), std::wstring(),
         L"window.domAutomationController.send(clickLink('badHTTPSLink'));",
         &success));
-  EXPECT_TRUE(success);
-  ui_test_utils::WaitForNavigation(&tab->controller());
+    ASSERT_TRUE(success);
+    observer.Wait();
+  }
 
   // The security style should still be secure.
   CheckAuthenticatedState(tab, false);
@@ -984,24 +1012,39 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestGoodFrameNavigation) {
   EXPECT_FALSE(is_content_evil);
 
   // Now go back, our state should still be OK.
+  {
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     tab->controller().GoBack();
-  ui_test_utils::WaitForNavigation(&tab->controller());
+    observer.Wait();
+  }
   CheckAuthenticatedState(tab, false);
 
   // Navigate to a page served over HTTP.
+  {
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
         tab->render_view_host(), std::wstring(),
         L"window.domAutomationController.send(clickLink('HTTPLink'));",
         &success));
-  EXPECT_TRUE(success);
-  ui_test_utils::WaitForNavigation(&tab->controller());
+    ASSERT_TRUE(success);
+    observer.Wait();
+  }
 
   // Our state should be insecure.
   CheckAuthenticatedState(tab, true);
 
   // Go back, our state should be unchanged.
+  {
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     tab->controller().GoBack();
-  ui_test_utils::WaitForNavigation(&tab->controller());
+    observer.Wait();
+  }
   CheckAuthenticatedState(tab, true);
 }
 
@@ -1028,12 +1071,15 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestBadFrameNavigation) {
 
   // Navigate to a good frame.
   bool success = false;
+  ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
   EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
       tab->render_view_host(), std::wstring(),
       L"window.domAutomationController.send(clickLink('goodHTTPSLink'));",
       &success));
-  EXPECT_TRUE(success);
-  ui_test_utils::WaitForNavigation(&tab->controller());
+  ASSERT_TRUE(success);
+  observer.Wait();
 
   // We should still be authentication broken.
   CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false,
@@ -1060,24 +1106,35 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestUnauthenticatedFrameNavigation) {
   CheckUnauthenticatedState(tab);
 
   // Now navigate inside the frame to a secure HTTPS frame.
+  {
     bool success = false;
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
         tab->render_view_host(), std::wstring(),
         L"window.domAutomationController.send(clickLink('goodHTTPSLink'));",
         &success));
-  EXPECT_TRUE(success);
-  ui_test_utils::WaitForNavigation(&tab->controller());
+    ASSERT_TRUE(success);
+    observer.Wait();
+  }
 
   // We should still be unauthenticated.
   CheckUnauthenticatedState(tab);
 
   // Now navigate to a bad HTTPS frame.
+  {
+    bool success = false;
+    ui_test_utils::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        Source<NavigationController>(&tab->controller()));
     EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
         tab->render_view_host(), std::wstring(),
         L"window.domAutomationController.send(clickLink('badHTTPSLink'));",
         &success));
-  EXPECT_TRUE(success);
-  ui_test_utils::WaitForNavigation(&tab->controller());
+    ASSERT_TRUE(success);
+    observer.Wait();
+  }
 
   // State should not have changed.
   CheckUnauthenticatedState(tab);

chrome/browser/ui/find_bar/find_bar_host_browsertest.cc

@@ -581,8 +581,12 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FindDisappearOnNavigate) {
   EXPECT_TRUE(fully_visible);
 
   // Reload the tab and make sure Find window doesn't go away.
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP,
+      Source<NavigationController>(
+          &browser()->GetSelectedTabContentsWrapper()->controller()));
   browser()->Reload(CURRENT_TAB);
-  ui_test_utils::WaitForNavigationInCurrentTab(browser());
+  observer.Wait();
 
   EXPECT_TRUE(GetFindBarWindowInfo(&position, &fully_visible));
   EXPECT_TRUE(fully_visible);
@@ -1048,9 +1052,10 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, MAYBE_NoIncognitoPrepopulate) {
   // Open a new incognito window and navigate to the same page.
   Profile* incognito_profile = browser()->profile()->GetOffTheRecordProfile();
   Browser* incognito_browser = Browser::Create(incognito_profile);
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP, NotificationService::AllSources());
   incognito_browser->AddSelectedTabWithURL(url, PageTransition::START_PAGE);
-  ui_test_utils::WaitForNavigation(
-      &incognito_browser->GetSelectedTabContents()->controller());
+  observer.Wait();
   incognito_browser->window()->Show();
 
   // Open the find box and make sure that it is prepopulated with "page".
@@ -1093,8 +1098,11 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, ActivateLinkNavigatesPage) {
   EXPECT_EQ(ordinal, 1);
 
   // End the find session, click on the link.
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP,
+      Source<NavigationController>(&tab->controller()));
   tab->find_tab_helper()->StopFinding(FindBarController::kActivateSelection);
-  EXPECT_TRUE(ui_test_utils::WaitForNavigationInCurrentTab(browser()));
+  observer.Wait();
 }
 
 // Tests that FindBar fits within a narrow browser window.
@@ -1102,12 +1110,12 @@ IN_PROC_BROWSER_TEST_F(FindInPageControllerTest, FitWindow) {
   Browser::CreateParams params(Browser::TYPE_POPUP, browser()->profile());
   params.initial_bounds = gfx::Rect(0, 0, 250, 500);
   Browser* popup = Browser::CreateWithParams(params);
+  ui_test_utils::WindowedNotificationObserver observer(
+      content::NOTIFICATION_LOAD_STOP, NotificationService::AllSources());
   popup->AddSelectedTabWithURL(GURL(chrome::kAboutBlankURL),
                                PageTransition::LINK);
-
   // Wait for the page to finish loading.
-  ui_test_utils::WaitForNavigation(
-      &popup->GetSelectedTabContents()->controller());
+  observer.Wait();
   popup->window()->Show();
 
   // On GTK, bounds change is asynchronous.

chrome/test/base/ui_test_utils.cc

@@ -57,6 +57,7 @@ namespace ui_test_utils {
 namespace {
 
 // Used to block until a navigation completes.
+// TODO(gbillock): this should be merged with TestNavigationObserver
 class NavigationNotificationObserver : public NotificationObserver {
  public:
   NavigationNotificationObserver(const NotificationSource& source,

chrome/test/base/ui_test_utils.h

@@ -230,10 +230,14 @@ bool IsViewFocused(const Browser* browser, ViewID vid);
 void ClickOnView(const Browser* browser, ViewID vid);
 
 // Blocks until a notification for given |type| is received.
+// TODO(gbillock): remove this race hazard.
+// Use WindowedNotificationObserver instead.
 void WaitForNotification(int type);
 
 // Blocks until a notification for given |type| from the specified |source|
 // is received.
+// TODO(gbillock): remove this race hazard.
+// Use WindowedNotificationObserver instead.
 void WaitForNotificationFrom(int type,
                              const NotificationSource& source);