[GTTF] Miscellanous UI tests cleanups:

- use built-in timeouts instead of "inventing" them in each test case
- avoid unneeded checks and operations
- use automation calls more effectively
- use FLAKY mark instead of DISABLED to maintain test coverage
- split some tests to make the above possible

TEST=UI test based
BUG=39785

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

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

6 files changed, 56 insertions, 68 deletions

diff --git a/chrome/test/automation/automation_proxy_uitest.cc b/chrome/test/automation/automation_proxy_uitest.cc
index b037a25..d784bfc 100644
--- a/chrome/test/automation/automation_proxy_uitest.cc
+++ b/chrome/test/automation/automation_proxy_uitest.cc
@@ -233,19 +233,16 @@ TEST_F(AutomationProxyTest, ActivateTab) {
 
   ASSERT_TRUE(window->AppendTab(GURL("about:blank")));
 
-  int at_index = 1;
-  ASSERT_TRUE(window->ActivateTab(at_index));
+  ASSERT_TRUE(window->ActivateTab(1));
   int active_tab_index = -1;
   ASSERT_TRUE(window->GetActiveTabIndex(&active_tab_index));
-  ASSERT_EQ(at_index, active_tab_index);
+  ASSERT_EQ(1, active_tab_index);
 
-  at_index = 0;
-  ASSERT_TRUE(window->ActivateTab(at_index));
+  ASSERT_TRUE(window->ActivateTab(0));
   ASSERT_TRUE(window->GetActiveTabIndex(&active_tab_index));
-  ASSERT_EQ(at_index, active_tab_index);
+  ASSERT_EQ(0, active_tab_index);
 }
 
-
 TEST_F(AutomationProxyTest, GetTab) {
   scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(window.get());
@@ -364,13 +361,11 @@ TEST_F(AutomationProxyTest2, GetActiveTabIndex) {
 
   int active_tab_index = -1;
   ASSERT_TRUE(window->GetActiveTabIndex(&active_tab_index));
-  int tab_count;
-  ASSERT_TRUE(window->GetTabCount(&tab_count));
   ASSERT_EQ(0, active_tab_index);
-  int at_index = 1;
-  ASSERT_TRUE(window->ActivateTab(at_index));
+
+  ASSERT_TRUE(window->ActivateTab(1));
   ASSERT_TRUE(window->GetActiveTabIndex(&active_tab_index));
-  ASSERT_EQ(at_index, active_tab_index);
+  ASSERT_EQ(1, active_tab_index);
 }
 
 TEST_F(AutomationProxyTest2, GetTabTitle) {
@@ -442,13 +437,11 @@ TEST_F(AutomationProxyTest, AcceleratorNewTab) {
   scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));
 
   int tab_count = -1;
-  ASSERT_TRUE(window->GetTabCount(&tab_count));
-  EXPECT_EQ(1, tab_count);
-
   ASSERT_TRUE(window->RunCommand(IDC_NEW_TAB));
   ASSERT_TRUE(window->GetTabCount(&tab_count));
   EXPECT_EQ(2, tab_count);
-  scoped_refptr<TabProxy> tab(window->GetTab(tab_count - 1));
+
+  scoped_refptr<TabProxy> tab(window->GetTab(1));
   ASSERT_TRUE(tab.get());
 }
 
@@ -566,24 +559,8 @@ TEST_F(AutomationProxyTest3, FrameDocumentCanBeAccessed) {
   std::wstring jscript3 = CreateJSStringForDOMQuery(L"mydiv");
   ASSERT_TRUE(tab->ExecuteAndExtractString(xpath3, jscript3, &actual));
   ASSERT_EQ(L"DIV", actual);
-
-  // TODO(evanm): fix or remove this.
-  // This part of the test appears to verify that executing JS fails
-  // non-HTML pages, but the new tab is now HTML so this test isn't
-  // correct.
-#if 0
-  // Open a new Destinations tab to execute script inside.
-  window->RunCommand(IDC_NEWTAB);
-  tab = window->GetTab(1);
-  ASSERT_TRUE(tab.get());
-  ASSERT_TRUE(window->ActivateTab(1));
-
-  ASSERT_FALSE(tab->ExecuteAndExtractString(xpath1, jscript1, &actual));
-#endif
 }
 
