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

[John Hendrikx]

On Thu, 11 May 2023 22:38:58 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Fixed a memory leak in TreeTableView by reverting to register**Listener (which is ok in this particular situation) - the leak is specific to TreeTableRowSkin.
>> 
>> Added a unit test.
>
> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   one more test case

I confirmed the tests fail before the changes in `setupTreeTableViewListeners` and pass with those changes.  Removing the constructor and dispose changes did not fail any tests.

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.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java line 161:

> 159:                     fixedCellSize = p.get();
> 160:                     fixedCellSizeEnabled = fixedCellSize > 0;
> 161:                 }

The `null` check on the property is not needed, properties never return `null`.  I would suggest just calling the getter: `t.getFixedCellSize()`.

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

Changes requested by jhendrikx (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1129#pullrequestreview-1423999964
PR Review Comment: https://git.openjdk.org/jfx/pull/1129#discussion_r1192054924
PR Review Comment: https://git.openjdk.org/jfx/pull/1129#discussion_r1192057917