[9-dev] RfR: JDK-8132743: Move netscape.javascript package from jdk.plugin to new module

[Kevin Rushforth]

Looks good.

-- Kevin


David DeHaven wrote:
> 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-
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>           
>>>>>>>
>>>>>>>               
>>>   
>>>
>>>       
>
>