On Wed, 23 Nov 2022 00:13:07 GMT, Kevin Rushforth <k...@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 209: > >> 207: }); >> 208: >> 209: >> ListenerHelper.get(this).addChangeListener(getSkinnable().heightProperty(), >> true, (ev) -> { > > Why does `fireImmediately` need to be true? this was needed to fix an issue found in PaginationSkin: Pagination class does not survive replacing its skin (all components disappear). > modules/javafx.controls/src/main/java/javafx/scene/control/skin/PaginationSkin.java > line 1361: > >> 1359: // FIX own listener helper? >> 1360: >> //getSkinnable().getStyleClass().removeListener(updateSkinIndicatorType); >> 1361: >> //tooltipVisibleProperty().removeListener(updateTooltipVisibility); > > I don't quite understand the comment. What follow-up is needed? this method can be removed now. > modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java > line 171: > >> 169: MenuBar.class, >> 170: MenuButton.class, >> 171: //Pagination.class, > > I would just remove this (which was done for other classes once the leaks > were fixed). Keeping this to avoid merge conflicts. Will remove the whole exclusion block when the last skin is fixed. ------------- PR: https://git.openjdk.org/jfx/pull/925
