RFR: 8196587: Remove use of deprecated finalize method from JPEGImageLoader

[Ambarish Rapte]

On Mon, 2 Dec 2019 20:40:09 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

> On Mon, 2 Dec 2019 20:36:20 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
> 
>> On Mon, 2 Dec 2019 15:03:47 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:
>> 
>>> On Tue, 26 Nov 2019 15:16:41 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>>> 
>>>> On Tue, 26 Nov 2019 15:16:38 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:
>>>> 
>>>>> The finalize() method is deprecated in JDK9. See [Java 9 deprecated features](https://www.oracle.com/technetwork/java/javase/9-deprecated-features-3745636.html).
>>>>> And so the JPEGImageLoader.finalize() method should be removed.
>>>>> 
>>>>> The change is,
>>>>> 1. Remove finalize method from JPEGImageLoader class.
>>>>> 
>>>>> 2. Instance of JPEGImageLoader is created and used in ImageStorage class. JPEGImageLoader.dispose() should be called after it's use over. This would be a common call for the other (GIF, PNG, BMP) ImageLoader classes, which have empty dispose() method.
>>>>> 
>>>>> 3. JPEGImageLoader.load() method almost always calls the dispose() method after the image is loaded. In normal scenario JPEGImageLoader is disposed here. The calls to dispose() added in ImageStorage seem logically correct place to add and should be added.
>>>>> 
>>>>> Verification:
>>>>> Verified :graphics:test and system tests on all three platforms.
>>>>> Verified that JPEGImageLoader.dispose() is always initiated by JPEGImageLoader.load()
>>>>> 
>>>>> ----------------
>>>>> 
>>>>> Commits:
>>>>>  - b48c8087: 8196587: Remove use of deprecated finalize method from JPEGImageLoader
>>>>> 
>>>>> Changes: https://git.openjdk.java.net/jfx/pull/50/files
>>>>>  Webrev: https://webrevs.openjdk.java.net/jfx/50/webrev.00
>>>>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8196587
>>>>>   Stats: 28 lines in 2 files changed: 14 ins; 12 del; 2 mod
>>>>>   Patch: https://git.openjdk.java.net/jfx/pull/50.diff
>>>>>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/50/head:pull/50
>>>> 
>>>> I still need to double-check all calls to dispose, but I think this is essentially the right solution, and is ready to be submitted for review. I added one comment inline.
>>>> 
>>>> modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java line 273:
>>>> 
>>>>> 272:             } else {
>>>>> 273:                 throw new ImageStorageException("No loader for image data");
>>>>> 274:             }
>>>> 
>>>> Now that this is moved inside a try/catch, this `ImageStorageException` will get wrapped in another `ImageStorageException` if it is ever thrown. You probably want to catch `ImageStorageException` below and re-throw it without wrapping it.
>>> 
>>> Updated the catch statement.
>> 
>> There is one more ImageLoader implementation class that still has a finalize method: [IosImageLoader.java(https://github.com/openjdk/jfx/blob/3ba659f0eb52496adb54a2b2c313e8da832e8389/modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ios/IosImageLoader.java#L215).
>> 
>> That method should also be removed, but @johanvos should confirm.
>> 
>> The rest looks fine to me.
> 
> This will need two reviewers anyway, so I tagged @johanvos to review as well.

> There is one more ImageLoader implementation class that still has a finalize method: [IosImageLoader.java](https://github.com/openjdk/jfx/blob/3ba659f0eb52496adb54a2b2c313e8da832e8389/modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ios/IosImageLoader.java#L215).

Removed `finalize()` from `IosImageLoader.java`, updated PR.

PR: https://git.openjdk.java.net/jfx/pull/50