Converted download UI tests to Browser tests.

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 0be4bd8f7cdc87f3254ceb2fa14f5f0089889ee0..ebca1431dda18015efac1a0dd82d183834c5ae02 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -48,6 +48,20 @@
 
 namespace ui_test_utils {
 
+// Waits for a new tab to be added anywhere.
+Browser* WaitForAnyNewTab();
+
+// 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.
+void NavigateToURLWithDispositionBlockUntilNavigationsComplete(
+    Browser* browser,
+    const GURL& url,
+    int number_of_navigations,
+    WindowOpenDisposition disposition,
+    int browser_test_flags);
+
 namespace {
 
 // Used to block until a navigation completes.
@@ -407,6 +421,21 @@ void WaitForNavigations(NavigationController* controller,
   NavigationNotificationObserver observer(controller, number_of_navigations);
 }
 
+void WaitForWindowClosed(Browser* browser) {

[Paweł Hajdan Jr., 2011/01/03 09:06:47]

I'd strongly prefer to add another variant of WaitForNotification that takes a
NotificationSource rather than create hundreds of functions for every
notification.

[ahendrickson, 2011/01/05 00:32:09]

Done.

+  TestNotificationObserver observer;
+  RegisterAndWait(&observer,
+                  NotificationType::BROWSER_CLOSED,
+                  Source<Browser>(browser));
+}
+
+Browser* WaitForAnyNewTab() {
+  TestNotificationObserver observer;
+  RegisterAndWait(&observer,
+                  NotificationType::TAB_ADDED,
+                  NotificationService::AllSources());
+  return Source<Browser>(observer.source()).ptr();
+}
+
 void WaitForNewTab(Browser* browser) {
   TestNotificationObserver observer;
   RegisterAndWait(&observer, NotificationType::TAB_ADDED,
@@ -429,7 +458,21 @@ Browser* WaitForNewBrowser() {
   TestNotificationObserver observer;
   RegisterAndWait(&observer, NotificationType::BROWSER_WINDOW_READY,
                   NotificationService::AllSources());
-  return Source<Browser>(observer.source()).ptr();
+  Browser* new_browser = Source<Browser>(observer.source()).ptr();
+  return new_browser;
+}
+
+Browser* WaitForNewBrowserWithCount(size_t start_count) {

[Paweł Hajdan Jr., 2011/01/03 09:06:47]

This function seems quite confusing. Can we avoid adding it to ui_test_utils?

[ahendrickson, 2011/01/05 00:32:09]

Replaced the function.

+  TestNotificationObserver observer;
+  size_t cur_count = BrowserList::size();
+  Browser* new_browser = NULL;
+  if (cur_count <= start_count) {
+    new_browser = WaitForNewBrowser();
+  } else {
+    int index = start_count;
+    new_browser = GetBrowser(index);
+  }
+  return new_browser;
 }
 
 void OpenURLOffTheRecord(Profile* profile, const GURL& url) {
@@ -440,16 +483,69 @@ void OpenURLOffTheRecord(Profile* profile, const GURL& url) {
 }
 
 void NavigateToURL(Browser* browser, const GURL& url) {
-  NavigateToURLBlockUntilNavigationsComplete(browser, url, 1);
+  NavigateToURLWithDisposition(browser, url, CURRENT_TAB,
+                               BROWSER_TEST_WAIT_FOR_NAVIGATION);
+}
+
+void NavigateToURLWithDisposition(Browser* browser,
+                                  const GURL& url,
+                                  WindowOpenDisposition disposition,
+                                  int browser_test_flags) {
+  NavigateToURLWithDispositionBlockUntilNavigationsComplete(
+      browser,
+      url,
+      1,
+      disposition,
+      browser_test_flags);
 }
 
 void NavigateToURLBlockUntilNavigationsComplete(Browser* browser,
                                                 const GURL& url,
                                                 int number_of_navigations) {
-  NavigationController* controller =
-      &browser->GetSelectedTabContents()->controller();
-  browser->OpenURL(url, GURL(), CURRENT_TAB, PageTransition::TYPED);
-  WaitForNavigations(controller, number_of_navigations);
+  NavigateToURLWithDispositionBlockUntilNavigationsComplete(
+      browser,
+      url,
+      number_of_navigations,
+      CURRENT_TAB,
+      BROWSER_TEST_WAIT_FOR_NAVIGATION);
+}
+
+void NavigateToURLWithDispositionBlockUntilNavigationsComplete(
+    Browser* browser,
+    const GURL& url,
+    int number_of_navigations,
+    WindowOpenDisposition disposition,
+    int browser_test_flags) {
+  size_t browser_count = BrowserList::size();
+  browser->OpenURL(url, GURL(), disposition, PageTransition::TYPED);
+  if (browser_test_flags & BROWSER_TEST_WAIT_FOR_BROWSER)
+    browser = WaitForNewBrowserWithCount(browser_count);
+  if (browser_test_flags & BROWSER_TEST_WAIT_FOR_TAB)
+    WaitForAnyNewTab();
+  if (!(browser_test_flags & BROWSER_TEST_WAIT_FOR_NAVIGATION))

[Paweł Hajdan Jr., 2011/01/03 09:06:47]

This seems to contradict the function's name. :-( Can we fix that?

[ahendrickson, 2011/01/05 00:32:09]

Added a comment explaining 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) {
+    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";
 }
 
 DOMElementProxyRef GetActiveDOMDocument(Browser* browser) {
@@ -606,6 +702,17 @@ bool BringBrowserWindowToFront(const Browser* browser) {
   return true;
 }
 
+Browser* GetBrowser(int index) {

[Paweł Hajdan Jr., 2011/01/03 09:06:47]

This seems more like something to be added to BrowserList, and I'm not sure
whether we want that (i.e. whether the index will always stay the same, etc,
etc).

[ahendrickson, 2011/01/05 00:32:09]

No longer using this -- replaced with a more appropriate function.

+  if (index >= 0) {
+    BrowserList::const_iterator iter = BrowserList::begin();
+    for ( ; (iter != BrowserList::end()) && (index > 0); ++iter, --index) {}
+    if (iter != BrowserList::end()) {
+      return *iter;
+    }
+  }
+  return NULL;
+}
+
 bool SendKeyPressSync(const Browser* browser,
                       app::KeyboardCode key,
                       bool control,