Make ui_test_utils::ExecuteJavaScript() friendlier to use. Also, add WARN_UNUSED_RESULT to make it less likely that callers ignore failures.

BUG=57216
TEST=tests should still run

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

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

1 files changed, 66 insertions, 42 deletions

diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
index 964ae60..f570707 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -109,13 +109,11 @@ class DOMOperationObserver : public NotificationObserver {
     MessageLoopForUI::current()->Quit();
   }
 
-  bool GetResponse(std::string* response) {
+  bool GetResponse(std::string* response) WARN_UNUSED_RESULT {
     *response = response_;
     return did_respond_;
   }
 
-  std::string response() const { return response_; }
-
  private:
   NotificationRegistrar registrar_;
   std::string response_;
@@ -277,18 +275,18 @@ class InProcessJavaScriptExecutionController
 
  protected:
   // Executes |script| and sets the JSON response |json|.
-  bool ExecuteJavaScriptAndGetJSON(const std::string& script,
-                                   std::string* json) {
+  virtual bool ExecuteJavaScriptAndGetJSON(const std::string& script,
+                                           std::string* json) {
     render_view_host_->ExecuteJavascriptInWebFrame(L"", UTF8ToWide(script));
     DOMOperationObserver dom_op_observer(render_view_host_);
     return dom_op_observer.GetResponse(json);
   }
 
-  void FirstObjectAdded() {
+  virtual void FirstObjectAdded() {
     AddRef();
   }
 
-  void LastObjectRemoved() {
+  virtual void LastObjectRemoved() {
     Release();
   }
 
@@ -297,6 +295,52 @@ class InProcessJavaScriptExecutionController
   RenderViewHost* render_view_host_;
 };
 
+// Specifying a prototype so that we can add the WARN_UNUSED_RESULT attribute.
+bool ExecuteJavaScriptHelper(RenderViewHost* render_view_host,
+                             const std::wstring& frame_xpath,
+                             const std::wstring& original_script,
+                             scoped_ptr<Value>* result) WARN_UNUSED_RESULT;
+
+// Executes the passed |original_script| in the frame pointed to by
+// |frame_xpath|.  If |result| is not NULL, stores the value that the evaluation
+// of the script in |result|.  Returns true on success.
+bool ExecuteJavaScriptHelper(RenderViewHost* render_view_host,
+                             const std::wstring& frame_xpath,
+                             const std::wstring& original_script,
+                             scoped_ptr<Value>* result) {
+  // TODO(jcampan): we should make the domAutomationController not require an
+  //                automation id.
+  std::wstring script = L"window.domAutomationController.setAutomationId(0);" +
+      original_script;
+  render_view_host->ExecuteJavascriptInWebFrame(frame_xpath, script);
+  DOMOperationObserver dom_op_observer(render_view_host);
+  std::string json;
+  if (!dom_op_observer.GetResponse(&json))
+    return false;
+
+  // Nothing more to do for callers that ignore the returned JS value.
+  if (!result)
+    return true;
+
+  // Wrap |json| in an array before deserializing because valid JSON has an
+  // array or an object as the root.
+  json.insert(0, "[");
+  json.append("]");
+
+  scoped_ptr<Value> root_val(base::JSONReader::Read(json, true));
+  if (!root_val->IsType(Value::TYPE_LIST))
+    return false;
+
+  ListValue* list = static_cast<ListValue*>(root_val.get());
+  Value* result_val;
+  if (!list || !list->GetSize() ||
+      !list->Remove(0, &result_val))  // Remove gives us ownership of the value.
+    return false;
+
+  result->reset(result_val);
+  return true;
+}
+
 }  // namespace
 
 void RunMessageLoop() {
@@ -410,32 +454,12 @@ DOMElementProxyRef GetActiveDOMDocument(Browser* browser) {
   return executor->GetObjectProxy<DOMElementProxy>(element_handle);
 }
 
-Value* ExecuteJavaScript(RenderViewHost* render_view_host,
-                         const std::wstring& frame_xpath,
-                         const std::wstring& original_script) {
-  // TODO(jcampan): we should make the domAutomationController not require an
-  //                automation id.
-  std::wstring script = L"window.domAutomationController.setAutomationId(0);" +
-      original_script;
-  render_view_host->ExecuteJavascriptInWebFrame(frame_xpath, script);
-  DOMOperationObserver dom_op_observer(render_view_host);
-  std::string json = dom_op_observer.response();
-  // Wrap |json| in an array before deserializing because valid JSON has an
-  // array or an object as the root.
-  json.insert(0, "[");
-  json.append("]");
-
-  scoped_ptr<Value> root_val(base::JSONReader::Read(json, true));
-  if (!root_val->IsType(Value::TYPE_LIST))
-    return NULL;
-
-  ListValue* list = static_cast<ListValue*>(root_val.get());
-  Value* result;
-  if (!list || !list->GetSize() ||
-      !list->Remove(0, &result))  // Remove gives us ownership of the value.
-    return NULL;
-
-  return result;
+bool ExecuteJavaScript(RenderViewHost* render_view_host,
+                       const std::wstring& frame_xpath,
+                       const std::wstring& original_script) {
+  std::wstring script =
+      original_script + L"window.domAutomationController.send(0);";
+  return ExecuteJavaScriptHelper(render_view_host, frame_xpath, script, NULL);
 }
 
 bool ExecuteJavaScriptAndExtractInt(RenderViewHost* render_view_host,
@@ -443,9 +467,9 @@ bool ExecuteJavaScriptAndExtractInt(RenderViewHost* render_view_host,
                                     const std::wstring& script,
                                     int* result) {
   DCHECK(result);
-  scoped_ptr<Value> value(ExecuteJavaScript(render_view_host, frame_xpath,
-                                            script));
-  if (!value.get())
+  scoped_ptr<Value> value;
+  if (!ExecuteJavaScriptHelper(render_view_host, frame_xpath, script, &value) ||
+      !value.get())
     return false;
 
   return value->GetAsInteger(result);
@@ -456,9 +480,9 @@ bool ExecuteJavaScriptAndExtractBool(RenderViewHost* render_view_host,
                                      const std::wstring& script,
                                      bool* result) {
   DCHECK(result);
-  scoped_ptr<Value> value(ExecuteJavaScript(render_view_host, frame_xpath,
-                                            script));
-  if (!value.get())
+  scoped_ptr<Value> value;
+  if (!ExecuteJavaScriptHelper(render_view_host, frame_xpath, script, &value) ||
+      !value.get())
     return false;
 
   return value->GetAsBoolean(result);
@@ -469,9 +493,9 @@ bool ExecuteJavaScriptAndExtractString(RenderViewHost* render_view_host,
                                        const std::wstring& script,
                                        std::string* result) {
   DCHECK(result);
-  scoped_ptr<Value> value(ExecuteJavaScript(render_view_host, frame_xpath,
-                                            script));
-  if (!value.get())
+  scoped_ptr<Value> value;
+  if (!ExecuteJavaScriptHelper(render_view_host, frame_xpath, script, &value) ||
+      !value.get())
     return false;
 
   return value->GetAsString(result);