On Tue, 26 Nov 2019 15:16:38 GMT, Ambarish Rapte <ara...@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.



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

Reply via email to