On Fri, 25 Jul 2025 03:36:44 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org>
wrote:
>> When trying to call 'icon.setImage(null);' where 'icon' is an instance of
>> ImageIcon, a null pointer exception is thrown at runtime.
>> The code tried to get the `id` for that image and instantiates
>> `MediaTracker` to associate the null image to that `id` and checks the
>> status of loading this null image, removes the null image from the tracker
>> and then tries to get the image width where it throws NPE as image is null.
>>
>> It's better to not go through all MediaTracker usage and bail out initially
>> itself for null image..
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Use code tag and update test
src/java.desktop/share/classes/javax/swing/ImageIcon.java line 64:
> 62: * are preloaded using MediaTracker to monitor the loaded state
> 63: * of the image.
> 64: * If the image source parameter to a constructor or method is non-null,
FWIW when I first suggested this statement I said it needed to cover the setter
too
'We probably should add the similar text on setImage().'
I guess we can keep it here now it's here.
src/java.desktop/share/classes/javax/swing/ImageIcon.java line 66:
> 64: * If the image source parameter to a constructor or method is non-null,
> 65: * but does not reference valid accessible image data,
> 66: * no exceptions will be thrown but the image will be 'effectively' null,
Joe asked for if there's an alternative to 'effectively' null. I'm open to
suggestions. "unset" ?
src/java.desktop/share/classes/javax/swing/ImageIcon.java line 184:
> 182: * be preloaded by using MediaTracker to monitor the loaded state
> 183: * of the image.
> 184: * Passing {@code null} URL will result in {@code
> NullPointerException}.
In the CSR, Joe quite reasonably requested that these all become @throws clauses
test/jdk/javax/swing/ImageIcon/ImageIconTest.java line 117:
> 115: ii.setImage((Image)null);
> 116: throw new RuntimeException("No NPE");
> 117: } catch (NullPointerException e) {
It occurred to me that you could include this in the switch with a new ARGTYPE
of "SETIMAGE".
Bit of a misnomer as its the type + the method but then you can also check the
non-null invalid data type.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2231847301
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2231851526
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2231839373
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2231842617
