[android_webview] Add a generic callback JNI bridge.

android_webview/java/src/org/chromium/android_webview/CallbackJNIBridge.java

Index: android_webview/java/src/org/chromium/android_webview/CallbackJNIBridge.java
diff --git a/android_webview/java/src/org/chromium/android_webview/CallbackJNIBridge.java b/android_webview/java/src/org/chromium/android_webview/CallbackJNIBridge.java
new file mode 100644
index 0000000000000000000000000000000000000000..0b18383a949a65d4d99669d9e4a02079fdb1a72f
--- /dev/null
+++ b/android_webview/java/src/org/chromium/android_webview/CallbackJNIBridge.java
@@ -0,0 +1,298 @@
+// Copyright (c) 2012 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.
+
+package org.chromium.android_webview;
+
+import android.util.Log;
+import android.webkit.ValueCallback;
+
+import org.chromium.base.CalledByNative;
+import org.chromium.base.CalledByNativeUnchecked;
+import org.chromium.base.NativeCall;
+import org.chromium.base.JNINamespace;
+import org.chromium.content.common.CleanupReference;
+
+import java.util.Set;
+import java.util.HashSet;
+
+/**
+ * This class facilitates delegating callback method invocations across the JNI boundary.
+ *
+ * The basic motivation for this code is to encapsulate C++ <-> Java callback invocations. There
+ * are two main use cases described below. The implementation makes two assumptions about the
+ * callbacks:
+ *  1) callbacks are "one-shot". Calling a ValueCallback.onReceiveValue or base::Callback.Run
+ *     method more than once on a given instance is not allowed
+ *  2) Java -> C++ callbacks are created and called on the same thread.
+ *
+ * Converting a Java ValueCallback<Integer> to a C++ base::Callback<void(int)>:
+ *
+ * Java:
+ *   doSomethingInCpp(ValueCallback<Integer> vc) {
+ *       nativeDoSomethingInCpp(CallbackJNIBridge.wrapCallbackInt(vc));
+ *   }
+ *
+ *   private native void nativeDoSomethingInCpp(JavaCallbackInt callback);
+ *
+ *   C++:
+ *   void object::DoSomethingInCpp(jobject jvalue_callback) {
+ *     base::Callback<void(int)> callback = CallbackJNIBridge::FromJavaCallbackInt(jvalue_callback);
+ *     callback.Run(1); // vc.onReceiveValue(1); gets called Java-side
+ *   }
+ *
+ * Converting a Java ValueCallback<T> to a C++ base::Callback<void(T)>:
+ *
+ * C++:
+ *   void CustomObject* customObjectConverter(ScopedJavaLocalRef<jobject> jobject);
+ *
+ *   void object::DoSomethingInCpp(jobject jvalue_callback) {
+ *     base::Callback<void(CustomObject*)> callback =
+ *       CallbackJNIBridge::FromJavaCallback(jvalue_callback, base::Bind(&customObjectConverter);
+ *     callback.Run(1); // vc.onReceiveValue(1); gets called Java-side
+ *   }
+ *
+ * Converting a C++ base::Callback<void(int)> to a Java ValueCallback<Integer>:
+ *
+ * C++:
+ *   android::ScopedLocalJavaRef<jobject> jcallback =
+ *     JavaIntCallback(cpp_object, &Object::onIntValue);
+ *   Java_className_doSomething(jcallback.obj());
+ *
+ * in Java:
+ *   private void doSomething(ValueCallback<Integer> vc) {
+ *        int value = 3;
+ *        vc.onReceiveValue(value); // calls cpp_object->onIntValue(3);
+ *   }
+ *
+ */
+@JNINamespace("android_webview")
+public class CallbackJNIBridge {
+    private static final String TAG = "CallbackJNIBridge";
+
+    /**
+     * Interface used for native -> Java callback dispatching.
+     */
+    public interface JavaCallback {
+        @CalledByNative("JavaCallback")
+        void invoke(Object value);
+    }
+
+    /**
+     * Convert a ValueCallback to a form that can be easily passed to the native side.
+     */
+    @SuppressWarnings("unchecked")

[mkosiba (inactive), 2013/02/25 23:43:44]

I think this guy will definitely stay - ie. I don't want to pass ValueCallbacks
directly to native. This wrap method gives us a nice point to do potential
future customizations (like memory ownership changes or the exception handling
that's here)

+    public static <T> JavaCallback wrapCallback(final ValueCallback<T> vc) {
+        return new JavaCallback() {
+            @Override
+            public void invoke(Object value) {
+                try {
+                    ((ValueCallback) vc).onReceiveValue(value);
+                } catch (RuntimeException e) {
+                    Log.e(TAG, "An error occurred while invoking a callback.", e);
+                    throw e;
+                }
+            }
+        };
+    }
+
+    public interface JavaCallbackInt {
+        @CalledByNative("JavaCallbackInt")
+        void invoke(int value);
+    }
+
+    public static JavaCallbackInt wrapCallbackInt(final ValueCallback<Integer> vc) {

[mkosiba (inactive), 2013/02/25 23:43:44]

It would be useful if the same method call could pick out the 'specialized'
method for some set of types, but fall back on the default object path (like the
C++ counterpart does). Unfortunately that's not possible. Why is this a problem?
I wrote wrapCallback() instead of wrapCallbackInt in my unittest code and had a
bad time figuring out what the JNI error was trying to tell me.

That seems like a strong argument against providing possibly maybe faster[1]
specialized versions of the wrapCallback method. The initial reason I added
these was for more safety[2], but now that I've got all the code out there I
don't think we gain that much - there are at least two places where you need to
call the correctly named method variant and even though there is a way to
mitigate some of this pain (see the comment about adding a DCHECK in the .cc
file) I'm not sure if keeping the specializations around makes any sense.

[1] using a ValueCallback<Integer> through the JavaCallback interface (instead
of JavaCallbackInt) would require two JNI calls - one for the callback and one
for creating the Integer.

[2] it *looks* safer since the JNI methods take in a jint, jlong and jboolean
but since you still pass the callback itself as a jobject there isn't anything
stopping you from calling the wrong C++ method (call a FromJavaCallback<int> on
a ValueCallback<String>)

+        return new JavaCallbackInt() {
+            @Override
+            public void invoke(int value) {
+                try {
+                    vc.onReceiveValue(value);
+                } catch (RuntimeException e) {
+                    Log.e(TAG, "An error occurred while invoking a callback.", e);
+                    throw e;
+                }
+            }
+        };
+    }
+
+    public interface JavaCallbackLong {
+        @CalledByNative("JavaCallbackLong")
+        void invoke(long value);
+    }
+
+    public static JavaCallbackLong wrapCallbackLong(final ValueCallback<Long> vc) {
+        return new JavaCallbackLong() {
+            @Override
+            public void invoke(long value) {
+                try {
+                    vc.onReceiveValue(value);
+                } catch (RuntimeException e) {
+                    Log.e(TAG, "An error occurred while invoking a callback.", e);
+                    throw e;
+                }
+            }
+        };
+    }
+
+    public interface JavaCallbackBoolean {
+        @CalledByNative("JavaCallbackBoolean")
+        void invoke(boolean value);
+    }
+
+    public static JavaCallbackBoolean wrapCallbackBoolean(final ValueCallback<Boolean> vc) {
+        return new JavaCallbackBoolean() {
+            @Override
+            public void invoke(boolean value) {
+                try {
+                    vc.onReceiveValue(value);
+                } catch (RuntimeException e) {
+                    Log.e(TAG, "An error occurred while invoking a callback.", e);
+                    throw e;
+                }
+            }
+        };
+    }
+
+    /**
+     * Base class for Java -> C++ callbacks.
+     */
+    private abstract static class NativeCallbackBase<T> implements ValueCallback<T> {

[mkosiba (inactive), 2013/02/25 23:43:44]

these guys don't seem to have the problem the JavaCallbackXXX interface had.
Would it make sense to go down to an Object-only version for the sake of
symmetry? (and some test simplification)

+        private CleanupReference mCleanupReference;
+        private int mNativeCallback;
+
+        public NativeCallbackBase(int nativeCallback) {
+            assert nativeCallback != 0;
+            mNativeCallback = nativeCallback;
+            mCleanupReference = new CleanupReference(this, new Runnable() {
+                @Override
+                public void run() {
+                    if (mNativeCallback != 0) {
+                        clear(mNativeCallback);
+                        mNativeCallback = 0;
+                    }
+                }
+            });
+        }
+
+        @Override
+        public void onReceiveValue(T value) {
+            assert mNativeCallback != 0;
+            if (mNativeCallback != 0) {
+                invoke(mNativeCallback, value);
+                clear(mNativeCallback);
+                mNativeCallback = 0;
+            }
+        }
+
+        abstract protected void clear(int nativeCallback);
+        abstract protected void invoke(int nativeCallback, T value);
+    }
+
+    private static class NativeCallback extends NativeCallbackBase {
+        public NativeCallback(int nativeCallback) {
+            super(nativeCallback);
+        }
+
+        @Override
+        protected void clear(int nativeCallback) {
+            nativeDestroyObj(nativeCallback);
+        }
+
+        @Override
+        protected void invoke(int nativeCallback, Object value) {
+            nativeInvokeObj(nativeCallback, value);
+        }
+
+        @CalledByNative("NativeCallback")
+        public static NativeCallback create(int callbackPtr) {
+            return new NativeCallback(callbackPtr);
+        }
+
+        @NativeCall("NativeCallback")
+        private static native void nativeDestroyObj(int callbackPtr);
+
+        @NativeCall("NativeCallback")
+        private static native void nativeInvokeObj(int callbackPtr, Object value);
+    }
+
+    private static class NativeCallbackInt extends NativeCallbackBase<Integer> {
+        public NativeCallbackInt(int nativeCallback) {
+            super(nativeCallback);
+        }
+
+        @Override
+        protected void clear(int nativeCallback) {
+            nativeDestroyInt(nativeCallback);
+        }
+
+        @Override
+        protected void invoke(int nativeCallback, Integer value) {
+            nativeInvokeInt(nativeCallback, value);
+        }
+
+        @CalledByNative("NativeCallbackInt")
+        public static NativeCallbackInt create(int callbackPtr) {
+            return new NativeCallbackInt(callbackPtr);
+        }
+
+        @NativeCall("NativeCallbackInt")
+        private static native void nativeDestroyInt(int callbackPtr);
+
+        @NativeCall("NativeCallbackInt")
+        private static native void nativeInvokeInt(int callbackPtr, int value);
+    }
+
+    private static class NativeCallbackLong extends NativeCallbackBase<Long> {
+        public NativeCallbackLong(int nativeCallback) {
+            super(nativeCallback);
+        }
+
+        @Override
+        protected void clear(int nativeCallback) {
+            nativeDestroyLong(nativeCallback);
+        }
+
+        @Override
+        protected void invoke(int nativeCallback, Long value) {
+            nativeInvokeLong(nativeCallback, value);
+        }
+
+        @CalledByNative("NativeCallbackLong")
+        public static NativeCallbackLong create(int callbackPtr) {
+            return new NativeCallbackLong(callbackPtr);
+        }
+
+        @NativeCall("NativeCallbackLong")
+        private static native void nativeDestroyLong(int callbackPtr);
+
+        @NativeCall("NativeCallbackLong")
+        private static native void nativeInvokeLong(int callbackPtr, long value);
+    }
+
+    private static class NativeCallbackBoolean extends NativeCallbackBase<Boolean> {
+        public NativeCallbackBoolean(int nativeCallback) {
+            super(nativeCallback);
+        }
+
+        @Override
+        protected void clear(int nativeCallback) {
+            nativeDestroyBoolean(nativeCallback);
+        }
+
+        @Override
+        protected void invoke(int nativeCallback, Boolean value) {
+            nativeInvokeBoolean(nativeCallback, value);
+        }
+
+        @CalledByNative("NativeCallbackBoolean")
+        public static NativeCallbackBoolean create(int callbackPtr) {
+            return new NativeCallbackBoolean(callbackPtr);
+        }
+
+        @NativeCall("NativeCallbackBoolean")
+        private static native void nativeDestroyBoolean(int callbackPtr);
+
+        @NativeCall("NativeCallbackBoolean")
+        private static native void nativeInvokeBoolean(int callbackPtr, boolean value);
+    }
+}