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

[Kevin Rushforth]

On Mon, 8 May 2023 18:55:38 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Fixed a memory leak in TreeTableView by using WeakInvalidationListener (which is the right approach in this particular situation) - the leak is specific to TreeTableRowSkin.
>> 
>> Added a unit test.
>> 
>> Added Refresh buttons and Skin menu to the Monkey Tester (will expand these to other controls as a part next MT update).
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision:
> 
>  - removed monkey tester changes
>  - Merge remote-tracking branch 'origin/master' into 8307538.refresh
>  - whitespace
>  - updated test
>  - variable tree cell fix
>  - fixed size selector
>  - list view page
>  - fix and test
>  - skin menu
>  - refresh button

I left a couple inline comments.

I also wonder whether the `index` and `treeItem` properties need to be weak (I don't think they do).

The failing test is something that will need to be tracked down. If I revert your changes to `treeTableRowWithFixedCellSizeShouldIgnoreVerticalPadding`, the test passes when that class is run by itself, but fails when the entire controls test suite is run. The modified test fails with that class is run by itself, but passes when the entire controls test suite is run.

This suggests a possible problem where the (now weak) listeners are getting GCed sometimes but not others, and that whether or not it is GCed causes differences that are visible to the test. So either the test is relying on an implementation detail, or there is a functional problem caused by the listeners being GCed too early or not early enough (the latter could point to the need to remove them in dispose).

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

PR Review: https://git.openjdk.org/jfx/pull/1129#pullrequestreview-1420989135