Bug 57230 – Add to Chromium WebKit API methods for injecting Java objects into JavaScript

• Status: RESOLVED
• Resolution: INVALID
• Component: WebCore Misc.

[Steve Block]

Add to Chromium WebKit API methods for injecting Java objects into JavaScript

[Steve Block]

Since Bug 55383, V8 performs all interaction with injected Java objects through the JavaInstance, JavaClass, JavaMethod and JavaField interfaces. These interfaces are independent of JNI.

Currently, the only implementations of these interfaces are JavaInstanceJobject etc, which are wrappers around a JNI jobject. These implementations use JNI to query the underlying Java object for its methods and properties and to access them.

We'd like to be able to implement injecting Java objects in the Android browser in a multi-process environment. Access to the underlying Java object must be in the remote process. This means that we need implementations of JavaInstance etc which simply proxy requests to the embedder. The browser can then take care of routing these requests to the Java object in the correct process.

We share code paths with Chromium in a number of places on Android and this would be another such place.

Adding these proxy implementations is tracked in Bug 57859.

[Steve Block]

Created attachment 88407 [details]
Patch

[Steve Block]

Comment on attachment 88407 [details]
Patch

Not for review. Provided as context for Bug 57859 only. WebCore changes will be made in that bug.

[Steve Block]

Created attachment 103371 [details]
Patch

[Steve Block]

Comment on attachment 103371 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103371&action=review

> Source/WebKit/chromium/public/WebJavaBridge.h:49
> +    WebJavaBridge();

Just realised that this will have to be guarded with !WEBKIT_IMPLEMENTATION. Is this acceptable style, or should I just leave out the private constructor?

> Source/WebKit/chromium/public/WebJavaValue.h:31
> +namespace JSC { namespace Bindings { class JavaValue; } }

bulach: guard the JSC namespace forward declaration with WEBKIT_IMPLEMENTATION? not sure what the style says though..
Looking at other code, it seems that some places add the guards, others don't. I can't see any direct advice in the style guide. It's probably best to add the guard.

> Source/WebKit/chromium/public/WebJavaValue.h:40
> +class WebJavaValue {

bulach: is WebJavaValue a concrete class? should it be part of the patch?
Yes, it is. Am happy to include it as part of this patch if you like.

[Steve Block]

Created attachment 103455 [details]
Patch

[Steve Block]

Uploaded a patch to Bug 65928 for context.

[Marcus Bulach]

Comment on attachment 103455 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103455&action=review

(not a reviewer, take with a grain of salt :)
it overall looks good, just one comment on the various getters / setters.

> Source/WebKit/chromium/src/WebJavaValue.cpp:234
> +    m_private->m_stringValue = value;

I'm not entirely sure what are the expected semantics, but maybe assert (or set?) the type?

[Steve Block]

Comment on attachment 103455 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103455&action=review

>> Source/WebKit/chromium/src/WebJavaValue.cpp:234
>> +    m_private->m_stringValue = value;
> 
> I'm not entirely sure what are the expected semantics, but maybe assert (or set?) the type?

Good idea. I could remove setType(), set the type from setFooValue() and add an assert in fooValue().

[Steve Block]

Comment on attachment 103455 [details]
Patch

Investigating an alternative way of doing this

[Steve Block]

We're no longer pursuing this approach. Chromium will use it's existing NPAPI APIs to implement the Java Bridge. See discussion in Bug 57859.