<Swing Dev> [9] Review request for 8167176 Exported elements referring to inaccessible types in java.desktop

[Phil Race]

On 10/18/2016 03:54 AM, Alexandr Scherbatiy wrote:
> On 10/18/2016 12:42 AM, Philip Race wrote:
>> Hi,
>>
>> First note that any of the alternatives here require an approved CCC 
>> before pushing.
>>
>> As I wrote in the bug the fields in JRootPane are not used. Making it 
>> a public supertype
>> is no more useful than just deleting it. This like hiding the peers 
>> which was a much
>> bigger change so please just delete these fields.
>>
>> I don't understand the reasoning behind changing "bumps" to be of the 
>> super-class type Icon
>> and introducing a new variable that is what is then used.
>>
>> Since "MetalBumps" is package private we should be able to safely 
>> assume no one has
>> been using these fields without the aid of reflection and I doubt 
>> anyone found a need.
>> And that won't work in JDK 9 with upcoming jigsaw changes
>> If you wanted people to usefully access these then you'd need to make 
>> MetalBumps public.
>> I *am not* suggesting that .. just pointing out that the alternatives 
>> I see are either
>> make it entirely public, or entirely hide is as being a mistake.
>> So make the fields all private and do not provide the non-useful 
>> public replacement.
>> And I mean remove today. In JDK 9. This is not as radical as it seems 
>> since left as
>> it is they are useless to anyone as you cannot use them.
>>
>> Also createDirectoryComboBoxRenderer(..) can be made private too.
>   All these fields and methods can be used with the public super-class 
> type in a user code.
>   For example the code below successfully compiled with JDK 8 and 9:
>   -------------
> public class Test {
>
>     static class CustomToolBarBorder extends MetalBorders.ToolBarBorder {
>
>         public void doSomething(Graphics g) {
>             Icon metalBumps = bumps;
>             metalBumps.paintIcon(new JLabel(), g, 0, 0);
>         }
>     }
>
>     static class CustomMetalFileChooserUI extends MetalFileChooserUI {
>
>         public CustomMetalFileChooserUI(JFileChooser filechooser) {
>             super(filechooser);
>         }
>
>         public void doSomething() {
>             DefaultListCellRenderer listCellRenderer = 
> createDirectoryComboBoxRenderer(getFileChooser());
>             // do something with the listCellRenderer
>         }
>     }

See if you can find any 3rd party code at all that uses any of these ...

>
>     static class CustomJRootPane extends JRootPane {
>
>         public void doSomething() {
>             AbstractAction pressAction = defaultPressAction != null
>                     ? defaultPressAction
>                     : new CustomAction();
>             AbstractAction releaseAction = defaultReleaseAction != null
>                     ? defaultReleaseAction
>                     : new CustomAction();
>         }
>     }
> }

This example is weird since defaultPressAction and defaultReleaseAction 
are ALWAYS null
unless the application itself set them to something.
It is completely acceptable to me to delete these fields.


>   -------------
>
>   I just need to be sure that the incompatible change introduced by 
> removing these fields or making them private is acceptable.

>
>>
>> Finally, when these are resolved you should be able to re-enable lint !
>> http://hg.openjdk.java.net/jdk9/dev/rev/81435a812f59
>   I prepared the hint enabling fix but I believe it should be reviewed 
> on the build-dev and jigsaw-dev aliases after the current fix is pushed.

I don't think this is at all necessary. The warning has nothing to do 
with jigsaw and isn't a build change per se.
It should be included with this webrev+subsequent push, under this bug ID.


-phil.

>
>   Thanks,
>   Alexandr.
>>
>> -phil.
>>
>> On 10/17/16, 4:22 AM, Alexandr Scherbatiy wrote:
>>>
>>>   Hello,
>>>
>>>   Could you review the updated fix:
>>>     http://cr.openjdk.java.net/~alexsch/8167176/webrev.01
>>>
>>>   MetalBorders.bumps and MetalScrollBarUI.bumps fields are nor 
>>> marked for removal any more.
>>>
>>>   Thanks,
>>>   Alexandr.
>>>
>>> On 10/14/2016 3:23 PM, Sergey Bylokhov wrote:
>>>> Is it necessary to remove tese fields? For example 
>>>> MetalScrollBarUI.bumps looks similar to other fields 
>>>> MetalScrollButton.increaseButton/decreaseButton.
>>>>
>>>> On 13.10.16 16:15, Alexander Scherbatiy wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the fix:
>>>>>   bug: https://bugs.openjdk.java.net/browse/JDK-8167176
>>>>>   webrev: http://cr.openjdk.java.net/~alexsch/8167176/webrev.00
>>>>>   - Inaccessible classes returned by public API in swing are 
>>>>> changed to
>>>>> their public super classes.
>>>>>   - Fields which expose inaccessible classes are deprecated and 
>>>>> marked
>>>>> for removal.
>>>>>
>>>>>   Changing a public field type to its superclass is incompatible 
>>>>> change
>>>>> with previous releases. However it is not possible to use a class 
>>>>> which
>>>>> is not exported in JDK 9 because of the modularization feature.
>>>>>
>>>>>   Thanks,
>>>>>   Alexandr.
>>>>>
>>>>
>>>>
>>>
>