On Tue, 13 Oct 2020 12:56:10 GMT, Ambarish Rapte <[email protected]> wrote:
> `TabPaneSkin` installs some listeners that are not removed when `TabPaneSkin`
> is changed.
> The fix converts listeners to WeakListeners and also removes them on dispose.
>
> There is a NPE check change needed in `isHosrizontal()`. Without this check
> it causes NPE if pulse is in progress while
> TabPaneSkin is getting disposed.
> `SkinMemoryLeakTest` already had a test which only needed to be enabled.
> Test fails before and passes after this change.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java
line 274:
> 272: getSkinnable().getTabs().removeListener(weakTabsListener);
> 273:
> 274: getChildren().remove(tabHeaderArea);
As mentioned offline, can you check whether removing `tabHeaderArea` is
necessary?
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java
line 716:
> 714:
> 715: private boolean isHorizontal() {
> 716: Side tabPosition = getSkinnable() != null ?
> getSkinnable().getSide() : Side.TOP;
I agree with @kleopatra This null check suggests that someone (a listener
perhaps?) is calling into the skin after it
has been disposed.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java
line 1226:
> 1224:
> 1225: void removeListeners() {
> 1226: for(Node child : headersRegion.getChildren()) {
Minor: space after the `for`
-------------
PR: https://git.openjdk.java.net/jfx/pull/318