Revert 175489

[javac] /b/build/slave/Android_Builder__dbg_/build/src/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewClientWrapper.java:62: cannot find symbol
[javac] symbol  : method onEvaluateJavaScriptResult(int,java.lang.String)
[javac] location: class org.chromium.content.browser.test.util.TestContentViewClient
[javac]         super.onEvaluateJavaScriptResult(id, jsonResult);
[javac]              ^
[javac] /b/build/slave/Android_Builder__dbg_/build/src/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewClientWrapper.java:63: cannot find symbol
[javac] symbol  : method onEvaluateJavaScriptResult(int,java.lang.String)
[javac] location: class org.chromium.content.browser.ContentViewClient
[javac]         mWrappedClient.onEvaluateJavaScriptResult(id, jsonResult);
[javac]                       ^
[javac] /b/build/slave/Android_Builder__dbg_/build/src/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewClientWrapper.java:60: method does not override or implement a method from a supertype
[javac]     @Override
[javac]     ^
[javac] 3 errors

> Switch Android code to use ExecuteJavascriptInWebFrameCallbackResult.
> 
> BUG=168169
> TEST=should all pass
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/11788005

TBR=avi@chromium.org
Review URL: https://codereview.chromium.org/11778034

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

android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java

@@ -72,6 +72,12 @@ class TestAwContentsClient extends NullContentsClient {
         mOnReceivedErrorHelper.notifyCalled(errorCode, description, failingUrl);
     }
 
+    @Override
+    public void onEvaluateJavaScriptResult(int id, String jsonResult) {
+        super.onEvaluateJavaScriptResult(id, jsonResult);
+        mOnEvaluateJavaScriptResultHelper.notifyCalled(id, jsonResult);
+    }
+
     @Override
     public boolean onConsoleMessage(ConsoleMessage consoleMessage) {
         mAddMessageToConsoleHelper.setLevel(consoleMessage.messageLevel().ordinal());

android_webview/javatests/src/org/chromium/android_webview/test/util/JSUtils.java

@@ -51,8 +51,7 @@ public class JSUtils {
                     "var evObj = document.createEvent('Events'); " +
                     "evObj.initEvent('click', true, false); " +
                     "document.getElementById('" + linkId + "').dispatchEvent(evObj);" +
-                    "console.log('element with id [" + linkId + "] clicked');",
-                    null);
+                    "console.log('element with id [" + linkId + "] clicked');");
             }
         });
     }

build/android/findbugs_filter/findbugs_exclude.xml

@@ -19,10 +19,4 @@ In particular, ~ at the start of a string means it's a regex.
   </Match>
   <!-- Ignore "reliance on default String encoding" warnings, as we're not multi-platform -->
   <Bug pattern="DM_DEFAULT_ENCODING" />
-  <!-- Disabled due to impossible false positives found in -->
-  <!-- https://codereview.chromium.org/11788005/ (look in comments) -->
-  <Match>
-    <Method name="handleMessage" params="android.os.Message" returns="void" />
-    <Bug pattern="BC_IMPOSSIBLE_CAST" />
-  </Match>
 </FindBugsFilter>

content/browser/android/content_view_core_impl.cc

@@ -11,7 +11,6 @@
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
-#include "base/values.h"
 #include "cc/layer.h"
 #include "content/browser/android/interstitial_page_delegate_android.h"
 #include "content/browser/android/load_url_params.h"
@@ -175,6 +174,10 @@ ContentViewCoreImpl::ContentViewCoreImpl(JNIEnv* env, jobject obj,
     dpi_scale_ = device_info->GetDPIScale();
   }
 
+  notification_registrar_.Add(this,
+                              NOTIFICATION_EXECUTE_JAVASCRIPT_RESULT,
+                              NotificationService::AllSources());
+
   // Currently, the only use case we have for overriding a user agent involves
   // spoofing a desktop Linux user agent for "Request desktop site".
   // Automatically set it for all WebContents so that it is available when a
@@ -269,6 +272,25 @@ void ContentViewCoreImpl::Observe(int type,
       }
       break;
     }
