RFR: 8345188: Support tree-structural pseudo-classes [v6]

[Michael Strauß]

On Wed, 8 Jan 2025 11:25:37 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
>> 
>>  - Merge branch 'master' into feature/root-pseudo-class
>>  - whitespace error
>>  - add child-indexed pseudo-classes
>>  - add pseudo-class table to Parent
>>  - update cssref
>>  - Set :root pseudo-class on sub-scene root node
>>  - Set :root pseudo-class on root node
>
> modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 341:
> 
>> 339: 
>> 340:         @Override
>> 341:         protected void onChanged(Change<Node> c) {
> 
> I hate to bring this up, and it perhaps should be dealt with separately, but how safe is this code in `onProposedChange` / `onChanged` when it is re-entered due to the many potential listener callbacks?  For example, the removal of a child on `parent` can trigger list change listeners from the user that can trigger all kinds of other changes that eventually lead to more children changes on this node, triggering a nested `onChanged`; the new changes are adding more opportunities in the form of listeners on pseudo state changes.
> 
> For example, `onProposedChange` will blatantly reset `geomChanged` to `false` in all cases, even if it rejects the change later.  It doesn't check if an `onChange` is in progress higher up the call stack, effectively resetting a flag that is in active use...
> 
> I believe this may be quite a difficult problem to solve if nested calls remain allowed, and one of the easiest ways to fix it IMHO is to reject any changes (in `onProposedChange`) when a change is already in progress. For users this would effectively mean that listeners triggered during a child modification will not be allowed to make further modifications to the same children list until the first one completes.  This is not a common situation, but can happen with very dynamic UI's that construct new nodes in response to triggers; without such protection for nested changes, the errors that crop up are often of a form that seem to be multi-threading issues (where certain invariants seem to be broken), while in reality they're just re-entrant code calls that use instance fields in an unsafe manner.

One option might be to move all pseudo-class modifications to the end of the `onChange` method. This would make the code equivalent to being called after the callback has completed.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1652#discussion_r1907567157