Summary: | Incorrect JNI handling of arrays causes browser crash | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Carson <dacarson> | ||||||||||
Component: | JavaScriptCore | Assignee: | David Carson <dacarson> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | ddkilzer, sam | ||||||||||
Priority: | P1 | Keywords: | InRadar | ||||||||||
Version: | 420+ | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.4 | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 12731 | ||||||||||||
Attachments: |
|
Description
David Carson
2007-02-06 12:24:57 PST
Created attachment 12981 [details]
Test case.
Attachment is a zip file containing HTML and Java class file (with source) that shows the crash.
Repro crash => P1. The fix is not as easy as I initially thought. I thought that if I could just separate recognition of Array parameters from generic object parameters, all would be good. There is more to it. Currently the JNI bindings don't know how to generate a Java Array class from a JS Array. Though, the reverse is true, the bindings can generate a JS Array from a Java Array. This is used when the return type is a Java Array. Created attachment 13016 [details]
Patch
This patch creates a new internal type, array_type.
This patch also handles the creation of Java arrays for the given Java parameter array type and fills it with the Javascript Array content.
Created attachment 13017 [details]
updated test case
This test case also tests all other Java array types, eg byte[], char[], etc. The previous test case only tested String[].
Comment on attachment 13016 [details]
Patch
r-
Comment on attachment 13016 [details]
Patch
Er, I meant, r=me
Is there a way to make a test case for this (even a manual test)?
(In reply to comment #8) > (From update of attachment 13016 [details] [edit]) > Er, I meant, r=me > > Is there a way to make a test case for this (even a manual test)? > There is a manual test case attached. I will move it to the WebCore\manual-tests and make it part of the patch. Created attachment 13083 [details]
Now includes manual test
Nothing has changed with respect to the code. I have added the test case to the manual-tests and included that as part of the patch, as mjs suggested.
Comment on attachment 13083 [details]
Now includes manual test
r=me
I am having trouble applying the patch as provided. I am getting an error "svn: 'WebCore/manual-tests/resources/ArrayParameterTestApplet.class' does not exist". (In reply to comment #12) > I am having trouble applying the patch as provided. I am getting an error > "svn: 'WebCore/manual-tests/resources/ArrayParameterTestApplet.class' does not > exist". Sam, did you use svn-apply to apply the patch? I just noticed a bug in svn-apply that won't apply the binary patch on Attachment 13083 [details] because it doesn't have a blank line after the end of the binary segment. Dave, yeah, I used svn-apply. Is there easy way for me (or you or anyone) to work around this in the meantime. Like say, just adding the blank line? Committed revision 19559. |