[rfc][icedtea-web] Rewrite of MethodOverloadResolver

[Jiri Vanek]

On 04/19/2013 11:43 AM, Jiri Vanek wrote:
> On 01/15/2013 05:14 PM, Adam Domurad wrote:
>> On 01/15/2013 08:46 AM, Jiri Vanek wrote:
>>> On 01/09/2013 09:05 PM, Adam Domurad wrote:
>>>> On 12/14/2012 03:43 PM, Adam Domurad wrote:
>>>>> Thought I'd do a simple patch to turn the test code embedded in MethodOverloadResolver into a a
>>>>> proper unit test ... unfortunately, my small look into this class lead me to find many subtle bugs
>>>>> -- and coincidentally decide to do it properly. Lots of corner cases covered, and it could
>>>>> probably use unit tests even still, but its a major improvement and I'm a bit tired of looking at
>>>>> it so I'm posting it.
>>>>>
>>>>> Some broken things I uncovered included improper overloading order, method/argument situations
>>>>> that would error-out if they ever occurred (but were valid), and the fact that the JSObject used
>>>>> throughout the class was actually a dummy value used at the bottom of the class. The code was also
>>>>> made a lot cleaner in the process, in the interest of 'doing it right' while fixing it.
>>>>> ChangeLog:
>>>>> 2012-12-14 Adam Domurad <adomurad at redhat.com>
>>>>>
>>>>> Rewrite of MethodOverloadResolver with detailed unittests.
>>>>> * plugin/icedteanp/java/sun/applet/MethodOverloadResolver.java:
>>>>> Rewritten to reduce duplicated code, fix very subtle bugs in
>>>>> never-tested codepaths, obey spec properly. Introduced new helper types
>>>>> where Object[] arrays with special-meaning positions were passed
>>>>> around.
>>>>> * plugin/icedteanp/java/sun/applet/PluginAppletSecurityContext.java:
>>>>> Updated to work with newly introduced types / refactored overload
>>>>> resolver.
>>>>> * tests/netx/unit/sun/applet/MethodOverloadResolverTest.java: In-depth
>>>>> unit tests of hairy details of method overloading in JS<->Java.
>>>>>
>>>>>
>>>> Ping?
>>>> -Adam
>>>
>>> Although I really like this patch as it is making MethodOverloadResolver much more readable and better, there is no significant *visible* improvement (according to Jana's tests). Also I was unable to review it properly - as it was de facto replacement.
>>>
>>> Unless you+jana will find some improvement in (at least) liveconect test or similar, then I must revoke this patch.
>>>
>>> If no "proof" will be found by you and jana, then I would like to get this into head by small (reviewable) steps accompanied by unit/reproducer tests.
>>>
>>> Sorry for troubles
>>> J.
>>
>> Just to be clear - I believe many fixed cases are visible in the unit tests. However these are difficult to determine because they were written against the new MethodOverloadResolver. Especially like I pointed out, JSObject was resolving to the completely wrong class. There were certain cases where exceptions would be thrown merely if a certain overload was present. If this is the issue, we can get more of these cases into the reproducers, but I do think there were a number of them. I'll work with Jana on this one.
>>
>> -Adam
> This get lost athe end?
I have Speaked with jana -  Not sure if then there was some private conversation, but AFAIK, this is ok

acked :)