RFR: 8295754: PaginationSkin: memory leak when changing skin

[Kevin Rushforth]

On Fri, 28 Oct 2022 20:08:33 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Fixes memory leaks as determined by SkinMemoryLeakTest (remove line 171) and a leak tester
>> https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java
>> 
>> Make sure to configure the current test in LeakTest:
>> protected final Type WE_ARE_TESTING = Type.PAGINATION;
>> 
>> Found another issue: Pagination class does not survive replacing its skin (all components disappear).
>> 
>> caused by:
>> - adding and not removing listeners
>> - adding and not removing children Nodes
>> - setting control's properties in the constructor
>> - incorrectly setting a clip rectangle
>> 
>> NOTE: the fix will requires both ListenerHelper [JDK-8294809](https://bugs.openjdk.org/browse/JDK-8294809) and Skin.install() [JDK-8290844](https://bugs.openjdk.org/browse/JDK-8290844) changes.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/PaginationSkin.java line 917:
> 
>> 915:             });
>> 916: 
>> 917:             ListenerHelper.get(PaginationSkin.this).addChangeListener(getSkinnable().currentPageIndexProperty(), (src, old, cur) -> {
> 
> so ugly...
> @kevinrushforth

Yes, I realize that by using an accessor rather than public API, getting the listener handler goes from:


   getListenerHelper();


to:


   ListenerHelper.get(this);


or, if in a nested class, to:


   ListenerHelper.get(PaginationSkin.this);


Since you are going to use it in multiple places, it would be less "ugly" to cache it, either as an instance field like this:


   ListenerHelper listenerHelper = ListenerHelper.get(this);


Or else as a local variable in the methods where you use it:


   var listenerHelper = ListenerHelper.get(PaginationSkin.this);


Then in all places where you use it, you would just reference the `listenerHelper` variable.

I'd probably lean towards the instance field.

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

PR: https://git.openjdk.org/jfx/pull/925