[android_webview] Add a generic callback JNI bridge.

android_webview/native/callback_jni_bridge.cc

+// Copyright 2013 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 "android_webview/native/callback_jni_bridge.h"
+
+#include "base/android/scoped_java_ref.h"
+#include "base/bind.h"
+#include "jni/CallbackJNIBridge_jni.h"
+
+using base::Callback;
+using base::android::AttachCurrentThread;
+using base::android::ScopedJavaLocalRef;
+using base::android::ScopedJavaGlobalRef;
+
+namespace android_webview {
+
+class JavaValueCallbackInt {
+ public:
+  JavaValueCallbackInt(JNIEnv* env,
+                       const ScopedJavaLocalRef<jobject>& jvalue_callback) {
+    jvalue_callback_.Reset(env, jvalue_callback.obj());
+  }
+
+  void onReceiveValue(int value) {
+    DCHECK(!jvalue_callback_.is_null());

[boliu, 2013/02/21 23:11:12]

Would be nice if it can automatically check threading error too. I assume most
callbacks need to happen on the thread it was originally passed in...but maybe
I'm asking for too much :)

[mkosiba (inactive), 2013/02/22 18:54:31]

nope, the more auto-checking we have the better. I can add the checks and if we
need thread-hopping callbacks there could be a more explicitly-named method to
create one.

+
+    if (!jvalue_callback_.is_null()) {
+      JNIEnv* env = AttachCurrentThread();
+      Java_CallbackJNIBridge_invokeValueCallbackInt(
+          env, jvalue_callback_.obj(), value);

[boliu, 2013/02/22 23:02:52]

Also just realized, generic types must be an object, so does this actually work?
ie does autobox also work from jni code?

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

no, you'd have to new up an Integer object if you wanted to call a
ValueCallback<Integer>

+      jvalue_callback_.Reset();
+    }
+  }
+
+ private:
+  ScopedJavaGlobalRef<jobject> jvalue_callback_;
+};
+
+Callback<void(int)> CallbackJNIBridge::FromJavaValueCallbackInt(

[boliu, 2013/02/21 23:11:12]

There is no way to use templates to interpret the types needed since it's just
the basic native types + jobject and that's it.

But how about this (if it's at all possible?): pass in a templated thing
(class/function?) that converts the required native arguments to the single
jobject (native types are boxed into objects in java anyway), so usage would be
something like:

Java:
nativeDoSomething(new ValueCallback<String[]>{...});

Native:
void object::doSomethingInCpp(jobject callback) {
  Callback<std::vector<std::string> > callback =
      CallbackJNIBridge::FromJavaValueCallback(
          callback, &base::android::ToJavaArrayOfStrings)
}


Basically template the return type and the second conversion argument.

Slightly mitigate the type unsafety too?...not really...

[mkosiba (inactive), 2013/02/22 18:54:31]

I talked to Marcus and he'd prefer us to make the types explicit (so
ValueCallbackStringArray in your example) exactly because the JNI boundary is
unsafe.

[boliu, 2013/02/22 23:02:52]

I'm gonna push back on this a bit at least for non-general complex types,
example being AwQuotaManagerBridge.Origin in my other patch.

If we go down the explicit route, we'll have something like
Callback<void(std::vector, std::vector, std::vector)
FromJavaValueCallbackOrigin(JNIEnv, jobject jcallback)

And the corresponding JavaValueCallbackOrigin class that needs to convert the 3
native vectors into the correct java Origin object. This code needs a lot of
knowledge from AwQuotaManagerBridge and will probably have 0 reuse value
elsewhere, so it should live in AwQuotaMangerBridge. But at that point, it seems
better to just do it like the way in my patch instead of doing all this
acrobatics just to consistent with everywhere else.

Another point is that as long as we are calling callback.onValueReceived from
native side, we are going to lose type safety somewhere. We are counting on the
caller to call the right function here, which is not that much better than
having the caller write the native->java conversion. Of course the conversion
for the basic types can still be provided here.

+    JNIEnv* env,
+    ScopedJavaLocalRef<jobject> jvalue_callback) {
+  JavaValueCallbackInt* value_callback =
+      new JavaValueCallbackInt(env, jvalue_callback);
+  return base::Bind(&JavaValueCallbackInt::onReceiveValue,
+                    base::Owned(value_callback));
+}
+
+ScopedJavaLocalRef<jobject> CallbackJNIBridge::ToJavaValueCallbackInt(
+    JNIEnv* env,
+    const Callback<void(int)>& callback) {
+  Callback<void(int)>* native_callback = new Callback<void(int)>(callback);

[boliu, 2013/02/21 23:11:12]

new vs copy by value, the native object only holds a GlobalRef, so copy by value
should be cheap too. But it's a scoped ref...ok, maybe not.

[mkosiba (inactive), 2013/02/22 18:54:31]

The Callback has an internal scoped_refptr<BindState> that has all of the actual
state. The reason I use new here is so that I can pass ownership of the callback
(and the BindState) to the Java side.

+  ScopedJavaLocalRef<jobject> jvalue_callback =
+      Java_CallbackJNIBridge_createValueCallbackInt(
+          env, reinterpret_cast<int>(native_callback));
+
+  return jvalue_callback;
+}
+
+// static methods -------------------------------------------------------------
+
+static void OnReceiveValueInt(JNIEnv* env,
+                              jclass clazz,
+                              jint callbackInt,
+                              jint value) {
+  Callback<void(int)>* native_callback =
+      reinterpret_cast<Callback<void(int)>*>(callbackInt);
+  native_callback->Run(value);
+}
+
+static void DestroyInt(JNIEnv* env, jclass clazz, jint callbackInt) {
+  Callback<void(int)>* native_callback =
+      reinterpret_cast<Callback<void(int)>*>(callbackInt);
+  delete native_callback;
+}
+
+bool RegisterCallbackJNIBridge(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+} // namespace android_webview