Use ExecuteJavascriptInWebFrameNotifyResult to return results even in error condition

chrome/browser/ui/webui/web_ui_test_handler.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/web_ui_test_handler.h"
 
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/test/ui_test_utils.h"
 #include "content/browser/renderer_host/render_view_host.h"
+#include "content/common/notification_details.h"
+#include "content/common/notification_registrar.h"
 
 bool WebUITestHandler::RunJavascript(const std::string& js_test,
                                             bool is_test) {

[Paweł Hajdan Jr., 2011/06/15 08:18:21]

nit: Please align this line.

[Sheridan Rawlins, 2011/06/15 15:41:14]

Done.

-  web_ui_->GetRenderViewHost()->ExecuteJavascriptInWebFrame(
-      string16(), UTF8ToUTF16(js_test));
-
-  if (is_test)
+  if (is_test) {
+    NotificationRegistrar notification_registrar;
+    notification_registrar.Add(
+        this, NotificationType::EXECUTE_JAVASCRIPT_RESULT,
+        Source<RenderViewHost>(web_ui_->GetRenderViewHost()));
+    web_ui_->GetRenderViewHost()->ExecuteJavascriptInWebFrameNotifyResult(
+        string16(), UTF8ToUTF16(js_test));
     return WaitForResult();
-  else
+  } else {
+    web_ui_->GetRenderViewHost()->ExecuteJavascriptInWebFrame(
+        string16(), UTF8ToUTF16(js_test));
     return true;
+  }
 }
 
-void WebUITestHandler::HandlePass(const ListValue* args) {
-  test_succeeded_ = true;
-  if (is_waiting_)
-    MessageLoopForUI::current()->Quit();
-}
+void WebUITestHandler::Observe(NotificationType type,
+                               const NotificationSource& source,
+                               const NotificationDetails& details) {
+  // Always quit the message loop so Waiting process can get result or error.
+  MessageLoopForUI::current()->Quit();
 
-void WebUITestHandler::HandleFail(const ListValue* args) {
-  test_succeeded_ = false;
-  if (is_waiting_)
-    MessageLoopForUI::current()->Quit();
-
-  std::string message;
-  ASSERT_TRUE(args->GetString(0, &message));
-  LOG(ERROR) << message;
-}
-
-void WebUITestHandler::RegisterMessages() {
-  web_ui_->RegisterMessageCallback("Pass",
-      NewCallback(this, &WebUITestHandler::HandlePass));
-  web_ui_->RegisterMessageCallback("Fail",
-      NewCallback(this, &WebUITestHandler::HandleFail));
+  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;
+  }
 }
 
 bool WebUITestHandler::WaitForResult() {
   is_waiting_ = true;

[Paweł Hajdan Jr., 2011/06/15 08:18:21]

The new version of the code doesn't seem to use is_waiting_, could you
completely remove that member variable?

[Sheridan Rawlins, 2011/06/15 15:41:14]

Nice catch - actually is_waiting_ is needed.  I'll add it to the Observe.

On 2011/06/15 08:18:21, Paweł Hajdan Jr. wrote:

   ui_test_utils::RunMessageLoop();
   is_waiting_ = false;
   return test_succeeded_;
 }