[OpenJDK 2D-Dev] [9] RFR JDK-8164931 : Verify if writer.abort() works properly for all writers in IIOWriteProgressListener.

[Prasanta Sadhukhan]

Is it not required to call abortRequested() after

1340                         processImageProgress(percentageDone);

in BMP

In GIF 1044 if (abortRequested()) {
1045 return;
1046 } and 1054                 if (abortRequested()) {
1055                     return;
1056                 }

can be merged and called after if-else block!!

In the testcase, I believe we do not need to have try-catch-finally 
block, just a try-finally block should do as we are not doing 
anyprocessing in catch block.

Regards
Prasanta
On 9/16/2016 5:26 PM, Jayathirth D V wrote:
> Hi Sergey,
>
> Thanks for your review.
> abortRequested in insert() should be at the end of the function only.
> It has come by mistake while I was creating webrev as I was testing other things.
> Please find the updated webrev with abortRequested() check at the end of insert() function for review:
> http://cr.openjdk.java.net/~jdv/8164931/webrev.01/
>
> Regards,
> Jay
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Friday, September 16, 2016 4:36 PM
> To: Jayathirth D V; Philip Race; Prasanta Sadhukhan; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8164931 : Verify if writer.abort() works properly for all writers in IIOWriteProgressListener.
>
> Looks fine, expect the changes in TIFFImageWriter.java. I am not sure I understand why abortRequested was moved from the end of insert();
>
> On 16.09.16 12:04, Jayathirth D V wrote:
>> Hi,
>>
>>
>>
>> Please review the following fix in JDK9 at your convenience:
>>
>>
>>
>> This issue is similar to
>> https://bugs.openjdk.java.net/browse/JDK-4924727 where we made changes
>> to all ImageReader plugins.
>>
>>
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8164931
>>
>>
>>
>> Webrev : http://cr.openjdk.java.net/~jdv/8164931/webrev.00/
>>
>>
>>
>> Issue : Verify that when we issue ImageWriter.abort() in
>> IIOWriteProgressListener callbacks whether writing is aborted properly.
>>
>>
>>
>> Root cause : In many writer plugins we are not checking for
>> abortRequested() right after IIOWriteProgressListener callbacks. In
>> which case writing may continue until we check for abortRequested().
>> Also in some writers we are not calling clearAbortRequest() before
>> every
>> write() call.
>>
>>
>>
>> Solution : Check for abortRequested() after every
>> IIOWriteProgressListener callbacks and before every write() call we
>> should have clearAbortRequest() called.
>>
>>
>>
>> In case of JPEG clearAbortRequest() is overridden in JPEGImageWriter
>> and it clears native abort flag also so there is no change in
>> JPEGImageWriter. WBMP changes will be done in JDK-8164930 as
>> checkSampleModel() is failing for WBMP.
>>
>> Thanks,
>>
>> Jay
>>
>>
>>
>
> --
> Best regards, Sergey.