Use the ExecuteJavascriptInWebFrameNotifyResult to return results rather than domui.

Also fixes the hanging issue by allowing result to returned always even when javascript to be executed is bogus.

BUG=86074,85990
R=dtseng@chromium.org
TEST=browser_tests --gtest_filter=WebUIBrowserTest.*


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

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

6 files changed, 88 insertions, 55 deletions

diff --git a/chrome/browser/ui/webui/web_ui_browsertest.cc b/chrome/browser/ui/webui/web_ui_browsertest.cc
index 15af6f2..66815d0 100644
--- a/chrome/browser/ui/webui/web_ui_browsertest.cc
+++ b/chrome/browser/ui/webui/web_ui_browsertest.cc
@@ -16,6 +16,7 @@
 #include "chrome/test/ui_test_utils.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/browser/webui/web_ui.h"
+#include "testing/gtest/include/gtest/gtest-spi.h"
 #include "ui/base/resource/resource_bundle.h"
 
 namespace {
@@ -204,6 +205,42 @@ IN_PROC_BROWSER_TEST_F(WebUIBrowserTest, TestSamplePass) {
   ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIDownloadsURL));
 
   ASSERT_TRUE(RunJavascriptTest("testAssertFalse"));
+  ASSERT_FALSE(RunJavascriptTest("FAILS_testAssertFalse"));
   ASSERT_TRUE(RunJavascriptTest("testInitialFocus"));
   ASSERT_FALSE(RunJavascriptTest("testConsoleError"));
 }
+
+// According to the interface for EXPECT_FATAL_FAILURE
+// (http://code.google.com/p/googletest/wiki/AdvancedGuide#Catching_Failures)
+// the statement must be statically available. Therefore, we make a static
+// global s_test_ which should point to |this| for the duration of the test run
+// and be cleared afterward.
+class WebUIBrowserExpectFailTest : public WebUIBrowserTest {
+ public:
+  WebUIBrowserExpectFailTest() {
+    EXPECT_FALSE(s_test_);
+    s_test_ = this;
+  }
+
+ protected:
+  virtual ~WebUIBrowserExpectFailTest() {
+    EXPECT_TRUE(s_test_);
+    s_test_ = NULL;
+  }
+
+  static void RunJavascriptTestNoReturn(const std::string& testname) {
+    EXPECT_TRUE(s_test_);
+    s_test_->RunJavascriptTest(testname);
+  }
+
+ private:
+  static WebUIBrowserTest* s_test_;
+};
+WebUIBrowserTest* WebUIBrowserExpectFailTest::s_test_ = NULL;
+
+IN_PROC_BROWSER_TEST_F(WebUIBrowserExpectFailTest, TestFailsFast) {
+  AddLibrary(FilePath(FILE_PATH_LITERAL("sample_downloads.js")));
+  ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIDownloadsURL));
+  EXPECT_FATAL_FAILURE(RunJavascriptTestNoReturn("FAILS_BogusFunctionName"),
+                       "WebUITestHandler::Observe");
+}
diff --git a/chrome/browser/ui/webui/web_ui_test_handler.cc b/chrome/browser/ui/webui/web_ui_test_handler.cc
index 29eb98c..4213487 100644
--- a/chrome/browser/ui/webui/web_ui_test_handler.cc
+++ b/chrome/browser/ui/webui/web_ui_test_handler.cc
@@ -8,39 +8,43 @@
 #include "base/values.h"
 #include "chrome/test/ui_test_utils.h"
 #include "content/browser/renderer_host/render_view_host.h"
