On Mon, 3 Jul 2023 15:49:31 GMT, Jose Pereda <jper...@openjdk.org> wrote:
> This PR adds a check to the Animation and AnimationTimer public methods to
> verify that these are called from the JavaFX Application thread. If the call
> is done from any other thread, an IllegalStateException will be thrown.
>
> This will prevent users from getting unexpected errors (typically NPE, like
> the one posted in the JBS issue), and will fail fast with a clear exception
> and reason for it.
>
> The javadoc of the Animation and AnimationTimer classes and public methods
> has been updated accordingly.
>
> Tests for both classes have been included, failing (as in no exceptions were
> thrown when calling from a background thread) before this patch, and passing
> (as in ISE was thrown).
modules/javafx.graphics/src/main/java/javafx/animation/Animation.java line 990:
> 988: */
> 989: public void play() {
> 990: Toolkit.getToolkit().checkFxUserThread();
minor: perhaps this should first check for non-null parent, then for fx thread
(here and below)
tests/system/src/test/java/test/com/sun/javafx/animation/AnimationTimerTest.java
line 62:
> 60: startupLatch.countDown();
> 61: }
> 62:
minor: extra newline?
tests/system/src/test/java/test/com/sun/javafx/animation/AnimationTimerTest.java
line 82:
> 80: @Override public void handle(long l) {
> 81: frameCounter.countDown();
> 82: if (frameCounter.getCount() == 0L) {
thank you for 0L!
tests/system/src/test/java/test/com/sun/javafx/animation/AnimationTimerTest.java
line 112:
> 110: Platform.runLater(timer::start);
> 111: }
> 112:
minor: extra newline
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1167#discussion_r1279641091
PR Review Comment: https://git.openjdk.org/jfx/pull/1167#discussion_r1279642701
PR Review Comment: https://git.openjdk.org/jfx/pull/1167#discussion_r1279642982
PR Review Comment: https://git.openjdk.org/jfx/pull/1167#discussion_r1279642236
