On Wed, 19 Feb 2025 00:22:59 GMT, Nir Lisker <nlis...@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

Reply via email to