Bug 57230 - Add to Chromium WebKit API methods for injecting Java objects into JavaScript
Summary: Add to Chromium WebKit API methods for injecting Java objects into JavaScript
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Steve Block
URL:
Keywords:
Depends on: 55383 57859
Blocks: 65928
  Show dependency treegraph
 
Reported: 2011-03-28 07:11 PDT by Steve Block
Modified: 2012-01-08 10:19 PST (History)
2 users (show)

See Also:


Attachments
Patch (60.80 KB, patch)
2011-04-06 06:39 PDT, Steve Block
no flags Details | Formatted Diff | Diff
Patch (12.65 KB, patch)
2011-08-09 10:51 PDT, Steve Block
no flags Details | Formatted Diff | Diff
Patch (19.93 KB, patch)
2011-08-10 02:06 PDT, Steve Block
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2011-03-28 07:11:51 PDT
Add to Chromium WebKit API methods for injecting Java objects into JavaScript
Comment 1 Steve Block 2011-04-06 06:28:46 PDT
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.
Comment 2 Steve Block 2011-04-06 06:39:37 PDT
Created attachment 88407 [details]
Patch
Comment 3 Steve Block 2011-04-06 06:40:53 PDT
Comment on attachment 88407 [details]
Patch

Not for review. Provided as context for Bug 57859 only. WebCore changes will be made in that bug.
Comment 4 Steve Block 2011-08-09 10:51:31 PDT
Created attachment 103371 [details]
Patch
Comment 5 Steve Block 2011-08-09 14:59:44 PDT
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.
Comment 6 Steve Block 2011-08-10 02:06:09 PDT
Created attachment 103455 [details]
Patch
Comment 7 Steve Block 2011-08-10 02:24:36 PDT
Uploaded a patch to Bug 65928 for context.
Comment 8 Marcus Bulach 2011-08-10 10:14:10 PDT
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?
Comment 9 Steve Block 2011-08-10 11:03:23 PDT
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().
Comment 10 Steve Block 2011-09-15 07:45:31 PDT
Comment on attachment 103455 [details]
Patch

Investigating an alternative way of doing this
Comment 11 Steve Block 2012-01-08 10:19:28 PST
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.