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

[John Hendrikx]

On Thu, 13 Mar 2025 03:31:13 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> John Hendrikx has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Change StackOverflowException to warning log
>>  - Support keeping last message in Logging helper
>
> modules/javafx.base/src/test/java/test/javafx/beans/ObservableValueTest.java line 70:
> 
>> 68: 
>> 69:     /*
>> 70:      * ObservableValue cases to test:
> 
> I would add that 2 values are provided to transition between for each properties test. For bindings, a setter and a modifier are provided too. I suggest that `Case`'s docs will outline what they are for.

I've been re-reading my own test code (as it is I think 2 years old already), and I added more docs where it was unclear :)  I added more docs here especially.

> modules/javafx.base/src/test/java/test/javafx/beans/ObservableValueTest.java line 669:
> 
>> 667:      * Takes a list of values and creates an iterator of pairs of those values. The iterator
>> 668:      * does not only return all possible pair combinations, but also different transitions between
>> 669:      * two pairs. Effectively, given x values it returns x^3 combinations.
> 
> Very worth noting that the pairs are used for the numbers for invalidation and change listeners.
> 
> Also, there are duplicate pairs with the same pairs before and after, so it seems to be repeating the same state. For example, the sequence
> [1, 2]
> [1, 0]
> [1, 50]
> repeats several times. Not sure if this is on purpose, but looks redundant.

It was intended; the add/removal code is complex (as it may switch implementations to conserve memory) and we need to be absolutely sure that a simple action like adding or removing a listener (even of a different type) doesn't affect the system.  In `ExpressionListener` there were bugs were adding or removing a listener could affect the next change being detected or not, and we'd want to make sure that can't happen.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r2134524537
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r2134524011