RFR: 8349091: Charts: exception initializing in a background thread [v6]
John Hendrikx
jhendrikx at openjdk.org
Mon Mar 3 21:32:58 UTC 2025
On Mon, 3 Mar 2025 21:10:39 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/chart/AreaChart.java line 546:
>>
>>> 544: symbol.setAccessibleRole(AccessibleRole.TEXT);
>>> 545: symbol.setAccessibleRoleDescription("Point");
>>> 546: symbol.setFocusTraversable(isAccessibilityActive());
>>
>> So, if I understand correctly, we can't directly do this `bind`:
>>
>> symbol.focusTraversableProperty().bind(Platform.accessibilityActiveProperty())
>>
>> Because on a background thread there may not yet be an initialized `Platform`? Or perhaps, `Platform` would need to initialize and we don't want to do that yet on a background thread?
>
> The reason we `should not` be binding in this PR is because in part it's **bad design(tm)**: we are creating a zillion of listeners (one per plot data point); a better solution would be to create one listener and use (already existing series) to toggle the data points.
>
> The other reason is https://bugs.openjdk.org/browse/JDK-8351067 - the `Platform::accessibilityActive` property getter is not thread safe, but even if was, registering a listener with it may cause the change to come in **before** the node becomes a part of the scene graph, leading to unsynchronized multi-threaded access.
Was this solution considered:
ObservableValue<Boolean> partOfSceneGraph = this.sceneProperty()
.flatMap(Scene::windowProperty)
.flatMap(Window::showingProperty)
.orElse(false);
symbol.focusTraversableProperty()
.bind(Platform.accessibilityActiveProperty().when(partOfSceneGraph));
What the above code does is create a binding on the `when` statement (a listener is added on the `when` result). However, the `when` property will not add a listener on `Platform.accessibilityActiveProperty()` until `partOfSceneGraph` is `true`. As soon as it does though, a listener is installed on `Platform.accessibilityActiveProperty()` and its latest value is set to `focusTraversableProperty` via the bind.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1697#discussion_r1978264839
More information about the openjfx-dev
mailing list