[9-dev] RfR: JDK-8132743: Move netscape.javascript package from jdk.plugin to new module
Kevin Rushforth
kevin.rushforth at oracle.com
Fri Mar 4 15:44:28 UTC 2016
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