RFR: 8307538: Memory leak in TreeTableView when calling refresh [v4]

[Andy Goryachev]

On Fri, 12 May 2023 08:13:01 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   one more test case
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java line 103:
> 
>> 101:             updateTreeItem();
>> 102:             // There used to be an isDirty = true statement here, but this was
>> 103:             // determined to be unnecessary and led to performance issues such as
> 
> These changes don't fail the test when I undo them.  As said before, they're unnecessary.
> 
> Reasoning: they register on `TreeTableRow`, which is associated with the skin directly. If their lifecycles don't match, then `dispose` will take care of unregistering.  If their lifecycles do match, then they go out of scope at the same time.
> 
> Unless you prefer using the `register` functions, I think this change should be undone.

Even though I don't particularly like register*Listener() because of its asymmetric nature (when it comes to removing listeners), here we do need to create weak listeners that get unregistered upon `dispose()`.  We need weak listeners because TreeTableView does not explicitly "disconnect" unused cells (e.g. refresh()), and we need `dispose()` due to skin life cycle.

So, in this particular case, I think this change should be ok.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1129#discussion_r1192542423