RFR: 8319925: CSS.BackgroundImage incorrectly uses double-checked locking

[Alexey Ivanov]

On Tue, 5 Dec 2023 07:55:22 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

> > > > It is probably easy just drop the usage of loadedImage and use the image instead?
> > > 
> > > 
> > > Ideally. Yet there's a corner case: if `url` is null, there's nothing to load; and it doesn't make sense to re-try, the URL won't change. If it were not for this case, the `image` field alone would do the job.
> > 
> > 
> > Not sure how it is expected to work, the url is a parameter and can be changed on the fly(see the usage of base in the StyleSheet.java)
> 
> Yes, seems like setBase can be used to change image URL on the fly...Modified to use image field for DCL...

I am against this latest change: it doesn't fix much but it would cause creating the URL each time `getImage` is called if the URL is invalid for whatever reason.

Usually, `setBase` isn't changed on the fly, it's set once when the `StyleSheet` and/or `HTMLDocument` is created. If `StyleSheet.setBase` is called to modify the base, the image needs reloading *whether it's already loaded or not* — the latest change doesn't address this problem in any way: if the image is successfully loaded, it's what's returned even though the base has been changed; and only if the image couldn't be loaded with the old base, the image will be re-loaded again.

This is asymmetric: in one case, it's re-loaded, in another — it isn't.

I'm for keeping the current behaviour: if the URL is invalid, the image isn't loaded and will never be for this instance of` CSS.BackgroundImage`; if the URL is valid, the image gets loaded.

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

PR Comment: https://git.openjdk.org/jdk/pull/16917#issuecomment-1840752465