RFR: 8320692: Null icon returned for .exe without custom icon
Alexey Ivanov
aivanov at openjdk.org
Thu Jan 18 17:39:13 UTC 2024
On Thu, 18 Jan 2024 00:47:00 GMT, Alexander Zuev <kizune at openjdk.org> wrote:
> Replaced asserts with NullPointerException calls because outside of testing that would be more informative - i do not think many people running their applications with assertions in system libraries enabled;
> Added a code that will analyze the result of the getIcon and will fall back to the default icon for the file type retrieved from the ShellFolder;
> Added a test case made by Aleksei Ivanov.
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`.
src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolder2.java line 1423:
> 1421: if (resolutionVariants.containsValue(null)) {
> 1422: throw new NullPointerException("There are null icons in the MRI variants map");
> 1423: }
I'm unsure about this. The idea with assert was to provide a debugging aid without affecting regular runtime. We control all the paths that call this constructor and, therefore, we should ensure it's never called if `resolutionVariants` contains `null`.
It's exactly what you've done in this PR.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17475#discussion_r1457770744
PR Review Comment: https://git.openjdk.org/jdk/pull/17475#discussion_r1457775052
More information about the client-libs-dev
mailing list