RFR: 8324658: Allow animation play/start/stop/pause methods to be called on any thread [v5]

[Jose Pereda]

On Sat, 27 Jan 2024 17:26:45 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> Added a utility method to run code on the FX thread if it's not already, and changed the animation methods to use it.
>
> Nir Lisker has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update documentation on AnimationTimer methods

Changes look fine. I have two minor comments.

modules/javafx.graphics/src/main/java/com/sun/javafx/util/Utils.java line 1008:

> 1006:      * @param runnable a {@code Runnable} encapsulating the code
> 1007:      */
> 1008:     public static void runOnFxThread(Runnable runnable) {

I understand that this is a private package, and not public API, but do we need a null check of `runnable`? (Not needed for the current uses added from this PR, but possibly for future uses?)

modules/javafx.graphics/src/main/java/javafx/animation/Animation.java line 903:

> 901:     }
> 902: 
> 903:     private void playFromOnFxThread(String cuePoint) {

I take that adding these private `xxOnFxThread` methods is convenient so they can be passed easily to the runnable in `Utils::runOnFxThread`, or accessed from other subclasses. 

However, I'm not sure about the method naming `OnFxThread`, as it might imply that what it does is already done on the FX thread, therefore not needing to be wrapped up by `runOnFxThread`. 

Does `runXX` (i.e. `runPlayFrom`) make more sense? In any case, I'd be fine with the current proposal as is.

-------------

PR Review: https://git.openjdk.org/jfx/pull/1352#pullrequestreview-1847205003
PR Review Comment: https://git.openjdk.org/jfx/pull/1352#discussion_r1468586394
PR Review Comment: https://git.openjdk.org/jfx/pull/1352#discussion_r1468590831