Hi Sergey/Alexey
Webrev: http://cr.openjdk.java.net/~pbansal/8230235/webrev01/
<< 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.
<<This requires a comment, doesn't it? Otherwise the condition in if looks
always true. Is it possible that image becomes null before getWidth() is called?
I have added the comment. I don't see a case where image is null before calling
getWidth, so did not add a null check there.
<< I agree with Sergey, try-catch are not necessary in this case: you declared
that main throws Exception. Any exception indicates the test failed.
<<Since JLabel is a Swing component, you should create its instance on EDT.
SwingUtilities.invokeAndWait would do the job.
Done
Regards,
Pankaj
-----Original Message-----
From: Alexey Ivanov
Sent: Wednesday, October 16, 2019 7:27 PM
To: Pankaj Bansal; swing-dev
Cc: Sergey Bylokhov
Subject: Re: <Swing Dev> [14] RFR JDK-8230235 - Rendering HTML with empty img
attribute and documentBaseKey cause Exception
Hi Pankaj,
I'd like to confirm:
793 int width = image.getWidth(imageObserver);
794 if (image != null) {
the image is not null at line 793, and as the side effect of calling
image.getWidth(imageObserver), image is set to null, right?
This requires a comment, doesn't it? Otherwise the condition in if looks always
true. Is it possible that image becomes null before getWidth() is called?
Can we implement this differently?
I guess we'll also get NPE if src attribute of <image> tag is not an image.
I agree with Sergey, try-catch are not necessary in this case: you declared
that main throws Exception. Any exception indicates the test failed.
Since JLabel is a Swing component, you should create its instance on EDT.
SwingUtilities.invokeAndWait would do the job.
Regards,
Alexey
On 10/10/2019 21:35, Sergey Bylokhov wrote:
> Hi, Pankaj.
> 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".
> Also, it is not necessary to have a try/catch block in the test, just
> call the problematic method
>
> On 10/10/19 11:01 am, Pankaj Bansal wrote:
>> Hi All,
>>
>> Please review the following fix for jdk14.
>>
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8230235
>>
>> Webrev: http://cr.openjdk.java.net/~pbansal/8230235/webrev00/
>>
>> Issue:
>>
>> Rendering HTML with empty img attribute and documentBaseKey set
>> causes NPE
>>
>> Fix:
>>
>> The issues is introduced after
>> https://bugs.openjdk.java.net/browse/JDK-8218674.
>>
>> When the documentBaseKey is set, but img tag is not set, the image
>> URL may not be complete and may not point to actual image. In
>> ImageView class, when getWidth(ImageObserver) is called from the
>> updateImageSize, it checks if image source is null and adds the
>> ImageObserver (ImageView.ImageHandler) on the Image and sets the
>> ImageView image as null. When getHeight is called, as image is set to
>> null, it results in NPE.
>>
>> We should verify that image is not null.
>>
>> Testing:
>>
>> I have tested it on Window, Linux and Mac.
>>
>>
>> Regards,
>> Pankaj Bansal
>>
>
>
