Add option for url_fetch_test to wait for a JS expression...

chrome/test/url_fetch_test/url_fetch_test.cc

Index: chrome/test/url_fetch_test/url_fetch_test.cc
===================================================================
--- chrome/test/url_fetch_test/url_fetch_test.cc	(revision 70404)
+++ chrome/test/url_fetch_test/url_fetch_test.cc	(working copy)
@@ -6,6 +6,7 @@
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
+#include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/chrome_paths.h"
@@ -15,7 +16,9 @@
 namespace {
 
 // Provides a UI Test that lets us take the browser to a url, and
-// wait for a cookie value to be set before closing the page.
+// wait for a cookie value to be set or a JavaScript expression to evaluate
+// true before closing the page. It is undefined what happens if you specify
+// both a cookie and a JS expression.
 class UrlFetchTest : public UITest {
  public:
   UrlFetchTest() {
@@ -45,10 +48,16 @@
     UITest::SetUp();
   }
 
-  void RunTest(const GURL& url, const char* wait_cookie_name,
-               const char* wait_cookie_value, const char* var_to_fetch,
+  void RunTest(const GURL& url,
+               const char* wait_cookie_name,
+               const char* wait_cookie_value,
+               const char* var_to_fetch,
+               const std::wstring& wait_js_expr,
+               const std::wstring& wait_js_frame_xpath,
+               int wait_js_timeout_ms,
                UrlFetchTestResult* result) {
     scoped_refptr<TabProxy> tab(GetActiveTab());
+

[tfarina, 2011/01/13 18:57:50]

I think you removed this line for readability, but I don't think it's necessary.

     ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, tab->NavigateToURL(url));
 
     if (wait_cookie_name) {
@@ -62,6 +71,12 @@
             tab.get(), url, wait_cookie_name, UITest::test_timeout_ms());
         ASSERT_TRUE(result->cookie_value.length());
       }
+    } else if (!wait_js_expr.empty()) {
+      bool completed = WaitUntilJavaScriptCondition(tab.get(),
+                                                    wait_js_frame_xpath,
+                                                    wait_js_expr,
+                                                    wait_js_timeout_ms);
+      ASSERT_TRUE(completed);
     }
     if (var_to_fetch) {
       std::string script = StringPrintf(
@@ -99,6 +114,22 @@
 //   In conjunction with --wait_cookie_name, this saves the cookie value to
 //   a file at the given path. (Incompatible with --wait_cookie_value)
 //
+// --wait_js_expr=<jscript_expr>
+//   Waits for a javascript expression to evaluate true before exiting
+//   successfully.
+//
+// --wait_js_timeout=<timeout_ms>
+//   In conjunction with --wait_js_condition, this sets the timeout in ms
+//   that we are prepared to wait. If this timeout is exceeded, we will exit
+//   with failure. Note that a timeout greater than the gtest timeout will not
+//   be honored.
+//
+// --wait_js_frame_xpath=<xpath>
+//

[tfarina, 2011/01/13 18:57:50]

As you did with the other switches, I think you can remove this blank line.

+//   In conjuction with --wait_js_condition, the JavaScript expression is
+//   executed in the context of the frame that matches the provided xpath.
+//   If this is not specified (or empty string), then the main frame is used.
+
 // --jsvar=<name>
 //   At the end of the test, fetch the named javascript variable from the page.
 //
@@ -118,15 +149,26 @@
       cmd_line->GetSwitchValueASCII("wait_cookie_name");
   std::string cookie_value =
       cmd_line->GetSwitchValueASCII("wait_cookie_value");
+  std::wstring js_expr =
+      UTF8ToWide(cmd_line->GetSwitchValueASCII("wait_js_expr"));
+  std::wstring js_frame_xpath =
+      UTF8ToWide(cmd_line->GetSwitchValueASCII("wait_js_frame_xpath"));
+  std::string js_timeout_ms_str =
+      cmd_line->GetSwitchValueASCII("wait_js_timeout");
 
   std::string jsvar = cmd_line->GetSwitchValueASCII("jsvar");
+  int js_timeout_ms = -1;  // no timeout, wait forever
 
+  if (!js_timeout_ms_str.empty()) {

[tfarina, 2011/01/13 18:57:50]

Please, remove these {}, they are not necessary.

+    base::StringToInt(js_timeout_ms_str, &js_timeout_ms);
+  }
+
   UrlFetchTestResult result;
   RunTest(GURL(cmd_line->GetSwitchValueASCII("url")),
           cookie_name.length() > 0 ? cookie_name.c_str() : NULL,
           cookie_value.length() > 0 ? cookie_value.c_str() : NULL,
           jsvar.length() > 0 ? jsvar.c_str() : NULL,
-          &result);
+          js_expr, js_frame_xpath, js_timeout_ms, &result);

[tfarina, 2011/01/13 18:57:50]

A minor style issue, although I don't think it's extremely necessary, I think
you could write one arg per line. It already looks a mess these lines.

 
   // Write out the cookie if requested
   FilePath cookie_output_path =