<Swing Dev> [jdk17] RFR: 8269269: [macos11] SystemIconTest fails with ClassCastException

[Sergey Bylokhov]

On Wed, 30 Jun 2021 01:13:29 GMT, Alexander Zuev <kizune at openjdk.org> wrote:

>> test/jdk/javax/swing/JFileChooser/FileSystemView/SystemIconTest.java line 75:
>> 
>>> 73:     static void testSystemIcon(File file, boolean implComplete) {
>>> 74:         int[] sizes = new int[] {16, 32, 48, 64, 128};
>>> 75:         if (!file.exists() || !file.canRead()) {
>> 
>> If this file is not accessible the "getSystemIcon" should return "null" you can check it here.
>
>> If this file is not accessible the "getSystemIcon" should return "null" you can check it here.
> 
> If file is not accessible we still return default icon for file or folder - if we, for example, looking at the folder in file explorer  with file not readable then we should display some icon for it. It will not be icon from the file itself - because we can not read it - but it will not be null.

1. If the file does not exist then the getSystemIcon() will always return null, but what happens if the file cannot be read? We will call the "sf = ShellFolder.getShellFolder" and then we call "sf.getIcon()" and it returns what? As far as I understand it always return MultiResolutionIconImage, no?

2. If we return some icon when the file cannot be read then you can check it here as well.

>> test/jdk/javax/swing/JFileChooser/FileSystemView/SystemIconTest.java line 88:
>> 
>>> 86:             }
>>> 87: 
>>> 88:             ImageIcon icon = (ImageIcon) i;
>> 
>> One more time would like to highlight that to get the data of the requested image the user need to do one "if" and two "instanceof". Still hope that we can improve this.
>> 
>>     Icon icon = fsv.getSystemIcon(file, width, height);
>>     if (icon.getIconWidth() != width && icon.getIconHeight() != height) {
>>         return scaleTheIconInTheSameWayAsBeforeTheFix(icon, width, height);
>>     } else if (icon instanceof ImageIcon) {
>>         ImageIcon imageIcon = (ImageIcon) icon;
>>         if (icon.getImage() instanceof MultiResolutionImage) {
>>             MultiResolutionImage mri = (MultiResolutionImage) icon.getImage();
>>             return  mri.getResolutionVariant(width, height);
>>         } else {
>>             return imageIcon;
>>         }
>>     } else {
>>         return icon;
>>     }
>
>> One more time would like to highlight that to get the data of the requested image the user need to do one "if" and two "instanceof". Still hope that we can improve this.
> 
> We might when the implementation will be complete on all the relevant platforms, then we can be sure that we can provide specific type of an icon like ImageIcon or even MultiResolutionImage - but until then i would tend to use common denominator like simple Icon.

But you cannot change the return type later, so all these instanceof+casts will be there forever.

-------------

PR: https://git.openjdk.java.net/jdk17/pull/178