[9-dev] RfR: JDK-8132743: Move netscape.javascript package from jdk.plugin to new module
David DeHaven
david.dehaven at oracle.com
Sat Mar 5 18:54:43 UTC 2016
I've updated the webrev, hopefully one last time with feedback from Joe Darcy.
http://cr.openjdk.java.net/~ddehaven/8132743/webrev.2/
- Relocated package Javadoc to above the package declaration in package-info.java
- Reworded the Javadoc on the default JSObject ctor
A point was brought up that the default ctor could probably be package-private, but there's no time to investigate what the impact would be so I will file a follow-up issue to investigate doing that at a later date.
-DrD-
> On Mar 4, 2016, at 7:44 AM, Kevin Rushforth <kevin.rushforth at oracle.com> wrote:
>
> Looks good to me.
>
> -- Kevin
>
>
> David DeHaven wrote:
>> Updated webrev:
>>
>> http://cr.openjdk.java.net/~ddehaven/8132743/webrev.1/
>>
>>
>> Added jdk.jsobject to MAIN_MODULES
>> Made suggested Javadoc changes
>> Gave JSException a real serialVersionUID
>>
>> -DrD-
>>
>>
>>
>>
>>> On Mar 3, 2016, at 5:06 PM, David DeHaven <david.dehaven at oracle.com>
>>> wrote:
>>>
>>>
>>> Adding it to MAIN_MODULES, I now see it in bootmodules.jimage:
>>> /jdk.jsobject/jdk/internal/netscape/javascript/spi/JSObjectProvider.class
>>> /jdk.jsobject/netscape/javascript/JSException.class
>>> /jdk.jsobject/netscape/javascript/JSObject$ProviderLoader$1.class
>>> /jdk.jsobject/netscape/javascript/JSObject$ProviderLoader.class
>>> /jdk.jsobject/netscape/javascript/JSObject.class
>>>
>>> -DrD-
>>>
>>>
>>>
>>>> jdk9-dev is not the right mailing list. I bcc’ed it and add jigsaw-dev instead.
>>>>
>>>>
>>>>
>>>>
>>>>> On Mar 3, 2016, at 3:57 PM, Kevin Rushforth <kevin.rushforth at oracle.com>
>>>>> wrote:
>>>>>
>>>>> Looks OK to me. I did a quick test build and I can see the new package in the exploded JDK, but not in the images. Maybe I did something wrong?
>>>>>
>>>>>
>>>>>
>>>> Good catch.
>>>>
>>>> jdk.jsobject needs to be added in MAIN_MODULES list in make/Images.gmk
>>>>
>>>> Mandy
>>>>
>>>>
>>>>
>>>>> -- Kevin
>>>>>
>>>>>
>>>>> David DeHaven wrote:
>>>>>
>>>>>
>>>>>>>> JBS Issue:
>>>>>>>>
>>>>>>>>
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8132743
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Code review:
>>>>>>>>
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ddehaven/8132743/webrev.0/
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> Looks okay. There is no @since - I guess it’s okay because netscape.javascript has been shipped with plugin for long time.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> I can't track down when it was first included. It pre-dates anything I've looked at so far.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> package-info.java
>>>>>>> "when running in an {@link java.applet.Applet Applet}” - is this true when running with JavaFX webkit?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Yes, I believe so, assuming you have a JSObject representing the root window object. Maybe that should be reworded, I think just remove the "when running in an Applet" part.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> JSObject.java
>>>>>>> @throws JSException is missing in the methods
>>>>>>>
>>>>>>> Does it throw NPE if the parameter is null? Or JSException - that needs to be specified.
>>>>>>>
>>>>>>> Nit: it’d be good to wrap null with {@code null} in the javadoc.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Ok. I can fix that.
>>>>>>
>>>>>> -DrD-
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>
>>
>>
More information about the jigsaw-dev
mailing list