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

[Alexander Zuev]

On Fri, 7 Feb 2025 18:43:32 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Root Cause:
>> (Multiple) properties are getting bound to the global `Platform.accessibilityActive` property. Binding (and I say, accessing) of properties is not thread-safe.
>> 
>> I also changed the design a bit.  Originally, every symbol in a chart had its `focusTraversableProperty` bound to `Platform.accessibilityActive` in order to enable the accessibility navigation across the chart data points.  This is rather inefficient, as the property has to manage (thousands?) of listeners.
>> 
>> Instead, a single boolean property is added to each chart, with a listener added to it which iterates over data symbols to toggle the `focusTraversableProperty` directly.
>> 
>> The exact moment when the new property gets bound is also important, and has to happen in the FX application thread.
>> 
>> With this change, it is safe to create and populate charts with data in a background thread.
>> 
>> ---
>> 
>> ## NOTES
>> 
>> 1. It looks like the `Platform.accessibilityActive` property never transitions back to false  after it transitioned to true.  Some say it is because there is no mechanism in the platform to get notified which cannot possibly be true.
>> 2. The javadoc for `Platform.accessibilityActiveProperty()` method stipulates that "_This method may be called from any thread_" which is patently not true as the current implementation is simply not thread-safe.
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 19 commits:
> 
>  - 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
>  - Merge remote-tracking branch 'origin/master' into 8348423.node.thread.safety
>  - skip tests
>  - jiggler
>  - ... and 9 more: https://git.openjdk.org/jfx/compare/2cf9779b...4b6089e4

Marked as reviewed by kizune (Author).

Ok, changes look good and they seems to work on Mac OS. I am curious why do we make labels in charts focus traversable when a11y is switched on? May be something to do with accessibility on Windows? I haven't tested this patch on Windows yet. On Mac OS user can move accessibility cursor to non-focusable elements such as text labels and images so we should not have a problem navigating to the chart labels. Needs to be investigated more.

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

PR Review: https://git.openjdk.org/jfx/pull/1697#pullrequestreview-2611391786
PR Comment: https://git.openjdk.org/jfx/pull/1697#issuecomment-2653202615