ui_test_utils::NavigateToURLBlockUntilNavigationsComplete can be raced

chrome/test/ui_test_utils.cc

Index: chrome/test/ui_test_utils.cc
diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
index c66f12c1c1d6e182e259414c6f2709b23bf8bcba..44bd0f03d413b6d399c911a4940c39ebb5da85f9 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -61,14 +61,22 @@ class NavigationNotificationObserver : public NotificationObserver {
                                  int number_of_navigations)
       : navigation_started_(false),
         navigations_completed_(0),
-        number_of_navigations_(number_of_navigations) {
+        number_of_navigations_(number_of_navigations),
+        running_(false),
+        done_(false) {
     registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
                    Source<NavigationController>(controller));
     registrar_.Add(this, NotificationType::LOAD_START,
                    Source<NavigationController>(controller));
     registrar_.Add(this, NotificationType::LOAD_STOP,
                    Source<NavigationController>(controller));
-    RunMessageLoop();
+  }
+
+  void Run() {
+    if (!done_) {
+      running_ = true;
+      RunMessageLoop();
+    }
   }
 
   virtual void Observe(NotificationType type,
@@ -81,7 +89,9 @@ class NavigationNotificationObserver : public NotificationObserver {
       if (navigation_started_ &&
           ++navigations_completed_ == number_of_navigations_) {
         navigation_started_ = false;
-        MessageLoopForUI::current()->Quit();
+        done_ = true;
+        if (running_)
+          MessageLoopForUI::current()->Quit();
       }
     }
   }
@@ -98,6 +108,13 @@ class NavigationNotificationObserver : public NotificationObserver {
   // The number of navigations to wait for.
   int number_of_navigations_;
 
+  // Calls to Observe() can happen early, before the user calls Run(), or
+  // after.  When we've seen all the navigations we're looking for, we set
+  // done_ to true; then when Run() is called we'll never need to run the
+  // event loop.  Also, we don't need to quit the event loop when we're
+  // done if we never had to start an event loop.
+  bool running_;
+  bool done_;
   DISALLOW_COPY_AND_ASSIGN(NavigationNotificationObserver);
 };
 
@@ -412,6 +429,7 @@ void WaitForNavigation(NavigationController* controller) {
 void WaitForNavigations(NavigationController* controller,
                         int number_of_navigations) {
   NavigationNotificationObserver observer(controller, number_of_navigations);
+  observer.Run();
 }
 
 void WaitForNewTab(Browser* browser) {
@@ -472,6 +490,10 @@ static void NavigateToURLWithDispositionBlockUntilNavigationsComplete(
     int number_of_navigations,
     WindowOpenDisposition disposition,
     int browser_test_flags) {
+  NavigationNotificationObserver
+      same_tab_observer(&browser->GetSelectedTabContents()->controller(),
+                        number_of_navigations);
+
   std::set<Browser*> initial_browsers;
   for (std::vector<Browser*>::const_iterator iter = BrowserList::begin();
        iter != BrowserList::end();
@@ -501,7 +523,10 @@ static void NavigateToURLWithDispositionBlockUntilNavigationsComplete(
     // The currently selected tab is the right one.
     tab_contents = browser->GetSelectedTabContents();
   }
-  if (tab_contents) {
+  if (disposition == CURRENT_TAB) {
+    same_tab_observer.Run();
+    return;
+  } else if (tab_contents) {
     NavigationController* controller = &tab_contents->controller();
     WaitForNavigations(controller, number_of_navigations);
     return;