+    case NOTIFICATION_EXECUTE_JAVASCRIPT_RESULT: {
+      if (!web_contents_ || Source<RenderViewHost>(source).ptr() !=
+          web_contents_->GetRenderViewHost()) {
+        return;
+      }
+
+      JNIEnv* env = base::android::AttachCurrentThread();
+      std::pair<int, Value*>* result_pair =
+          Details<std::pair<int, Value*> >(details).ptr();
+      std::string json;
+      base::JSONWriter::Write(result_pair->second, &json);
+      ScopedJavaLocalRef<jstring> j_json = ConvertUTF8ToJavaString(env, json);
+      ScopedJavaLocalRef<jobject> j_obj = java_ref_.get(env);
+      if (!j_obj.is_null()) {
+        Java_ContentViewCore_onEvaluateJavaScriptResult(env, j_obj.obj(),
+            static_cast<jint>(result_pair->first), j_json.obj());
+      }
+      break;
+    }
   }
 }
 
@@ -1219,46 +1241,15 @@ void ContentViewCoreImpl::UndoScrollFocusedEditableNodeIntoView(
       new ViewMsg_UndoScrollFocusedEditableNodeIntoView(host->GetRoutingID()));
 }
 
-namespace {
-void JavaScriptResultCallback(ScopedJavaGlobalRef<jobject>* message,
-                              const base::Value* result) {
-  // |message| is passed as base::Owned, so it will automatically be deleted
-  // when the callback goes out of scope.
-  JNIEnv* env = base::android::AttachCurrentThread();
-  std::string json;
-  base::JSONWriter::Write(result, &json);
-  ScopedJavaLocalRef<jstring> j_json = ConvertUTF8ToJavaString(env, json);
-  Java_ContentViewCore_onEvaluateJavaScriptResult(env,
-                                                  j_json.obj(),
-                                                  message->obj());
-}
-}  // namespace
-
-void ContentViewCoreImpl::EvaluateJavaScript(JNIEnv* env,
+jint ContentViewCoreImpl::EvaluateJavaScript(JNIEnv* env,
                                              jobject obj,
-                                             jstring script,
-                                             jobject message) {
+                                             jstring script) {
   RenderViewHost* host = web_contents_->GetRenderViewHost();
   DCHECK(host);
 
-  if (!message) {
-    // No callback requested.
-    host->ExecuteJavascriptInWebFrame(string16(),  // frame_xpath
-                                      ConvertJavaStringToUTF16(env, script));
-    return;
-  }
-
-  // Secure the message in a scoped object and give ownership of it to the
-  // callback.
-  ScopedJavaGlobalRef<jobject>* j_message = new ScopedJavaGlobalRef<jobject>();
-  j_message->Reset(env, message);
-  content::RenderViewHost::JavascriptResultCallback callback =
-      base::Bind(&JavaScriptResultCallback, base::Owned(j_message));
-
-  host->ExecuteJavascriptInWebFrameCallbackResult(
-      string16(),  // frame_xpath
-      ConvertJavaStringToUTF16(env, script),
-      callback);
+  string16 script_utf16 = ConvertJavaStringToUTF16(env, script);
+  return host->ExecuteJavascriptInWebFrameNotifyResult(string16(),
+                                                       script_utf16);
 }
 
 bool ContentViewCoreImpl::GetUseDesktopUserAgent(
@@ -1334,6 +1325,14 @@ jint Init(JNIEnv* env, jobject obj,
   return reinterpret_cast<jint>(view);
 }
 
+jint EvaluateJavaScript(JNIEnv* env, jobject obj, jstring script) {
+  ContentViewCoreImpl* view = static_cast<ContentViewCoreImpl*>(
+      ContentViewCore::GetNativeContentViewCore(env, obj));
+  DCHECK(view);
+
+  return view->EvaluateJavaScript(env, obj, script);
+}
+
 bool RegisterContentViewCore(JNIEnv* env) {
   if (!base::android::HasClass(env, kContentViewCoreClassPath)) {
     DLOG(ERROR) << "Unable to find class ContentViewCore!";

content/browser/android/content_view_core_impl.h

@@ -157,10 +157,7 @@ class ContentViewCoreImpl : public ContentViewCore,
   void ContinuePendingReload(JNIEnv* env, jobject obj);
   jboolean NeedsReload(JNIEnv* env, jobject obj);
   void ClearHistory(JNIEnv* env, jobject obj);
-  void EvaluateJavaScript(JNIEnv* env,
-                          jobject obj,
-                          jstring script,
-                          jobject message);
+  jint EvaluateJavaScript(JNIEnv* env, jobject obj, jstring script);
   int GetNativeImeAdapter(JNIEnv* env, jobject obj);
   void SetFocus(JNIEnv* env, jobject obj, jboolean focused);
   void ScrollFocusedEditableNodeIntoView(JNIEnv* env, jobject obj);

content/public/android/java/src/org/chromium/content/browser/ContentView.java

@@ -420,11 +420,17 @@ public class ContentView extends FrameLayout implements ContentViewCore.Internal
 
     /**
      * Injects the passed JavaScript code in the current page and evaluates it.
+     * Once evaluated, an asynchronous call to
+     * ContentViewClient.onJavaScriptEvaluationResult is made. Used in automation
+     * tests.
      *
+     * @return an id that is passed along in the asynchronous onJavaScriptEvaluationResult callback
      * @throws IllegalStateException If the ContentView has been destroyed.
+     *
+     * TODO(nileshagrawal): Remove this method from the public interface.
      */
-    public void evaluateJavaScript(String script) throws IllegalStateException {
-        mContentViewCore.evaluateJavaScript(script, null);
+    public int evaluateJavaScript(String script) throws IllegalStateException {
+        return mContentViewCore.evaluateJavaScript(script);
     }
 
     /**

content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java

@@ -92,6 +92,15 @@ public class ContentViewClient {
     public void onImeEvent() {
     }
 
+    /**
+     * A callback invoked after the JavaScript code passed to evaluateJavaScript
+     * has finished execution.
+     * Used in automation tests.
+     * @hide
+     */
+    public void onEvaluateJavaScriptResult(int id, String jsonResult) {
+    }
+
     // TODO (dtrainor): Should expose getScrollX/Y from ContentView or make
     // computeHorizontalScrollOffset()/computeVerticalScrollOffset() public.
     /**

content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java

@@ -18,7 +18,6 @@ import android.graphics.Rect;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.Handler;
-import android.os.Message;
 import android.os.ResultReceiver;
 import android.text.Editable;
 import android.util.Log;
@@ -1050,19 +1049,17 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
     }
 
     /**
-     * Injects the passed Javascript code in the current page and evaluates it.
-     * If a result is required, pass in a Message to be sent to its target.
-     * Used in automation tests.
+     * Injects the passed JavaScript code in the current page and evaluates it.
+     * Once evaluated, an asynchronous call to
+     * ContentViewClient.onJavaScriptEvaluationResult is made. Used in automation
+     * tests.
      *
-     * @param script The Javascript to execute.
-     * @param message The message to be fired off when a result is ready. The script's
-     *                result will be json encoded in the Message's {@link Message#obj} field.
-     *                If no result is required, pass null.
+     * @return an id that is passed along in the asynchronous onJavaScriptEvaluationResult callback
      * @throws IllegalStateException If the ContentView has been destroyed.
      */
-    public void evaluateJavaScript(String script, Message message) throws IllegalStateException {
+    public int evaluateJavaScript(String script) throws IllegalStateException {
         checkIsAlive();
-        nativeEvaluateJavaScript(mNativeContentViewCore, script, message);
+        return nativeEvaluateJavaScript(script);
     }
 
     /**
@@ -1993,9 +1990,8 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
 
     @SuppressWarnings("unused")
     @CalledByNative
-    private static void onEvaluateJavaScriptResult(String jsonResult, Message message) {
-        message.obj = jsonResult;
-        message.sendToTarget();
+    private void onEvaluateJavaScriptResult(int id, String jsonResult) {
+        getContentViewClient().onEvaluateJavaScriptResult(id, jsonResult);
     }
 
     @SuppressWarnings("unused")
@@ -2506,8 +2502,7 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
 
     private native void nativeClearHistory(int nativeContentViewCoreImpl);
 
-    private native void nativeEvaluateJavaScript(int nativeContentViewCoreImpl,
-            String script, Message message);
+    private native int nativeEvaluateJavaScript(String script);
 
     private native int nativeGetNativeImeAdapter(int nativeContentViewCoreImpl);
 

content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityInjector.java

@@ -132,7 +132,7 @@ public class AccessibilityInjector extends WebContentsObserverAndroid {
 
                 if (onDeviceScriptInjectionEnabled && js != null && mContentViewCore.isAlive()) {
                     addOrRemoveAccessibilityApisIfNecessary();
-                    mContentViewCore.evaluateJavaScript(js, null);
+                    mContentViewCore.evaluateJavaScript(js);
                     mInjectedScriptEnabled = true;
                     mScriptInjected = true;
                 }
@@ -183,7 +183,7 @@ public class AccessibilityInjector extends WebContentsObserverAndroid {
         if (mContentViewCore.isAlive()) {
             String js = String.format(TOGGLE_CHROME_VOX_JAVASCRIPT, Boolean.toString(
                     mInjectedScriptEnabled));
-            mContentViewCore.evaluateJavaScript(js, null);
+            mContentViewCore.evaluateJavaScript(js);
 
             if (!mInjectedScriptEnabled) {
                 // Stop any TTS/Vibration right now.

content/public/android/java/src/org/chromium/content/browser/accessibility/JellyBeanAccessibilityInjector.java

@@ -172,7 +172,7 @@ class JellyBeanAccessibilityInjector extends AccessibilityInjector {
             final int resultId = mResultIdCounter.getAndIncrement();
             final String js = String.format(JAVASCRIPT_ACTION_TEMPLATE, mInterfaceName, resultId,
                     code);
-            contentView.evaluateJavaScript(js, null);
+            contentView.evaluateJavaScript(js);
 
             return getResultAndClear(resultId);
         }
@@ -228,7 +228,7 @@ class JellyBeanAccessibilityInjector extends AccessibilityInjector {
          * request to a waiting (or potentially timed out) thread.
          *
          * @param id The result id of the request as a {@link String}.
-         * @param result The result of a request as a {@link String}.
+         * @param result The result o fa request as a {@link String}.
          */
         @JavascriptInterface
         @SuppressWarnings("unused")
@@ -249,4 +249,4 @@ class JellyBeanAccessibilityInjector extends AccessibilityInjector {
             }
         }
     }
-}
+}
\ No newline at end of file

content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestCallbackHelperContainer.java

@@ -5,8 +5,6 @@
 package org.chromium.content.browser.test.util;
 
 
-import android.os.Handler;
-import android.os.Message;
 import android.util.Log;
 
 import org.chromium.content.browser.ContentView;
@@ -83,7 +81,8 @@ public class TestCallbackHelperContainer {
     }
 
     public static class OnEvaluateJavaScriptResultHelper extends CallbackHelper {
-        private boolean mRequestSent;
+        private volatile Integer mRequestId;
+        private volatile Integer mId;
         private String mJsonResult;
 
         /**
@@ -92,24 +91,14 @@ public class TestCallbackHelperContainer {
          * @param code A JavaScript code to be evaluated.
          */
         public void evaluateJavaScript(ContentViewCore contentViewCore, String code) {
-            assert !mRequestSent;
-            Message message = Message.obtain(new Handler() {
-                @Override
-                public void handleMessage(Message msg) {
-                    String json = (String)msg.obj;
-                    notifyCalled(json);
-                }
-            });
-            contentViewCore.evaluateJavaScript(code, message);
-            mRequestSent = true;
-            mJsonResult = null;
+            setRequestId(contentViewCore.evaluateJavaScript(code));
         }
 
         /**
          * Returns true if the evaluation started by evaluateJavaScript() has completed.
          */
         public boolean hasValue() {
-            return mJsonResult != null;
+            return mId != null;
         }
 
         /**
@@ -120,8 +109,7 @@ public class TestCallbackHelperContainer {
         public String getJsonResultAndClear() {
             assert hasValue();
             String result = mJsonResult;
-            mRequestSent = false;
-            mJsonResult = null;
+            setRequestId(null);
             return result;
         }
 
@@ -153,8 +141,21 @@ public class TestCallbackHelperContainer {
             return hasValue();
         }
 
-        public void notifyCalled(String jsonResult) {
-            assert !hasValue();
+        private void setRequestId(Integer requestId) {
+            mRequestId = requestId;
+            mId = null;
+            mJsonResult = null;
+        }
+
+        public void notifyCalled(int id, String jsonResult) {
+            if (mRequestId == null) {
+                Log.w("TestCallbackHelperContainer",
+                        "Received JavaScript eval result when request id was not set");
+                return;
+            }
+            if (id != mRequestId.intValue()) return;
+            assert mId == null;
+            mId = Integer.valueOf(id);
             mJsonResult = jsonResult;
             notifyCalled();
         }

content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewClient.java

@@ -35,11 +35,16 @@ public class TestContentViewClient extends ContentViewClient {
     }
 
     /**
-     * ATTENTION!: When overriding the following method, be sure to call
-     * the corresponding method in the super class. Otherwise
+     * ATTENTION!: When overriding the following methods, be sure to call
+     * the corresponding methods in the super class. Otherwise
      * {@link CallbackHelper#waitForCallback()} methods will
      * stop working!
      */
+    @Override
+    public void onEvaluateJavaScriptResult(int id, String jsonResult) {
+        super.onEvaluateJavaScriptResult(id, jsonResult);
+        mOnEvaluateJavaScriptResultHelper.notifyCalled(id, jsonResult);
+    }
 
     @Override
     public void onStartContentIntent(Context context, String contentUrl) {