-
-bool WebUITestHandler::RunJavascript(const std::string& js_test,
-                                            bool is_test) {
-  web_ui_->GetRenderViewHost()->ExecuteJavascriptInWebFrame(
-      string16(), UTF8ToUTF16(js_test));
-
-  if (is_test)
+#include "content/common/notification_details.h"
+#include "content/common/notification_registrar.h"
+
+bool WebUITestHandler::RunJavascript(const std::string& js_test, bool 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::HandleFail(const ListValue* args) {
-  test_succeeded_ = false;
+void WebUITestHandler::Observe(NotificationType type,
+                               const NotificationSource& source,
+                               const NotificationDetails& details) {
+  // 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_)
     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));
+  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;
+  }
 }
 
 bool WebUITestHandler::WaitForResult() {
diff --git a/chrome/browser/ui/webui/web_ui_test_handler.h b/chrome/browser/ui/webui/web_ui_test_handler.h
index b0135d2..304f6d4 100644
--- a/chrome/browser/ui/webui/web_ui_test_handler.h
+++ b/chrome/browser/ui/webui/web_ui_test_handler.h
@@ -2,31 +2,32 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CONTENT_BROWSER_WEBUI_WEB_UI_HANDLER_BROWSERTEST_H_
-#define CONTENT_BROWSER_WEBUI_WEB_UI_HANDLER_BROWSERTEST_H_
+#ifndef CHROME_BROWSER_UI_WEBUI_WEB_UI_TEST_HANDLER_H_
+#define CHROME_BROWSER_UI_WEBUI_WEB_UI_TEST_HANDLER_H_
 #pragma once
 
 #include <string>
 
 #include "content/browser/webui/web_ui.h"
+#include "content/common/notification_observer.h"
 
 // This class registers test framework specific handlers on WebUI objects.
-class WebUITestHandler : public WebUIMessageHandler {
+class WebUITestHandler : public WebUIMessageHandler,
+                         public NotificationObserver {
  public:
   // Runs a string of javascript. Returns pass fail.
   bool RunJavascript(const std::string& js_test, bool is_test);
 
- protected:
-  // WebUI handlers which deliver results to any waiting message loops.
-  // |args| is currently ignored.
-  void HandlePass(const ListValue* args);
-  void HandleFail(const ListValue* args);
-
+ private:
   // WebUIMessageHandler overrides.
   // Add test handlers to the current WebUI object.
-  virtual void RegisterMessages();
+  virtual void RegisterMessages() {}
+
+  // From NotificationObserver.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) OVERRIDE;
 
- private:
   // Runs a message loop until test finishes.  Returns the result of the test.
   bool WaitForResult();
 
@@ -37,4 +38,4 @@ class WebUITestHandler : public WebUIMessageHandler {
   bool is_waiting_;
 };
 
-#endif  // CONTENT_BROWSER_WEBUI_WEB_UI_HANDLER_BROWSERTEST_H_
+#endif  // CHROME_BROWSER_UI_WEBUI_WEB_UI_TEST_HANDLER_H_
diff --git a/chrome/test/data/webui/sample_downloads.js b/chrome/test/data/webui/sample_downloads.js
index 0d976f3..5d4dad9 100644
--- a/chrome/test/data/webui/sample_downloads.js
+++ b/chrome/test/data/webui/sample_downloads.js
@@ -8,6 +8,11 @@ function testAssertFalse() {
   assertFalse(false);
 }
 
+function FAILS_testAssertFalse() {
+  assertFalse(true);
+  assertFalse(false);
+}
+
 function testInitialFocus() {
   assertTrue(document.activeElement.id == 'term', '');
 }
diff --git a/chrome/test/data/webui/test_api.js b/chrome/test/data/webui/test_api.js
index 27d94f8..1058a26 100644
--- a/chrome/test/data/webui/test_api.js
+++ b/chrome/test/data/webui/test_api.js
@@ -5,16 +5,6 @@
 // Library providing basic test framework functionality.
 
 (function() {
-  // Indicates a pass to the C++ backend.
-  function pass() {
-    chrome.send('Pass', []);
-  }
-
-  // Indicates a fail to the C++ backend.
-  function fail(message) {
-    chrome.send('Fail', [String(message)]);
-  }
-
   // Asserts.
   // Use the following assertions to verify a condition within a test.
   // If assertion fails, the C++ backend will be immediately notified.
@@ -66,14 +56,10 @@
     } catch (e) {
       console.log(
           'Failed: ' + currentTest.name + '\nwith exception: ' + e.message);
-
-      fail(e.message);
-      return;
+      return [false, e.message] ;
     }
 
-
-    // All tests passed.
-    pass('');
+    return [true];
   }
 
   // Exports.
diff --git a/content/renderer/render_view.cc b/content/renderer/render_view.cc
index 2def6db..fc83a1d 100644
--- a/content/renderer/render_view.cc
+++ b/content/renderer/render_view.cc
@@ -3279,7 +3279,7 @@ void RenderView::EvaluateScript(const string16& frame_xpath,
     result = web_frame->executeScriptAndReturnValue(WebScriptSource(script));
   if (notify_result) {
     ListValue list;
-    if (web_frame) {
+    if (!result.IsEmpty() && web_frame) {
       v8::HandleScope handle_scope;
       v8::Local<v8::Context> context = web_frame->mainWorldScriptContext();
       v8::Context::Scope context_scope(context);