[rfc][icedtea-web] Remove wrongly 'undummied' JSObject->Java array code from MethodOverloadResolver

[Adam Domurad]

On 05/01/2013 09:38 AM, Jiri Vanek wrote:
> On 05/01/2013 03:06 PM, Adam Domurad wrote:
>> On 05/01/2013 08:19 AM, Jiri Vanek wrote:
>>> On 04/29/2013 09:15 PM, Adam Domurad wrote:
>>>> So part of what motivated me to rewrite the old 
>>>> MethodOverloadResolver was the fact that I noticed it had a dummy 
>>>> JSObject defined at the bottom of the code, and it was mistakenly 
>>>> using that. It puzzled me why this wasn't observable. Well, it 
>>>> seems it is because the only code path that relied on JSObject 
>>>> itself was incorrect.
>>>>
>>>> Thanks to Jana for the thorough tests that caught this!
>>>> It looks like we do not support conversion of JSObject to a Java 
>>>> array in method signatures. This will bring the code back to its 
>>>> old functionality.
>>>>
>>>> There is a want for the real fix, but I think we should do a fix in 
>>>> a separate step.
>>>>
>>>
>>> Looks mostly corect. Will the tests need some fixes?
>>
>> No -- unfortunately the JSObject related tests are commented don't 
>> have the necessary privileges to create a JSObject.
>
> Not even in jana's tests?

Oi, I'm a dummy. Current tests are correct, but see below about a few 
more we could have.

I just realized we're not missing this functionality. The C++ function 
'createJavaObjectFromVariant' automatically converts JS arrays to Java 
arrays before passing them. I have attached some additional unit tests. 
(ChangeLog in patch)

However unfortunately I see other fundamental brokenness. It seems we'd 
need to ask for thie conversion from the Java-side to be sure it is correct.

For example:

JSObject someField; from Java side
applet.someField = [1,1] from Javascript side,

You'd expect it to be able to do this (it is a JSObject, after all), but 
actually it is not possible. This is because it is converted to a 
String[] array before-hand.

As well:
Object[] someField; from Java side
applet.someField = [{}, 1]
Is not possible. You'd expected an array of [JSObject, Integer], but the 
whole thing is a JSObject.

Ignorance is bliss ... These cases do not seem to be handled by the 
reproducers (but I could be mistaken), some KnownToFail reproducers 
cases might be nice.

Still happy hacking,
-Adam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: MethodOverloadResolver-additional-tests.patch
Type: text/x-patch
Size: 7619 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130501/3a79070e/MethodOverloadResolver-additional-tests.patch