RFR: 8242621: TabPane: Memory leak when switching skin

[Kevin Rushforth]

On Tue, 13 Oct 2020 12:56:10 GMT, Ambarish Rapte <arapte at openjdk.org> 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