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