-// TODO(port): Need to port constrained_window_proxy.* first.
-#if defined(OS_WIN)
 TEST_F(AutomationProxyTest, BlockedPopupTest) {
   scoped_refptr<BrowserProxy> window(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(window.get());
@@ -597,11 +574,10 @@ TEST_F(AutomationProxyTest, BlockedPopupTest) {
 
   ASSERT_TRUE(tab->NavigateToURL(net::FilePathToFileURL(filename)));
 
-  ASSERT_TRUE(tab->WaitForBlockedPopupCountToChangeTo(2, 5000));
+  ASSERT_TRUE(tab->WaitForBlockedPopupCountToChangeTo(2,
+                                                      action_max_timeout_ms()));
 }
 
-#endif  // defined(OS_WIN)
-
 // TODO(port): Remove HWND if possible
 #if defined(OS_WIN)
 
@@ -1357,7 +1333,7 @@ TEST_F(AutomationProxyVisibleTest, AutocompleteMatchesTest) {
   EXPECT_TRUE(browser->ApplyAccelerator(IDC_FOCUS_LOCATION));
   EXPECT_TRUE(edit->is_valid());
   EXPECT_TRUE(edit->SetText(L"Roflcopter"));
-  EXPECT_TRUE(edit->WaitForQuery(30000));
+  EXPECT_TRUE(edit->WaitForQuery(action_max_timeout_ms()));
   bool query_in_progress;
   EXPECT_TRUE(edit->IsQueryInProgress(&query_in_progress));
   EXPECT_FALSE(query_in_progress);
@@ -1366,9 +1342,14 @@ TEST_F(AutomationProxyVisibleTest, AutocompleteMatchesTest) {
   EXPECT_FALSE(matches.empty());
 }
 
-// This test is flaky, see http://crbug.com/5314. Disabled because it hangs
-// on Mac (http://crbug.com/25039).
-TEST_F(AutomationProxyTest, DISABLED_AppModalDialogTest) {
+#if defined(OS_MACOSX)
+// Hangs on Mac, http://crbug.com/25039.
+#define AppModalDialogTest DISABLED_AppModalDialogTest
+#else
+// Flaky, http://crbug.com/5314.
+#define AppModalDialogTest FLAKY_AppModalDialogTest
+#endif
+TEST_F(AutomationProxyTest, AppModalDialogTest) {
   scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
   ASSERT_TRUE(browser.get());
   scoped_refptr<TabProxy> tab(browser->GetTab(0));
@@ -1480,7 +1461,7 @@ TEST_F(AutomationProxyTest5, TestLifetimeOfDomAutomationController) {
             tab->NavigateToURL(net::FilePathToFileURL(filename)));
 
   // Allow some time for the popup to show up and close.
-  PlatformThread::Sleep(2000);
+  PlatformThread::Sleep(sleep_timeout_ms());
 
   std::wstring expected(L"string");
   std::wstring jscript = CreateJSString(L"\"" + expected + L"\"");
diff --git a/chrome/test/ui/fast_shutdown_uitest.cc b/chrome/test/ui/fast_shutdown_uitest.cc
index 8b2b554..6f4e174 100644
--- a/chrome/test/ui/fast_shutdown_uitest.cc
+++ b/chrome/test/ui/fast_shutdown_uitest.cc
@@ -26,8 +26,7 @@ TEST_F(FastShutdown, DISABLED_SlowTermination) {
   ASSERT_TRUE(window.get());
 
   // This page has an unload handler.
-  GURL url = GetTestUrl(L"fast_shutdown", L"on_unloader.html");
-  NavigateToURLBlockUntilNavigationsComplete(url, 1);
+  NavigateToURL(GetTestUrl(L"fast_shutdown", L"on_unloader.html"));
   gfx::Rect bounds;
   ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER, &bounds, true));
   // This click will launch a popup which has a before unload handler.
diff --git a/chrome/test/ui/history_uitest.cc b/chrome/test/ui/history_uitest.cc
index 522c221..22c1aee 100644
--- a/chrome/test/ui/history_uitest.cc
+++ b/chrome/test/ui/history_uitest.cc
@@ -33,34 +33,36 @@ class HistoryTester : public UITest {
 // These tests are flaky because automatic and user-initiated transitions are
 // distinguished based on the interval between page load and redirect.
 
-TEST_F(HistoryTester, DISABLED_VerifyHistoryLength) {
+// Flaky, http://crbug.com/39785.
+TEST_F(HistoryTester, FLAKY_VerifyHistoryLength1) {
   // Test the history length for the following page transitions.
-  //
-  // Test case 1:
   //   -open-> Page 1.
-  // Test case 2:
-  //   -open-> Page 2 -redirect-> Page 3.
-  // Test case 3:
-  //   -open-> Page 4 -navigate_backward-> Page 3 -navigate_backward->Page 1
-  //   -navigate_forward-> Page 3 -navigate_forward-> Page 4
-  //
-  // Note that Page 2 is not visited on navigating backward/forward.
 
-  // Test case 1
   std::wstring test_case_1 = L"history_length_test_page_1.html";
   GURL url_1 = GetTestUrl(L"History", test_case_1);
   NavigateToURL(url_1);
   WaitForFinish("History_Length_Test_1", "1", url_1, kTestCompleteCookie,
-                  kTestCompleteSuccess, action_max_timeout_ms());
+                kTestCompleteSuccess, action_max_timeout_ms());
+}
+
+// Disabled, http://crbug.com/39785.
+TEST_F(HistoryTester, DISABLED_VerifyHistoryLength2) {
+  // Test the history length for the following page transitions.
+  //   -open-> Page 2 -redirect-> Page 3.
 
-  // Test case 2
   std::wstring test_case_2 = L"history_length_test_page_2.html";
   GURL url_2 = GetTestUrl(L"History", test_case_2);
   NavigateToURL(url_2);
   WaitForFinish("History_Length_Test_2", "1", url_2, kTestCompleteCookie,
                 kTestCompleteSuccess, action_max_timeout_ms());
+}
+
+// Disabled, http://crbug.com/39785.
+TEST_F(HistoryTester, DISABLED_VerifyHistoryLength3) {
+  // Test the history length for the following page transitions.
+  //   -open-> Page 4 -navigate_backward-> Page 3 -navigate_backward->Page 1
+  //   -navigate_forward-> Page 3 -navigate_forward-> Page 4
 
-  // Test case 3
   std::wstring test_case_3 = L"history_length_test_page_4.html";
   GURL url_3 = GetTestUrl(L"History", test_case_3);
   NavigateToURL(url_3);
@@ -69,7 +71,8 @@ TEST_F(HistoryTester, DISABLED_VerifyHistoryLength) {
 }
 
 #if defined(OS_WIN) || defined(OS_LINUX)
-TEST_F(HistoryTester, DISABLED_ConsiderRedirectAfterGestureAsUserInitiated) {
+// Flaky, http://crbug.com/39785.
+TEST_F(HistoryTester, FLAKY_ConsiderRedirectAfterGestureAsUserInitiated) {
   // Test the history length for the following page transition.
   //
   // -open-> Page 11 -slow_redirect-> Page 12.
@@ -86,7 +89,9 @@ TEST_F(HistoryTester, DISABLED_ConsiderRedirectAfterGestureAsUserInitiated) {
 
   // Simulate click. This only works for Windows.
   scoped_refptr<BrowserProxy> browser = automation()->GetBrowserWindow(0);
+  ASSERT_TRUE(browser.get());
   scoped_refptr<WindowProxy> window = browser->GetWindow();
+  ASSERT_TRUE(window.get());
   gfx::Rect tab_view_bounds;
   ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER, &tab_view_bounds,
                                     true));
@@ -100,7 +105,8 @@ TEST_F(HistoryTester, DISABLED_ConsiderRedirectAfterGestureAsUserInitiated) {
 }
 #endif  // defined(OS_WIN) || defined(OS_LINUX)
 
-TEST_F(HistoryTester, DISABLED_ConsiderSlowRedirectAsUserInitiated) {
+// Flaky, http://crbug.com/39785.
+TEST_F(HistoryTester, FLAKY_ConsiderSlowRedirectAsUserInitiated) {
   // Test the history length for the following page transition.
   //
   // -open-> Page 21 -redirect-> Page 22.
diff --git a/chrome/test/ui/mouseleave_uitest.cc b/chrome/test/ui/mouseleave_uitest.cc
index 91a269f..be647b2 100644
--- a/chrome/test/ui/mouseleave_uitest.cc
+++ b/chrome/test/ui/mouseleave_uitest.cc
@@ -29,7 +29,7 @@
 #elif defined(OS_WIN)
 // Test succeeds locally, flaky on trybot
 // http://code.google.com/p/chromium/issues/detail?id=26349
-#define MAYBE_TestOnMouseOut DISABLED_TestOnMouseOut
+#define MAYBE_TestOnMouseOut FLAKY_TestOnMouseOut
 #endif
 
 namespace {
@@ -47,10 +47,12 @@ class MouseLeaveTest : public UITest {
 TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
   GURL test_url = GetTestUrl(L"", L"mouseleave.html");
 
-  scoped_refptr<TabProxy> tab(GetActiveTab());
-  ASSERT_TRUE(tab.get());
   scoped_refptr<BrowserProxy> browser = automation()->GetBrowserWindow(0);
+  ASSERT_TRUE(browser.get());
   scoped_refptr<WindowProxy> window = browser->GetWindow();
+  ASSERT_TRUE(window.get());
+  scoped_refptr<TabProxy> tab(GetActiveTab());
+  ASSERT_TRUE(tab.get());
 
   gfx::Rect tab_view_bounds;
   ASSERT_TRUE(window->GetViewBounds(VIEW_ID_TAB_CONTAINER, &tab_view_bounds,
@@ -73,8 +75,7 @@ TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
   // Wait for the onload() handler to complete so we can do the
   // next part of the test.
   ASSERT_TRUE(WaitUntilCookieValue(
-      tab.get(), test_url, "__state", timeout_ms,
-      "initial"));
+      tab.get(), test_url, "__state", timeout_ms, "initial"));
 
   // Move the cursor to the top-center of the content, which will trigger
   // a javascript onMouseOver event.
@@ -82,8 +83,7 @@ TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
 
   // Wait on the correct intermediate value of the cookie.
   ASSERT_TRUE(WaitUntilCookieValue(
-      tab.get(), test_url, "__state", timeout_ms,
-      "initial,entered"));
+      tab.get(), test_url, "__state", timeout_ms, "initial,entered"));
 
   // Move the cursor above the content again, which should trigger
   // a javascript onMouseOut event.
@@ -91,8 +91,7 @@ TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
 
   // Wait on the correct final value of the cookie.
   ASSERT_TRUE(WaitUntilCookieValue(
-      tab.get(), test_url, "__state", timeout_ms,
-      "initial,entered,left"));
+      tab.get(), test_url, "__state", timeout_ms, "initial,entered,left"));
 }
 
 }  // namespace
diff --git a/chrome/test/ui/omnibox_uitest.cc b/chrome/test/ui/omnibox_uitest.cc
index ebf5d7b..6ccb107 100644
--- a/chrome/test/ui/omnibox_uitest.cc
+++ b/chrome/test/ui/omnibox_uitest.cc
@@ -76,7 +76,9 @@ bool OmniboxTest::IsMatch(const std::wstring& input_text,
 void OmniboxTest::RunQueryChain(const std::wstring& input_text) {
   // Get a handle on the omnibox and give it focus.
   scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(browser.get());
   scoped_refptr<WindowProxy> window(browser->GetWindow());
+  ASSERT_TRUE(window.get());
   scoped_refptr<AutocompleteEditProxy> autocomplete_edit(
       browser->GetAutocompleteEdit());
   ASSERT_TRUE(browser->ApplyAccelerator(IDC_FOCUS_LOCATION));
@@ -90,7 +92,7 @@ void OmniboxTest::RunQueryChain(const std::wstring& input_text) {
     // matches to be returned to us.
     ASSERT_TRUE(autocomplete_edit->SetText(input_text.substr(0, i)));
     PerfTimer timer;
-    if (autocomplete_edit->WaitForQuery(30000)) {
+    if (autocomplete_edit->WaitForQuery(action_max_timeout_ms())) {
       ASSERT_TRUE(autocomplete_edit->GetAutocompleteMatches(&matches));
       int64 time_elapsed = timer.Elapsed().InMilliseconds();
 
diff --git a/chrome/test/ui/v8_benchmark_uitest.cc b/chrome/test/ui/v8_benchmark_uitest.cc
index 792328e..b020af1 100644
--- a/chrome/test/ui/v8_benchmark_uitest.cc
+++ b/chrome/test/ui/v8_benchmark_uitest.cc
@@ -40,6 +40,7 @@ class V8BenchmarkTest : public UITest {
     GURL test_url(net::FilePathToFileURL(test_path));
 
     scoped_refptr<TabProxy> tab(GetActiveTab());
+    ASSERT_TRUE(tab.get());
     ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, tab->NavigateToURL(test_url));
 
     // Wait for the test to finish.