<Swing Dev> [14] RFR JDK-8230235 - Rendering HTML with empty img attribute and documentBaseKey cause Exception

[Pankaj Bansal]

Hi Sergey/Alexey

<< You need to read the image to the local var under synchronization and then get W/H from the local image variable. I guess in this case both W/H will be 0 since error is occured?
Done. As Alexey pointed, the values are set to -1 when the error occurs.

<< I suggest clarifying the reason why image may become null:
<<794                     int width = image.getWidth(imageObserver);
<<795                     // Calling image.getWidth could reset image to null if an error during loading occurred
<<796                     if (image != null) {
This will not be needed now as we are using the local variable.

<<Thanks
<<The <img> tag should not be closed as in XML. I mean <img src=''/> should be <img src=''>
<<Backlash for closing tag is not standard HTML.
<<Other than that, the test looks good.
Done. Along with this, I have removed the "headful" tag from the test as it is headless test. I added it by mistake earlier.

I have run a mach5 job and it is green on all platforms.

Webrev: http://cr.openjdk.java.net/~pbansal/8230235/webrev02/

Regards,
Pankaj

-----Original Message-----
From: Alexey Ivanov 
Sent: Wednesday, November 13, 2019 1:28 AM
To: Sergey Bylokhov; Pankaj Bansal; swing-dev
Subject: Re: <Swing Dev> [14] RFR JDK-8230235 - Rendering HTML with empty img attribute and documentBaseKey cause Exception

On 31/10/2019 19:53, Sergey Bylokhov wrote:
> On 10/31/19 5:59 am, Pankaj Bansal wrote:
>> << I guess that the problem is that the "image" is accessed w/o 
>> synchronization on "this" like in other places, but it will be 
>> necessary to add one null check before usage of "image".
>> I have updated the code to access image with synchronization. But 
>> that does not solve the issue. So I have kept the null check also. I 
>> tried to add the null check before calling getWidth, calling getWidth 
>> still makes the image null. So the check before calling getHeight is 
>> still required.
>
> You need to read the image to the local var under synchronization and 
> then get W/H from the local image variable.
> I guess in this case both W/H will be 0 since error is occured?

You're right! This approach seems to be easier to understand.

I think the current code can deadlock: we call image.getWidth() holding the monitor of ImageView.this; ImageObserver (line 958) needs to lock the monitor of ImageView.this to reset 'image' field to null if an error occurred.

If an error occurs, width and height are set to -1.

--
Regards,
Alexey