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

[John Hendrikx]

On Wed, 19 Feb 2025 00:22:59 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

> Another scenario I want to bring to attention: Listener B does nothing, listener A sets the value to 5 and then removes listener B. Listener A is registered first. Setting the property value from 0 to 1 to trigger the chain.
> 
> ```
> A got 0->1; A set->5
>   A got 1->5; does nothing
>   A removes B
> A removes B
> ```
> 
> B does not have time to react to either change. It does not react to the `1->5` nested change because these were trimmed so that it won't have a `1->5` incorrect old value notification. It also doesn't react to the `0->5` event at the top level because it was removed in the nested event.
> 
> I'm not sure if this is the expected behavior from a user. Should B never have a chance to react?

I'm going to assume that A and B are aware of each other, and are somehow working together, as this scenario makes little sense otherwise.  The code that is using these two listeners is then likely coming from the same implementation (how else would A have a reference to B).  So, given that, if an implementation decides to remove B, no matter when that happens exactly (outside a notification call, or within one), would it be reasonable to expect a callback still?  

A removal may have entailed cleaning up of other resources.  Having to code listeners defensively "just in case" it was called still after removal seems to me not something you should need to do.

So, I think the least surprising scenario would be to have removals take immediate effect. It is however up to the system as a whole to decide this. The rules governing the system IMHO should be:

- When listener is first added, the system has leeway in deciding when it will be called first, but at a minimum should call it for the next top-level change
- When a listener has been called once, it **must** be called for every subsequent change after the first (with the exception of callbacks that are not changes, ie. `5 -> 5`, which can only happen if earlier listeners modify the value)
- When a listener is removed, the system has leeway in deciding when its last notification will be, but at a minimum should not call a removed listener for a new top-level change

Given that:
- a top-level action is an action that is not happening in a notification callback of the same property; ie. the property is currently "idle" and not processing a notification
- a nested action is an action that occurs in a notification of the **same** property

Here is a comparison table:

|    |When?|ExpressionHelper|ListenerManager (new)|
|---|---|---|---|
|Listener addition|Top-level|Called on next top-level change|Called on next top-level change|
| |Nested|Called on next top-level change, unless value changes|Called on next top-level change(*)|
|Value Changes|Top-level|Calls always with correct old value|Calls always with correct old value|
| |Nested|Calls always but possibly with an old value not matching previous new value|Calls only if value changed to ensure it does not have to send a bogus old value|
|Listener removal|Top-level|No further calls|No further calls|
| |Nested|May still be called **after** removal if registered later|Never, removal is immediate|

(*) subject to debate

I feel I also should point out that such dependencies between listeners (where listener A removes B, or adds C or whatever) can only happen in code that is aware of these other listeners.  There is no way to obtain a reference to a listener from the system, and so all of these scenario's have perfect knowledge of what each listener does and their life cycles.  There can be no surprises here, listeners can't be added or removed by some other class or 3rd party dependency without a reference to them.

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

PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-2667270380