On Thu, 18 Jan 2024 19:34:11 GMT, Alexander Zuev <kiz...@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
