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

[Nir Lisker]

On Thu, 20 Feb 2025 14:08:26 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

> > Z 0->1
> > A 0->1; set 2
> >   Z 1->2;
> >   A 1->2; no change
> >   B 0->2
> >   C 0->2; sets 3
> >     Z 2->3
> >     A 2->3; if A sets 2 we will get a SOE because of recursive changes; let's say A wants value>=2 and not ==2
> >     B 2->3
> >     C 2->3; no change
> >     D 0->3
> >   D ignores set->2 event since it got a more updated value (tracked by the progress value?)
> > B, C, D ignore set->1 event since they got a more updated value

> Apart from the nesting you have, this is exactly what this PR currently does. The problem with the nesting you have is that we can really only track one old value per nesting. If we need to track individual old values then we need to wrap each listener and give the wrapper a field with what value was sent last to it.

Yes, it does the same, except for the SOE I noted. If the way I do the nesting incurs a performance penalty, then it can be ignored since it has the same result. As for the SOE...

Edge cases of 2 listeners fighting: As noted from my example, I prefer an exception (doesn't need to be SOE), but it will be difficult to produce artificially. I don't think you can know if 2 listeners will converge on a value or diverge.

Edge case of listener changing value again: I agree.

Edge cases of addition and removal: I'm fine with this PR's behavior, but noted my surprise that a listener (on the same property) did not receive a value *before* it was removed. I agree that this is not critical.

> I think that this implementation operates well within the current specification (ie. it could be integrated without any documentation clarifications), but I'm open to specifying some behavior (as provided by JavaFX) explicitly so applications may rely on it more (if they aren't already).

I think that for now we shouldn't specify anything, but wait to see if problems crop up after this gets released into the wild.

> Is there anything that still needs to be discussed at this point? I think that you found some interesting edge cases, but as most involved registering and removing listener on the same property on which you already have a listener, I think that those are more hypothetical than cases we need to document or remain compatible with.

Not really, except for maybe the first edge case where your opinion differs from the current behavior. You also noted

> When it comes to value changes, you can only have it one way; either last wins and earlier listeners may be unaware of a value being reset, or first wins and later listeners may be unaware of a value being reset. Neither will be intuitive; given the way listeners are currently already used for "veto" style behavior, I think it would be far better to go for the "first wins" option, as anyone can always install a later listener breaking whatever you intended in the last-wins case.

which is also reasonable ("first wins").

---

I still have to review the tests and some of the implementation, but I'm willing to approve this PR at any time if you're eager to integrate it and have enough reviewers.

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

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