Provide ability for WebUIBrowserTests to run asynchronous tests.

chrome/browser/ui/webui/web_ui_test_handler.cc

Index: chrome/browser/ui/webui/web_ui_test_handler.cc
diff --git a/chrome/browser/ui/webui/web_ui_test_handler.cc b/chrome/browser/ui/webui/web_ui_test_handler.cc
index 14b407fa43b14fe7a8bdac81af2a560ec979c991..d5ef95d7fe0de2efdb1d5ffd7607c28b59d8dacd 100644
--- a/chrome/browser/ui/webui/web_ui_test_handler.cc
+++ b/chrome/browser/ui/webui/web_ui_test_handler.cc
@@ -12,6 +12,14 @@
 #include "content/common/notification_details.h"
 #include "content/common/notification_registrar.h"
 
+WebUITestHandler::WebUITestHandler()
+    : test_succeeded_(false),
+      is_waiting_(false),
+      test_done_async_(false),
+      test_succeeded_async_(false),
+      is_waiting_async_(false) {
+}
+
 void WebUITestHandler::PreloadJavaScript(const string16& js_text,
                                          RenderViewHost* preload_host) {
   DCHECK(preload_host);
@@ -35,6 +43,44 @@ bool WebUITestHandler::RunJavaScriptTestWithResult(const string16& js_text) {
   return WaitForResult();
 }
 
+bool WebUITestHandler::WaitForAsyncResult() {

[David Tseng, 2011/08/05 13:45:40]

This looks virtually identical to WaitForResult. It would be cleaner if we got
rid of this codepath (and it's not really async since we're waiting via a
message loop). Its about as async as WaitForResult which is to say not (in the
context of C++).

You could just have the test call WaitForResult directly...

[Sheridan Rawlins, 2011/08/05 15:41:59]

The distinction is that our "normal" RunJavascriptTest runs the test and returns
- that's it.

Matt needed a way to let javascript register handlers and have some over/back
between the handler before the test finishes.  In that aspect, the C++ and the
Javascript are asynchronous, and running the message loop is a pattern to wait
for completion while letting messages run. Effectively both sides hook up their
message handlers, but each handler needs to return in order for their respective
message loops to continue.  Javascript doesn't have a way to run the message
loop recursively, so we needed a way to let the call to RunJavascriptTest return
and the test continue somehow, and let us know when it's done via a chrome.send
message.

We felt that a separate wait loop was required because the setup call will
return with one message, and the asyncTestDone() will return another.  While it
is not intended, it _is_ possible that something in the test causes
asyncTestDone() to be called before the RunJavascriptTest and that method needs
to store its done state independently of its waiting state to handle this out of
order ("async"-like) invocation.

On 2011/08/05 13:45:40, David Tseng wrote:

+  if (!test_done_async_) {
+    is_waiting_async_ = true;
+    ui_test_utils::RunMessageLoop();
+    is_waiting_async_ = false;
+  }
+  return test_succeeded_async_;
+};
+
+void WebUITestHandler::RegisterMessages() {
+  web_ui_->RegisterMessageCallback("asyncTestResult", NewCallback(
+      this, &WebUITestHandler::HandleAsyncTestResult));
+}
+
+void WebUITestHandler::HandleAsyncTestResult(const ListValue* test_result) {
+  // Quit the message loop if we were waiting so Waiting process can get result
+  // or error. To ensure this gets done, do this before ASSERT* calls.
+  if (is_waiting_async_)

[David Tseng, 2011/08/05 13:45:40]

Why do we need separate *async versions of these booleans? Is there ever a
chance that WaitForResult calls WaitForAsyncResult (or vice versa)?

[Sheridan Rawlins, 2011/08/05 15:41:59]

We need separate booleans because the messages are different and may be called
in either order.

On 2011/08/05 13:45:40, David Tseng wrote:

+    MessageLoopForUI::current()->Quit();
+
+  EXPECT_FALSE(test_done_async_);
+  test_done_async_ = true;

[David Tseng, 2011/08/05 13:45:40]

What's the difference between test_done_async and is_waiting_async? Isn't
test_done_async == !is_waiting_async? And, again, why are we using dup versions
of these booleans? I don't think we ever stack message loops.

[mmenke, 2011/08/05 13:54:18]

The async result could theoretically be reported before the synchronous callback
exits the message loop, so it needs its own bool.  The need for
WaitForAsyncResult follows from that.  That having been said, looks like
|is_waiting_async_| and |is_waiting_| could be merged, though might be a good
idea to throw in an ASSERT to vertify that they're false when entering the wait
functions.

[Sheridan Rawlins, 2011/08/05 15:41:59]

I like duplicating the booleans to be explicit about the messages and following
the pattern seen elsewhere in various wait utilities (see ui_test_utils).

Pattern for async stuff is 2 booleans (done, running) 1 return-value (result).
We ditched the done variable for our WaitForResult because we knew that the only
way for our callback to occur is when we were running the message loop, whereas
the callback for WaitForAsyncResult could occur while we're waiting for _this_
message loop.

It seemed easier to maintain if we split the two loops than trying to have the
logic and corresponding calls (passing a flag/boolean all the way through
RunJavascriptTest) in WaitForResult().  Afterall, the C++ side knows whether
it's kicking off asynchronous stuff and can just call the wait after it's primed
the test's JS handlers with RunJavascriptTest().

On 2011/08/05 13:54:18, Matt Menke wrote:

+
+  SCOPED_TRACE("WebUITestHandler::HandleAsyncTestResult");
+  HandleTestResult(test_result, &test_succeeded_async_);
+}
+
+void WebUITestHandler::HandleTestResult(const ListValue* test_result,
+                                        bool* success) {
+  *success = false;
+  ASSERT_TRUE(test_result->GetBoolean(0, success));
+  if (!success) {
+    std::string message;
+    ASSERT_TRUE(test_result->GetString(1, &message));
+    LOG(ERROR) << message;
+  }
+}
+
 void WebUITestHandler::Observe(int type,
                                const NotificationSource& source,
                                const NotificationDetails& details) {
@@ -45,14 +91,9 @@ void WebUITestHandler::Observe(int type,
 
   SCOPED_TRACE("WebUITestHandler::Observe");
   Value* value = Details<std::pair<int, Value*> >(details)->second;
-  ListValue* list_value;
-  ASSERT_TRUE(value->GetAsList(&list_value));
-  ASSERT_TRUE(list_value->GetBoolean(0, &test_succeeded_));
-  if (!test_succeeded_) {
-    std::string message;
-    ASSERT_TRUE(list_value->GetString(1, &message));
-    LOG(ERROR) << message;
-  }
+  ListValue* test_result;
+  ASSERT_TRUE(value->GetAsList(&test_result));
+  HandleTestResult(test_result, &test_succeeded_);
 }
 
 bool WebUITestHandler::WaitForResult() {