On Tue, 21 Feb 2023 16:31:10 GMT, John Hendrikx <jhendr...@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 incrementally with one additional > commit since the last revision: > > Do not collapse nested changes into a single change I've included a fix which prevents nested changes from being collapsed into a single change. This should fix the failing test in `ScheduledServiceTest`. There is still two open issues that I'm aware of, and which I'm looking into: 1. `ExpressionHelper` can morph itself from `Generic` to single variants and vice versa. If such a change were to occur and another nested change is triggered, a different implementation of `fireValueChangedEvent` is called that may not have the same state as the other implementation (nested changes may potentially get lost). 2. When listeners are added or removed, this currently doesn't take effect until all emissions have completed. This may or may not be a good thing. In the implementation before this PR, the changes would take effect for only the nested calls, but not for the emissions that are higher up on the stack. ------------- PR: https://git.openjdk.org/jfx/pull/837
