RFR: 8159055: ImageIcon.setImage can't handle null parameter [v19]

[Phil Race]

On Fri, 25 Jul 2025 03:36:44 GMT, Prasanta Sadhukhan <psadhukhan at 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