[GTTF] Unify polling interval in some of the polling functions

in the UI test framework:

- WaitUntilCookieValue
- WaitUntilCookieNonEmpty
- WaitUntilJavaScriptCondition

This helps reduce number of arbitrary "magic timeouts" which are different
in each test.

We don't remove the test timeout parameter, because it varies depending on test case,
and for now we don't have a better solution. Trying a "naive" way leads to hangs
on flaky tests.

TEST=UI-based tests
BUG=none

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

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

18 files changed, 49 insertions, 73 deletions

diff --git a/chrome/browser/browser_uitest.cc b/chrome/browser/browser_uitest.cc
index f22c32b..11a613fb 100644
--- a/chrome/browser/browser_uitest.cc
+++ b/chrome/browser/browser_uitest.cc
@@ -231,7 +231,7 @@ TEST_F(SecurityTest, DisallowFileUrlUniversalAccessTest) {
   ASSERT_TRUE(tab->NavigateToURL(url));
 
   std::string value = WaitUntilCookieNonEmpty(tab.get(), url,
-        "status", sleep_timeout_ms(), action_max_timeout_ms());
+        "status", action_max_timeout_ms());
   ASSERT_STREQ("Disallowed", value.c_str());
 }
 
diff --git a/chrome/browser/dom_ui/bookmarks_ui_uitest.cc b/chrome/browser/dom_ui/bookmarks_ui_uitest.cc
index 774495a..70552c9 100644
--- a/chrome/browser/dom_ui/bookmarks_ui_uitest.cc
+++ b/chrome/browser/dom_ui/bookmarks_ui_uitest.cc
@@ -21,7 +21,7 @@ class BookmarksUITest : public UITest {
         L"domAutomationController.send("
         L"    location.protocol == 'chrome-extension:' && "
         L"    document.readyState == 'complete')",
-        100, UITest::test_timeout_ms());
+        UITest::test_timeout_ms());
   }
 
   scoped_refptr<TabProxy> GetBookmarksUITab() {
diff --git a/chrome/browser/in_process_webkit/dom_storage_uitest.cc b/chrome/browser/in_process_webkit/dom_storage_uitest.cc
index d3f97f6..2da6e66 100644
--- a/chrome/browser/in_process_webkit/dom_storage_uitest.cc
+++ b/chrome/browser/in_process_webkit/dom_storage_uitest.cc
@@ -72,8 +72,7 @@ class DOMStorageTest : public UILayoutTest {
     ASSERT_TRUE(tab->SetCookie(url, ""));
     ASSERT_TRUE(tab->NavigateToURL(url));
 
-    WaitUntilCookieNonEmpty(tab.get(), url, "cleared", kTestIntervalMs,
-                            action_max_timeout_ms());
+    WaitUntilCookieNonEmpty(tab.get(), url, "cleared", action_max_timeout_ms());
   }
 
   // Runs each test in an array of strings until it hits a NULL.
diff --git a/chrome/test/automation/automation_proxy_uitest.cc b/chrome/test/automation/automation_proxy_uitest.cc
index 004b99b..31ed3e4 100644
--- a/chrome/test/automation/automation_proxy_uitest.cc
+++ b/chrome/test/automation/automation_proxy_uitest.cc
@@ -433,7 +433,7 @@ TEST_F(AutomationProxyTest, NavigateToURLAsync) {
   GURL newurl = net::FilePathToFileURL(filename);
 
   ASSERT_TRUE(tab->NavigateToURLAsync(newurl));
-  std::string value = WaitUntilCookieNonEmpty(tab.get(), newurl, "foo", 250,
+  std::string value = WaitUntilCookieNonEmpty(tab.get(), newurl, "foo",
                                               action_max_timeout_ms());
   ASSERT_STREQ("baz", value.c_str());
 }
diff --git a/chrome/test/nacl/nacl_test.cc b/chrome/test/nacl/nacl_test.cc
index 2ad62bb..1560484 100644
--- a/chrome/test/nacl/nacl_test.cc
+++ b/chrome/test/nacl/nacl_test.cc
@@ -106,7 +106,6 @@ void NaClTest::WaitForFinish(const FilePath& filename,
   bool test_result = WaitUntilCookieValue(tab.get(),
                                           url,
                                           kTestCompleteCookie,
-                                          action_timeout_ms(),
                                           wait_time,
                                           kTestCompleteSuccess);
   EXPECT_TRUE(test_result);
diff --git a/chrome/test/page_cycler/page_cycler_test.cc b/chrome/test/page_cycler/page_cycler_test.cc
index dc40036..96f678e 100644
--- a/chrome/test/page_cycler/page_cycler_test.cc
+++ b/chrome/test/page_cycler/page_cycler_test.cc
@@ -235,7 +235,7 @@ class PageCyclerTest : public UITest {
 
     // Wait for the test to finish.
     ASSERT_TRUE(WaitUntilCookieValue(tab.get(), test_url, "__pc_done",
-                                     3000, UITest::test_timeout_ms(), "1"));
+                                     UITest::test_timeout_ms(), "1"));
 
     std::string cookie;
     ASSERT_TRUE(tab->GetCookieByName(test_url, "__pc_pages", &cookie));
diff --git a/chrome/test/selenium/selenium_test.cc b/chrome/test/selenium/selenium_test.cc
index f5710f7..4811167 100644
--- a/chrome/test/selenium/selenium_test.cc
+++ b/chrome/test/selenium/selenium_test.cc
@@ -108,7 +108,7 @@ class SeleniumTest : public UITest {
 
     // Wait for the test to finish.
     ASSERT_TRUE(WaitUntilCookieValue(tab.get(), test_url, "__tests_finished",
-                                     3000, UITest::test_timeout_ms(), "1"));
+                                     UITest::test_timeout_ms(), "1"));
 
     std::string cookie;
     ASSERT_TRUE(tab->GetCookieByName(test_url, "__num_tests_total", &cookie));
diff --git a/chrome/test/ui/dom_checker_uitest.cc b/chrome/test/ui/dom_checker_uitest.cc
index 2d5e2db..2f3d858 100644
--- a/chrome/test/ui/dom_checker_uitest.cc
+++ b/chrome/test/ui/dom_checker_uitest.cc
@@ -139,7 +139,7 @@ class DomCheckerTest : public UITest {
   bool WaitUntilTestCompletes(TabProxy* tab) {
     return WaitUntilJavaScriptCondition(tab, L"",
         L"window.domAutomationController.send(automation.IsDone());",
-        1000, UITest::test_timeout_ms());
+        UITest::test_timeout_ms());
   }
 
   bool GetTestCount(TabProxy* tab, int* test_count) {
diff --git a/chrome/test/ui/dromaeo_benchmark_uitest.cc b/chrome/test/ui/dromaeo_benchmark_uitest.cc
index 9e5dcc3..2c971d7 100644
--- a/chrome/test/ui/dromaeo_benchmark_uitest.cc
+++ b/chrome/test/ui/dromaeo_benchmark_uitest.cc
@@ -59,7 +59,7 @@ class DromaeoTest : public UITest {
   }
 
   bool WaitUntilTestCompletes(TabProxy* tab, const GURL& test_url) {
-    return WaitUntilCookieValue(tab, test_url, "__done", 1000,
+    return WaitUntilCookieValue(tab, test_url, "__done",
                                 UITest::test_timeout_ms(), "1");
   }
 
diff --git a/chrome/test/ui/mouseleave_uitest.cc b/chrome/test/ui/mouseleave_uitest.cc
index 311b27a..91a269f 100644
--- a/chrome/test/ui/mouseleave_uitest.cc
+++ b/chrome/test/ui/mouseleave_uitest.cc
@@ -69,12 +69,11 @@ TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
   ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, tab->NavigateToURL(test_url));
 
   const int timeout_ms = 5 * action_max_timeout_ms();
-  const int check_interval_ms = action_max_timeout_ms() / 10;
 
   // 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", check_interval_ms, timeout_ms,
+      tab.get(), test_url, "__state", timeout_ms,
       "initial"));
 
   // Move the cursor to the top-center of the content, which will trigger
