ui_test_utils::NavigateToURLBlockUntilNavigationsComplete can be raced

chrome/test/ui_test_utils.cc

 // 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.
 
 #include "chrome/test/ui_test_utils.h"
 
 #include <vector>
 
 #include "base/callback.h"
 #include "base/command_line.h"
@@ skipping 50 matching lines @@
                                  int number_of_navigations)
       : navigation_started_(false),
         navigations_completed_(0),
         number_of_navigations_(number_of_navigations) {
     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));
+  }
+
+  void Run() {
     RunMessageLoop();
   }
 
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details) {
     if (type == NotificationType::NAV_ENTRY_COMMITTED ||
         type == NotificationType::LOAD_START) {
       navigation_started_ = true;
     } else if (type == NotificationType::LOAD_STOP) {
       if (navigation_started_ &&
           ++navigations_completed_ == number_of_navigations_) {
         navigation_started_ = false;
         MessageLoopForUI::current()->Quit();

[sky, 2011/03/09 21:52:50]

Doesn't this mean Quit may be invoked before Run? Who knows what problems that
may cause. This code should only quit if Run was invoked, and similarly Run
should only Run if needed.

Also, add some documentation that describes the two cases.

       }
     }
   }
 
  private:
   NotificationRegistrar registrar_;
 
   // If true the navigation has started.
   bool navigation_started_;
 
@@ skipping 310 matching lines @@
   return true;
 }
 
 void WaitForNavigation(NavigationController* controller) {
   WaitForNavigations(controller, 1);
 }
 
 void WaitForNavigations(NavigationController* controller,
                         int number_of_navigations) {
   NavigationNotificationObserver observer(controller, number_of_navigations);
+  observer.Run();
 }
 
 void WaitForNewTab(Browser* browser) {
   TestNotificationObserver observer;
   RegisterAndWait(&observer, NotificationType::TAB_ADDED,
                   Source<Browser>(browser));
 }
 
 void WaitForBrowserActionUpdated(ExtensionAction* browser_action) {
   TestNotificationObserver observer;
@@ skipping 40 matching lines @@
 // Navigates the specified tab (via |disposition|) of |browser| to |url|,
 // blocking until the |number_of_navigations| specified complete.
 // |disposition| indicates what tab the download occurs in, and
 // |browser_test_flags| controls what to wait for before continuing.
 static void NavigateToURLWithDispositionBlockUntilNavigationsComplete(
     Browser* browser,
     const GURL& url,
     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();
        ++iter) {
     initial_browsers.insert(*iter);
   }
   browser->OpenURL(url, GURL(), disposition, PageTransition::TYPED);
   if (browser_test_flags & BROWSER_TEST_WAIT_FOR_BROWSER)
     browser = WaitForBrowserNotInSet(initial_browsers);
   if (browser_test_flags & BROWSER_TEST_WAIT_FOR_TAB)
     WaitForNotification(NotificationType::TAB_ADDED);
   if (!(browser_test_flags & BROWSER_TEST_WAIT_FOR_NAVIGATION)) {
     // Some other flag caused the wait prior to this.
     return;
   }
   TabContents* tab_contents = NULL;
   if (disposition == NEW_BACKGROUND_TAB) {
     // We've opened up a new tab, but not selected it.
     tab_contents = browser->GetTabContentsAt(browser->selected_index() + 1);
     EXPECT_TRUE(tab_contents != NULL)
         << " Unable to wait for navigation to \"" << url.spec()
         << "\" because the new tab is not available yet";
     return;
   } else if ((disposition == CURRENT_TAB) ||
       (disposition == NEW_FOREGROUND_TAB) ||
       (disposition == SINGLETON_TAB)) {
     // 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;
   }
   EXPECT_TRUE(NULL != tab_contents) << " Unable to wait for navigation to \""
                                     << url.spec() << "\""
                                     << " because we can't get the tab contents";
 }
 
 void NavigateToURLWithDisposition(Browser* browser,
@@ skipping 530 matching lines @@
   return taker.TakeRenderWidgetSnapshot(rwh, page_size, page_size, bitmap);
 }
 
 bool TakeEntirePageSnapshot(RenderViewHost* rvh, SkBitmap* bitmap) {
   DCHECK(bitmap);
   SnapshotTaker taker;
   return taker.TakeEntirePageSnapshot(rvh, bitmap);
 }
 
 }  // namespace ui_test_utils