RFR: 8295809: TreeTableViewSkin: memory leak when changing skin [v2]

[Ajit Ghaisas]

On Tue, 6 Dec 2022 17:29:40 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> as determined by SkinMemoryLeakTest (remove line 180) and a leak tester
>> https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java
>> 
>> caused by:
>> - adding and not removing listeners
>> - adding and not removing event handlers/filters
>> - adding and not removing cell factory
>> - holding unnecessary instance in TreeTableViewSkin.treeTableViewSkin // test-only
>> 
>> NOTES:
>> 1. this fix requires JDK-8294809 ListenerHelper and JDK-8295806 TableViewSkin.
>> 2. there were more memory leaks in this skin than detected by SkinMemoryLeakTest, probably because of cell factory (?).  A manual test using LeakTest shows that there are no memory leaks after a) replacing skin, b) moving the components to a new window, and c) removing all the components from the window.
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 36 commits:
> 
>  - 8295809: cleanup
>  - Merge remote-tracking branch 'origin/master' into
>    8295809.tree.table.view.skin
>  - Merge remote-tracking branch 'origin/master' into 8295809.tree.table.view.skin
>  - Merge branch '8295806.table.view.skin' into 8295809.tree.table.view.skin
>  - Merge remote-tracking branch 'origin/master' into
>    8295806.table.view.skin
>  - Merge branch '8295806.table.view.skin' into 8295809.tree.table.view.skin
>  - Merge remote-tracking branch 'origin/master' into
>    8295806.table.view.skin
>  - 8295809: cleanup
>  - Merge branch '8295806.table.view.skin' into 8295809.tree.table.view.skin
>  - Merge remote-tracking branch 'origin/master' into
>    8295806.table.view.skin
>  - ... and 26 more: https://git.openjdk.org/jfx/compare/6f36e704...cfa05b79

Marked as reviewed by aghaisas (Reviewer).

I have identified the minor changes required.

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java line 277:

> 275:     }
> 276: 
> 277:     @Ignore("JDK-8295809") // TODO probably need to verify the result of listener action,

We need to file a follow-on bug to modify or cleanup these ignored tests in future.
Use the bug ID of the newly filed bug with `@Ignore` tag for these two tests.

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java line 235:

> 233: 
> 234:                 //
> 235:                 //PasswordField.class,

This seems an unrelated change.

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java line 238:

> 236: 
> 237:                 //
> 238:                 Spinner.class

This seems an unrelated change.

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

PR: https://git.openjdk.org/jfx/pull/931Changes requested by aghaisas (Reviewer).