@@ -83,7 +82,7 @@ TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
 
   // Wait on the correct intermediate value of the cookie.
   ASSERT_TRUE(WaitUntilCookieValue(
-      tab.get(), test_url, "__state", check_interval_ms, timeout_ms,
+      tab.get(), test_url, "__state", timeout_ms,
       "initial,entered"));
 
   // Move the cursor above the content again, which should trigger
@@ -92,7 +91,7 @@ TEST_F(MouseLeaveTest, MAYBE_TestOnMouseOut) {
 
   // Wait on the correct final value of the cookie.
   ASSERT_TRUE(WaitUntilCookieValue(
-      tab.get(), test_url, "__state", check_interval_ms, timeout_ms,
+      tab.get(), test_url, "__state", timeout_ms,
       "initial,entered,left"));
 }
 
diff --git a/chrome/test/ui/sunspider_uitest.cc b/chrome/test/ui/sunspider_uitest.cc
index 2322f34..1c4f484 100644
--- a/chrome/test/ui/sunspider_uitest.cc
+++ b/chrome/test/ui/sunspider_uitest.cc
@@ -60,7 +60,7 @@ class SunSpiderTest : public UITest {
   }
 
   bool WaitUntilTestCompletes(TabProxy* tab, const GURL& test_url) {
-    return WaitUntilCookieValue(tab, test_url, "__done", 1000,
+    return WaitUntilCookieValue(tab, test_url, "__done",
                                 UITest::test_timeout_ms(), "1");
   }
 
diff --git a/chrome/test/ui/ui_layout_test.cc b/chrome/test/ui/ui_layout_test.cc
index 7dabbd9..2c7dfdc 100644
--- a/chrome/test/ui/ui_layout_test.cc
+++ b/chrome/test/ui/ui_layout_test.cc
@@ -194,7 +194,7 @@ void UILayoutTest::RunLayoutTest(const std::string& test_case_file_name,
   ASSERT_TRUE(tab->NavigateToURL(*new_test_url.get()));
   std::string escaped_value =
       WaitUntilCookieNonEmpty(tab.get(), *new_test_url.get(),
-          status_cookie.c_str(), kTestIntervalMs, action_max_timeout_ms());
+          status_cookie.c_str(), action_max_timeout_ms());
 
   // Unescapes and normalizes the actual result.
   std::string value = UnescapeURLComponent(escaped_value,
diff --git a/chrome/test/ui/ui_layout_test.h b/chrome/test/ui/ui_layout_test.h
index 707e302..c1f1fe1 100644
--- a/chrome/test/ui/ui_layout_test.h
+++ b/chrome/test/ui/ui_layout_test.h
@@ -36,7 +36,6 @@ class UILayoutTest : public UITest {
   FilePath rebase_result_win_dir_;
   std::string layout_test_controller_;
 
-  static const int kTestIntervalMs = 250;
   static const int kNoHttpPort = -1;
   static const int kHttpPort = 8080;
   static const int kWebSocketPort = 8880;
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index af8514b..cd1a81b 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -746,85 +746,69 @@ int UITestBase::GetTabCount(int window_index) {
 bool UITestBase::WaitUntilCookieValue(TabProxy* tab,
                                       const GURL& url,
                                       const char* cookie_name,
-                                      int interval_ms,
                                       int time_out_ms,
                                       const char* expected_value) {
-  const int kMaxIntervals = time_out_ms / interval_ms;
+  const int kIntervalMs = 250;
+  const int kMaxIntervals = time_out_ms / kIntervalMs;
 
   std::string cookie_value;
-  bool completed = false;
   for (int i = 0; i < kMaxIntervals; ++i) {
-    bool browser_survived = CrashAwareSleep(interval_ms);
+    bool browser_survived = CrashAwareSleep(kIntervalMs);
+    EXPECT_TRUE(browser_survived);
+    if (!browser_survived)
+      return false;
 
     EXPECT_TRUE(tab->GetCookieByName(url, cookie_name, &cookie_value));
-
-    if (cookie_value == expected_value) {
-      completed = true;
-      break;
-    }
-    EXPECT_TRUE(browser_survived);
-    if (!browser_survived) {
-      // The browser process died.
-      break;
-    }
+    if (cookie_value == expected_value)
+      return true;
   }
-  return completed;
+
+  return false;
 }
 
 std::string UITestBase::WaitUntilCookieNonEmpty(TabProxy* tab,
                                                 const GURL& url,
                                                 const char* cookie_name,
-                                                int interval_ms,
                                                 int time_out_ms) {
-  const int kMaxIntervals = time_out_ms / interval_ms;
+  const int kIntervalMs = 250;
+  const int kMaxIntervals = time_out_ms / kIntervalMs;
 
-  std::string cookie_value;
   for (int i = 0; i < kMaxIntervals; ++i) {
-    bool browser_survived = CrashAwareSleep(interval_ms);
+    bool browser_survived = CrashAwareSleep(kIntervalMs);
+    EXPECT_TRUE(browser_survived);
+    if (!browser_survived)
+      return std::string();
 
+    std::string cookie_value;
     EXPECT_TRUE(tab->GetCookieByName(url, cookie_name, &cookie_value));
-
     if (!cookie_value.empty())
-      break;
-
-    EXPECT_TRUE(browser_survived);
-    if (!browser_survived) {
-      // The browser process died.
-      break;
-    }
+      return cookie_value;
   }
 
-  return cookie_value;
+  return std::string();
 }
 
 bool UITestBase::WaitUntilJavaScriptCondition(TabProxy* tab,
                                               const std::wstring& frame_xpath,
                                               const std::wstring& jscript,
-                                              int interval_ms,
                                               int time_out_ms) {
-  DCHECK_GE(time_out_ms, interval_ms);
-  DCHECK_GT(interval_ms, 0);
-  const int kMaxIntervals = time_out_ms / interval_ms;
+  const int kIntervalMs = 250;
+  const int kMaxIntervals = time_out_ms / kIntervalMs;
 
   // Wait until the test signals it has completed.
-  bool completed = false;
   for (int i = 0; i < kMaxIntervals; ++i) {
-    bool browser_survived = CrashAwareSleep(interval_ms);
-
+    bool browser_survived = CrashAwareSleep(kIntervalMs);
     EXPECT_TRUE(browser_survived);
     if (!browser_survived)
-      break;
+      return false;
 
     bool done_value = false;
     EXPECT_TRUE(tab->ExecuteAndExtractBool(frame_xpath, jscript, &done_value));
-
-    if (done_value) {
-      completed = true;
-      break;
-    }
+    if (done_value)
+      return true;
   }
 
-  return completed;
+  return false;
 }
 
 void UITestBase::WaitUntilTabCount(int tab_count) {
@@ -897,7 +881,6 @@ void UITestBase::WaitForFinish(const std::string &name,
                            const std::string& test_complete_cookie,
                            const std::string& expected_cookie_value,
                            const int wait_time) {
-  const int kIntervalMilliSeconds = 50;
   // The webpage being tested has javascript which sets a cookie
   // which signals completion of the test.  The cookie name is
   // a concatenation of the test name and the test id.  This allows
@@ -913,7 +896,7 @@ void UITestBase::WaitForFinish(const std::string &name,
   ASSERT_TRUE(tab.get());
   bool test_result = WaitUntilCookieValue(tab.get(), url,
                                           cookie_name.c_str(),
-                                          kIntervalMilliSeconds, wait_time,
+                                          wait_time,
                                           expected_cookie_value.c_str());
   EXPECT_EQ(true, test_result);
 }
diff --git a/chrome/test/ui/ui_test.h b/chrome/test/ui/ui_test.h
index ab5d2d8..940e061 100644
--- a/chrome/test/ui/ui_test.h
+++ b/chrome/test/ui/ui_test.h
@@ -164,7 +164,6 @@ class UITestBase {
   // - The time_out value has been exceeded.
   bool WaitUntilCookieValue(TabProxy* tab, const GURL& url,
                             const char* cookie_name,
-                            int interval_ms,
                             int time_out_ms,
                             const char* expected_value);
   // Polls the tab for the cookie_name cookie and returns once one of the
@@ -175,7 +174,6 @@ class UITestBase {
   std::string WaitUntilCookieNonEmpty(TabProxy* tab,
                                       const GURL& url,
                                       const char* cookie_name,
-                                      int interval_ms,
                                       int time_out_ms);
 
   // Polls the tab for a JavaScript condition and returns once one of the
@@ -189,7 +187,6 @@ class UITestBase {
   bool WaitUntilJavaScriptCondition(TabProxy* tab,
                                     const std::wstring& frame_xpath,
                                     const std::wstring& jscript,
-                                    int interval_ms,
                                     int time_out_ms);
 
   // Polls up to kWaitForActionMaxMsec ms to attain a specific tab count. Will
diff --git a/chrome/test/ui/v8_benchmark_uitest.cc b/chrome/test/ui/v8_benchmark_uitest.cc
index e52c330..792328e 100644
--- a/chrome/test/ui/v8_benchmark_uitest.cc
+++ b/chrome/test/ui/v8_benchmark_uitest.cc
@@ -60,7 +60,7 @@ class V8BenchmarkTest : public UITest {
   }
 
   bool WaitUntilTestCompletes(TabProxy* tab, const GURL& test_url) {
-    return WaitUntilCookieValue(tab, test_url, "__done", 1000,
+    return WaitUntilCookieValue(tab, test_url, "__done",
                                 UITest::test_timeout_ms(), "1");
   }
 
diff --git a/chrome/test/url_fetch_test/url_fetch_test.cc b/chrome/test/url_fetch_test/url_fetch_test.cc
index 9b757fd..ef2f392 100644
--- a/chrome/test/url_fetch_test/url_fetch_test.cc
+++ b/chrome/test/url_fetch_test/url_fetch_test.cc
@@ -52,12 +52,12 @@ class UrlFetchTest : public UITest {
     if (waitCookieName) {
       if (waitCookieValue) {
         bool completed = WaitUntilCookieValue(tab.get(), url, waitCookieName,
-                                              3000, UITest::test_timeout_ms(),
+                                              UITest::test_timeout_ms(),
                                               waitCookieValue);
         ASSERT_TRUE(completed);
       } else {
         result->cookie_value = WaitUntilCookieNonEmpty(
-            tab.get(), url, waitCookieName, 3000, UITest::test_timeout_ms());
+            tab.get(), url, waitCookieName, UITest::test_timeout_ms());
         ASSERT_TRUE(result->cookie_value.length());
       }
     }
diff --git a/chrome/worker/worker_uitest.cc b/chrome/worker/worker_uitest.cc
index 3bbce76..b7cb91e 100644
--- a/chrome/worker/worker_uitest.cc
+++ b/chrome/worker/worker_uitest.cc
@@ -27,7 +27,7 @@ class WorkerTest : public UILayoutTest {
     ASSERT_TRUE(tab->NavigateToURL(url));
 
     std::string value = WaitUntilCookieNonEmpty(tab.get(), url,
-        kTestCompleteCookie, kTestIntervalMs, action_max_timeout_ms());
+        kTestCompleteCookie, action_max_timeout_ms());
     ASSERT_STREQ(kTestCompleteSuccess, value.c_str());
   }
 
@@ -45,7 +45,7 @@ class WorkerTest : public UILayoutTest {
     ASSERT_TRUE(tab->NavigateToURL(url));
 
     std::string value = WaitUntilCookieNonEmpty(tab.get(), url,
-        kTestCompleteCookie, kTestIntervalMs, action_max_timeout_ms());
+        kTestCompleteCookie, action_max_timeout_ms());
 
     // Close the incognito window
     ASSERT_TRUE(incognito->RunCommand(IDC_CLOSE_WINDOW));
@@ -534,7 +534,7 @@ TEST_F(WorkerTest, DISABLED_WorkerClose) {
   GURL url = GetTestUrl(L"workers", L"worker_close.html");
   ASSERT_TRUE(tab->NavigateToURL(url));
   std::string value = WaitUntilCookieNonEmpty(tab.get(), url,
-      kTestCompleteCookie, kTestIntervalMs, action_max_timeout_ms());
+      kTestCompleteCookie, action_max_timeout_ms());
   ASSERT_STREQ(kTestCompleteSuccess, value.c_str());
   ASSERT_TRUE(WaitForProcessCountToBe(1, 0));
 }
@@ -550,7 +550,7 @@ TEST_F(WorkerTest, QueuedSharedWorkerShutdown) {
   ASSERT_TRUE(tab.get());
   ASSERT_TRUE(tab->NavigateToURL(url));
   std::string value = WaitUntilCookieNonEmpty(tab.get(), url,
-      kTestCompleteCookie, kTestIntervalMs, action_max_timeout_ms());
+      kTestCompleteCookie, action_max_timeout_ms());
   ASSERT_STREQ(kTestCompleteSuccess, value.c_str());
   ASSERT_TRUE(WaitForProcessCountToBe(1, max_workers_per_tab));
 }
@@ -579,7 +579,7 @@ TEST_F(WorkerTest, MultipleTabsQueuedSharedWorker) {
   ASSERT_TRUE(window->AppendTab(url2));
 
   std::string value = WaitUntilCookieNonEmpty(tab.get(), url,
-      kTestCompleteCookie, kTestIntervalMs, action_max_timeout_ms());
+      kTestCompleteCookie, action_max_timeout_ms());
   ASSERT_STREQ(kTestCompleteSuccess, value.c_str());
   ASSERT_TRUE(WaitForProcessCountToBe(3, max_workers_per_tab));
 }
@@ -605,7 +605,7 @@ TEST_F(WorkerTest, DISABLED_QueuedSharedWorkerStartedFromOtherTab) {
   ASSERT_TRUE(window->AppendTab(url2));
 
   std::string value = WaitUntilCookieNonEmpty(tab.get(), url,
-      kTestCompleteCookie, kTestIntervalMs, action_max_timeout_ms());
+      kTestCompleteCookie, action_max_timeout_ms());
   ASSERT_STREQ(kTestCompleteSuccess, value.c_str());
   ASSERT_TRUE(WaitForProcessCountToBe(2, max_workers_per_tab+1));
 }