RFR: 8349091: Charts: exception initializing in a background thread [v4]

[Andy Goryachev]

On Mon, 24 Feb 2025 23:56:01 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 22 commits:
>> 
>>  - Merge remote-tracking branch 'origin/master' into 8349091.charts.thread.safety
>>  - enabled pie chart test
>>  - Merge branch 'master' into 8349091.charts.thread.safety
>>  - Merge branch 'master' into 8349091.charts.thread.safety
>>  - whitespace
>>  - Merge remote-tracking branch 'origin/master' into 8349091.charts.thread.safety
>>  - cleanup
>>  - tests pass
>>  - chart tests only
>>  - more jitter
>>  - ... and 12 more: https://git.openjdk.org/jfx/compare/307e3087...e60c027b
>
> modules/javafx.controls/src/main/java/javafx/scene/chart/Chart.java line 551:
> 
>> 549:             ChangeListener<Scene> li = new ChangeListener<>() {
>> 550:                 @Override
>> 551:                 public void changed(ObservableValue<? extends Scene> src, Scene prev, Scene scene) {
> 
> This is insufficient. It will miss the case where both the node and the scene are created in the background thread, and the scene is later added to a window on the FX app thread. I tested this case and verified my assertion. This also means that in some cases, if this listener is called from a background thread, you will create a new scene listener from the current scene listener.
> 
> Consider instead using `TreeShowingProperty`, which is used by `ProgressIndicatorSkin` to set up its animation.`TreeShowingProperty` sets up a listener chain that fires when the the "tree showing" status of a node changed (tree showing means the node and all ancestors are visible and is attached to a scene that is attached to a window that is showing.
> 
> Alternatively, since you might not care whether the node is visible, you can just set up a binding using `ObservableValue::flatMap`. The code snippet of the docs:
> 
> 
> ObservableValue<Boolean> isShowing = sceneProperty()
>     .flatMap(Scene::windowProperty)
>     .flatMap(Window::showingProperty)
>     .orElse(false);
> 
> 
> And then you can then add a listener (or a value subscription might be better) on `isShowing` -- when it becomes true, you can setup the binding or listener to `Platform.accessibilityActiveProperty`.
> 
> With either of the above two solutions, the showing state will only become true on the FX app thread.

thanks for noticing!  the old code was indeed flawed.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1697#discussion_r1970371417