Skip to content

Commit

Permalink
8295175: SplitPaneSkin: memory leak when changing skin
Browse files Browse the repository at this point in the history 
Reviewed-by: aghaisas
  • Loading branch information
Andy Goryachev committed Dec 6, 2022
1 parent bb13920 commit 6f36e70
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 38 deletions.
83 changes: 46 additions & 37 deletions modules/javafx.controls/src/main/java/javafx/scene/control/skin/SplitPaneSkin.java
View file
Expand Up @@ -25,6 +25,11 @@

package javafx.scene.control.skin;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;

import javafx.beans.value.ChangeListener;
import javafx.beans.value.ObservableValue;
import javafx.collections.FXCollections;
Expand All @@ -42,10 +47,8 @@
import javafx.scene.input.MouseEvent;
import javafx.scene.layout.StackPane;
import javafx.scene.shape.Rectangle;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.ListIterator;

import com.sun.javafx.scene.control.ListenerHelper;

/**
* Default skin implementation for the {@link SplitPane} control.
Expand All @@ -63,6 +66,7 @@ public class SplitPaneSkin extends SkinBase<SplitPane> {

private ObservableList<Content> contentRegions;
private ObservableList<ContentDivider> contentDividers;
private ListenerHelper contentDividerListenerHelper;
private boolean horizontal;
/**
* Flag which is used to determine whether we need to request layout when a divider position changed or not.
Expand All @@ -89,7 +93,7 @@ public class SplitPaneSkin extends SkinBase<SplitPane> {
*/
public SplitPaneSkin(final SplitPane control) {
super(control);
// control.setManaged(false);

horizontal = getSkinnable().getOrientation() == Orientation.HORIZONTAL;

contentRegions = FXCollections.<Content>observableArrayList();
Expand All @@ -101,20 +105,19 @@ public SplitPaneSkin(final SplitPane control) {
}
initializeContentListener();

for (SplitPane.Divider d: getSkinnable().getDividers()) {
addDivider(d);
}
addDividers();

registerChangeListener(control.orientationProperty(), e -> {
ListenerHelper lh = ListenerHelper.get(this);
lh.addChangeListener(control.orientationProperty(), (v) -> {
this.horizontal = getSkinnable().getOrientation() == Orientation.HORIZONTAL;
this.previousSize = -1;
for (ContentDivider c: contentDividers) {
c.setGrabberStyle(horizontal);
}
getSkinnable().requestLayout();
});
registerChangeListener(control.widthProperty(), e -> getSkinnable().requestLayout());
registerChangeListener(control.heightProperty(), e -> getSkinnable().requestLayout());
lh.addChangeListener(control.widthProperty(), (v) -> getSkinnable().requestLayout());
lh.addChangeListener(control.heightProperty(), (v) -> getSkinnable().requestLayout());
}


Expand All @@ -125,6 +128,13 @@ public SplitPaneSkin(final SplitPane control) {
* *
**************************************************************************/

@Override
public void dispose() {
removeAllDividers();

super.dispose();
}

/** {@inheritDoc} */
@Override protected void layoutChildren(final double x, final double y,
final double w, final double h) {
Expand Down Expand Up @@ -520,7 +530,7 @@ private void removeContent(Node n) {
}

private void initializeContentListener() {
getSkinnable().getItems().addListener((ListChangeListener<Node>) c -> {
ListenerHelper.get(this).addListChangeListener(getSkinnable().getItems(), (ListChangeListener<Node>) c -> {
while (c.next()) {
if (c.wasPermutated() || c.wasUpdated()) {
/**
Expand All @@ -545,12 +555,9 @@ private void initializeContentListener() {
}
}
}
// TODO there may be a more efficient way than rebuilding all the dividers
// everytime the list changes.

removeAllDividers();
for (SplitPane.Divider d: getSkinnable().getDividers()) {
addDivider(d);
}
addDividers();
});
}

Expand Down Expand Up @@ -618,27 +625,38 @@ private void checkDividerPosition(ContentDivider divider, double newPos, double
checkDividerPos = true;
}

private void addDivider(SplitPane.Divider d) {
ContentDivider c = new ContentDivider(d);
c.setInitialPos(d.getPosition());
c.setDividerPos(-1);
ChangeListener<Number> posPropertyListener = new PosPropertyListener(c);
c.setPosPropertyListener(posPropertyListener);
d.positionProperty().addListener(posPropertyListener);
initializeDividerEventHandlers(c);
contentDividers.add(c);
getChildren().add(c);
private void addDividers() {
contentDividerListenerHelper = new ListenerHelper();

for (SplitPane.Divider d : getSkinnable().getDividers()) {
ContentDivider c = new ContentDivider(d);
c.setInitialPos(d.getPosition());
c.setDividerPos(-1);

ChangeListener<Number> li = new PosPropertyListener(c);
contentDividerListenerHelper.addChangeListener(d.positionProperty(), li);

initializeDividerEventHandlers(c);

contentDividers.add(c);
getChildren().add(c);
}
}

private void removeAllDividers() {
ListIterator<ContentDivider> dividers = contentDividers.listIterator();
while (dividers.hasNext()) {
ContentDivider c = dividers.next();
getChildren().remove(c);
c.getDivider().positionProperty().removeListener(c.getPosPropertyListener());
dividers.remove();
}

lastDividerUpdate = 0;

if (contentDividerListenerHelper != null) {
contentDividerListenerHelper.disconnect();
contentDividerListenerHelper = null;
}
}

private void initializeDividerEventHandlers(final ContentDivider divider) {
Expand Down Expand Up @@ -978,7 +996,6 @@ class ContentDivider extends StackPane {
private double x;
private double y;
private boolean posExplicit;
private ChangeListener<Number> listener;

public ContentDivider(SplitPane.Divider d) {
getStyleClass().setAll("split-pane-divider");
Expand Down Expand Up @@ -1074,14 +1091,6 @@ public void setY(double y) {
this.y = y;
}

public ChangeListener<Number> getPosPropertyListener() {
return listener;
}

public void setPosPropertyListener(ChangeListener<Number> listener) {
this.listener = listener;
}

@Override protected double computeMinWidth(double height) {
return computePrefWidth(height);
}
Expand Down
2 changes: 1 addition & 1 deletion modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java
View file
Expand Up @@ -238,7 +238,7 @@ public static Collection<Object[]> data() {
Spinner.class,

//
SplitPane.class,
//SplitPane.class,

//
//TableView.class,
Expand Down

0 comments on commit 6f36e70

Please sign in to comment.