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

[Alexandr Scherbatiy]

Hello,

Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/8167176/webrev.02

   - JRootPane.defaultPressAction/defaultReleaseAction fields are removed
   - MetalBorders.bumps and MetalScrollBarUI.bumps are made private
   - MetalFileChooserUI.createDirectoryComboBoxRenderer() method is made 
private

Thanks,
Alexandr.

On 10/18/2016 8:23 PM, Phil Race wrote:
>
>
> 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.
>>>>>>
>>>>>
>>>>>
>>>>
>>
>