On Fri, 17 Apr 2026 22:57:56 GMT, Marius Hanl <[email protected]> wrote:
>> This fixes an IOOBE and even improves the performance / change aggregation.
>> I suggest reading Jeanette's comment in the ticket, that helped a lot to
>> understand it.
>>
>> There was a code path that broke change events.
>> Jeanette wondered as well in the comment why this exists at all, if this was
>> meant to be some kind of optimization?
>>
>> What I found out is: Because of this code path, we actually got 3 change
>> events and the order was wrong.
>> What we got with the test from Jeanette: `subChanges: { [B] removed at 1,
>> [D] removed at 2, [C, E, F] removed at 1 }`
>>
>> Removing the code path and just calling `insertRemoved(..)` directly will
>> correctly find an alreadx existing sub change with `findSubChange` and
>> aggregate the changes together. So now, we only get one event: `subChanges:
>> { [B, C, D, E, F] removed at 1 }`. No error and we use the smart aggregation
>> from `insertRemoved`.
>>
>> My speculation why this code existed is that there may was a former bug,
>> similar like [JDK-8208758](https://bugs.openjdk.org/browse/JDK-8208758)
>> (#2117) where I also did a fix in the `insertRemoved(..)` method.
>>
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Marius Hanl has updated the pull request incrementally with one additional
> commit since the last revision:
>
> removed whitespace
I agree, the original "optimization" looked sus.
Verified that the added tests fail in master and pass with the fix.
[JDK-8195614](https://bugs.openjdk.org/browse/JDK-8195614) and
[JDK-8195750](https://bugs.openjdk.org/browse/JDK-8195750) are still broken (as
expected).
It's as if we need to write a comprehensive set of tests of `FilteredList` and
maybe `SortedList`...
-------------
Marked as reviewed by angorya (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/2154#pullrequestreview-4132650327
