Bug 57019 - JavaInstance should not use jvalue in its API
Summary: JavaInstance should not use jvalue in its API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 57022
Blocks: 55383
  Show dependency treegraph
 
Reported: 2011-03-24 08:47 PDT by Steve Block
Modified: 2011-03-29 16:47 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.07 KB, patch)
2011-03-29 04:12 PDT, Steve Block
no flags Details | Formatted Diff | Diff
Patch (6.36 KB, patch)
2011-03-29 15: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-24 08:47:39 PDT
The JavaInstance API should represent a generic Java object and should be independent of JNI, to give embedders flexibility in how a Java object is provided. This means that the API should use a new JavaValue type to represent a Java value, rather than the JNI jvalue type.
Comment 1 Steve Block 2011-03-24 08:48:30 PDT
See bug 55383.
Comment 2 Steve Block 2011-03-29 01:19:03 PDT
This bug tracks fixing only the V8 version of JavaInstance.

The JSC version can not be fixed until Bug 57023 is fixed.
Comment 3 Steve Block 2011-03-29 04:12:53 PDT
Created attachment 87296 [details]
Patch
Comment 4 Andrei Popescu 2011-03-29 11:31:12 PDT
LGTM

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

> Source/WebCore/bridge/jni/v8/JavaInstanceV8.cpp:76
> +    jvalue* jvalueArgs = new jvalue[numParams];

Is there a scoped_pointer for arrays?

> Source/WebCore/bridge/jni/v8/JavaInstanceV8.cpp:78
> +        jvalueArgs[i] = javaValueToJvalue(args[i]);

Maybe add a comment to say that args is guaranteed to have at least numParams elements.
Comment 5 Steve Block 2011-03-29 15:03:56 PDT
> Is there a scoped_pointer for arrays?
Yes - OwnArrayPtr - fixed.

> Maybe add a comment to say that args is guaranteed to have at least numParams elements.
I've added a comment in the header. Note that this is not a new requirement, as previously the array was passed straight through to callJNIMethod().
Comment 6 Steve Block 2011-03-29 15:06:50 PDT
Created attachment 87413 [details]
Patch
Comment 7 Jeremy Orlow 2011-03-29 15:10:26 PDT
Comment on attachment 87413 [details]
Patch

r+ based on Andrei's review
Comment 8 WebKit Commit Bot 2011-03-29 16:47:03 PDT
Comment on attachment 87413 [details]
Patch

Clearing flags on attachment: 87413

Committed r82361: <http://trac.webkit.org/changeset/82361>
Comment 9 WebKit Commit Bot 2011-03-29 16:47:07 PDT
All reviewed patches have been landed.  Closing bug.