On Sat, 27 Jan 2024 15:48:02 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/javafx/animation/Timeline.java line
>> 196:
>>
>>> 194: @Override
>>> 195: void stopOnFxThread() {
>>> 196: // The parent check is redone in the super method. Consider
>>> refactoring.
>>
>> As you've probably already seen, this causes infinite recursion and an
>> eventual StackOverflow. The easiest (and probably safest) fix is to revert
>> this part of the change, keeping the overridden `stop` method, and have it
>> call a private `stopOnFxThread` method (and make the one in the parent class
>> also private). You can file a bug to refactor it later.
>
> I just pushed a fix that calls the correct super method. The implementation
> isn't fully tested yet because I wanted to get the CSR moving in parallel.
>
> I'm still thinking on the least intrusive way to write the tests. It seems
> that the only way to check if a method runs on the FX thread is to override a
> method on its stack and check there. That means that I would need to expose
> (package-private) the `play/start/stop/pauseOnFxThread` methods.
It works now with change to call the right super method.
As for the tests, I'd sooner not do anything intrusive in this PR, especially
given the tight window to get this in. I'm not even sure whether it's worth
inspecting the internals to make sure it is on the right thread in all cases,
but if so, a follow-up testbug could be filed for that.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1352#discussion_r1468515272
