RFR: 8290310: ChangeListener events are incorrect or misleading when a nested change occurs [v3]

[Ambarish Rapte]

On Wed, 8 Feb 2023 14:54:52 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> This contains the following:
>> - Nested changes or invalidations using ExpressionHelper are delayed until the current emission completes
>>   - This fixes odd change events being produced (with incorrect oldValue)
>>   - Also fixes a bug in ExpressionHelper where a nested change would unlock the listener list early, which could cause a `ConcurrentModificationException` if a nested change was combined with a remove/add listener call
>> - A test for ExpressionHelper to verify the new behavior
>> - A test for all *Property and *Binding classes that verifies correct listener behavior at the API level (this tests gets 85% coverage on ExpressionHelper on its own, the only thing it is not testing is the locking behavior, which is not relevant at the API level).
>> - A fix for `WebColorFieldSkin` which triggered a nested change which used a flag to prevent an event loop (I've changed it now to match how `DoubleFieldSkin` and `IntegerFieldSkin` do it
>
> John Hendrikx has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:
> 
>  - Add copyright header
>  - Merge branch 'master' of https://git.openjdk.org/jfx into feature/delayed-nested-emission
>  - Merge branch 'master' of https://git.openjdk.org/jfx into feature/delayed-nested-emission
>  - Delay nested event emission

I did test this well and looks good to me. I tested changing the property value in more than one listener and combination of Invalidation and Change Listener. The behavior looks good and did not seem like it will cause regression(of course depends on specific scenarios)
I think the change may need a doc update, to explain this behavior.

Coming to the broad level of discussion, I think this change can still go in and other aspects can be worked on later whenever we get to them.

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/WebColorFieldSkin.java line 112:

> 110: 
> 111:                 if (!newText.equals(text)) {
> 112:                     getTextField().setText(newText);

I would recommend to keep only listener specific change in this PR. If the pattern matching can be skipped please revert those. I would be glad to review in a separate PR.

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

Changes requested by arapte (Reviewer).

PR: https://git.openjdk.org/jfx/pull/837