RFR: 8320692: Null icon returned for .exe without custom icon [v2]

[Alexey Ivanov]

On Thu, 18 Jan 2024 19:34:11 GMT, Alexander Zuev <kizune at openjdk.org> wrote:

>> src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1208:
>> 
>>> 1206:             } else {
>>> 1207:                 return new MultiResolutionIconImage(size, multiResolutionIcon);
>>> 1208:             }
>> 
>> Can we return `null` immediately if an icon that we're about to put into `multiResolutionIcon` is `null`?
>> 
>> That is change the line 1198(1195)
>> https://github.com/openjdk/jdk/blob/1640fbb1d4b1bcc5196b0055858403a4bd524359/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L1198
>> 
>> to
>> 
>> 
>>     if (newIcon == null) {
>>         return null;
>>     }
>>     multiResolutionIcon.put(s, newIcon);
>> 
>> 
>> I assume if `newIcon == null` then `hIcon` is also `null`, in which case we can move this condition to the line 1195(1192) 
>> 
>> https://github.com/openjdk/jdk/blob/1640fbb1d4b1bcc5196b0055858403a4bd524359/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java#L1195
>> 
>> before calling `makeIcon`.
>
> Ok, i will move it to the line 1198 because i can not guarantee that we do not need to dispose hIcon if we can not make icon out of it so i would rather avoid potential resource leak.

We need to dispose of `hIcon` if it's non-null. I see that in this case `hIcon` is not null yet `makeIcon` fails to get the bitmap for whatever reason.

It would be good to understand why `makeIcon` fails even though `hIcon` is not null.

It is possible that `makeIcon` fails to extract an icon, so the added null-check ensures `null` is returned instead of returning a broken MRI.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17475#discussion_r1459441089