Skip to content

Commit fbad15d

Browse files
Andy Goryachevkevinrushforth
Andy Goryachev
authored and
kevinrushforth
committed
8295796: ScrollPaneSkin: memory leak when changing skin
8295242: ScrollBarSkin: memory leak when changing skin Reviewed-by: aghaisas
1 parent 4ad8582 commit fbad15d

File tree

3 files changed

+81
-84
lines changed

3 files changed

+81
-84
lines changed

Diff for: ‎modules/javafx.controls/src/main/java/javafx/scene/control/skin/ScrollBarSkin.java

+19-21
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2010, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -25,28 +25,24 @@
2525

2626
package javafx.scene.control.skin;
2727

28-
import com.sun.javafx.scene.control.Properties;
29-
import com.sun.javafx.scene.control.behavior.BehaviorBase;
30-
import javafx.beans.value.ObservableValue;
3128
import javafx.geometry.Orientation;
3229
import javafx.geometry.Point2D;
3330
import javafx.scene.AccessibleAction;
3431
import javafx.scene.AccessibleAttribute;
3532
import javafx.scene.AccessibleRole;
36-
import javafx.scene.control.Accordion;
37-
import javafx.scene.control.Button;
33+
import javafx.scene.Node;
3834
import javafx.scene.control.Control;
3935
import javafx.scene.control.ScrollBar;
4036
import javafx.scene.control.SkinBase;
4137
import javafx.scene.input.MouseButton;
4238
import javafx.scene.input.ScrollEvent;
4339
import javafx.scene.layout.Region;
4440
import javafx.scene.layout.StackPane;
45-
import javafx.scene.Node;
46-
import com.sun.javafx.util.Utils;
47-
import com.sun.javafx.scene.control.behavior.ScrollBarBehavior;
4841

49-
import java.util.function.Consumer;
42+
import com.sun.javafx.scene.control.ListenerHelper;
43+
import com.sun.javafx.scene.control.Properties;
44+
import com.sun.javafx.scene.control.behavior.ScrollBarBehavior;
45+
import com.sun.javafx.util.Utils;
5046

5147
/**
5248
* Default skin implementation for the {@link ScrollBar} control.
@@ -98,21 +94,24 @@ public ScrollBarSkin(ScrollBar control) {
9894

9995
// install default input map for the ScrollBar control
10096
this.behavior = new ScrollBarBehavior(control);
101-
// control.setInputMap(behavior.getInputMap());
10297

10398
initialize();
10499
getSkinnable().requestLayout();
105100

106-
// Register listeners
107-
final Consumer<ObservableValue<?>> consumer = e -> {
101+
ListenerHelper lh = ListenerHelper.get(this);
102+
103+
lh.addChangeListener(() -> {
108104
positionThumb();
109105
getSkinnable().requestLayout();
110-
};
111-
registerChangeListener(control.minProperty(), consumer);
112-
registerChangeListener(control.maxProperty(), consumer);
113-
registerChangeListener(control.visibleAmountProperty(), consumer);
114-
registerChangeListener(control.valueProperty(), e -> positionThumb());
115-
registerChangeListener(control.orientationProperty(), e -> getSkinnable().requestLayout());
106+
}, control.minProperty(), control.maxProperty(), control.visibleAmountProperty());
107+
108+
lh.addChangeListener(control.valueProperty(), (ev) -> {
109+
positionThumb();
110+
});
111+
112+
lh.addChangeListener(control.orientationProperty(), (ev) -> {
113+
getSkinnable().requestLayout();
114+
});
116115
}
117116

118117

@@ -510,8 +509,7 @@ public void executeAccessibleAction(AccessibleAction action, Object... parameter
510509
}
511510
});
512511

513-
514-
getSkinnable().addEventHandler(ScrollEvent.SCROLL, event -> {
512+
ListenerHelper.get(this).addEventHandler(getSkinnable(), ScrollEvent.SCROLL, event -> {
515513
/*
516514
** if the tracklength isn't greater then do nothing....
517515
*/

Diff for: ‎modules/javafx.controls/src/main/java/javafx/scene/control/skin/ScrollPaneSkin.java

+60-61
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,8 @@
2525

2626
package javafx.scene.control.skin;
2727

28-
import com.sun.javafx.scene.NodeHelper;
29-
import com.sun.javafx.scene.ParentHelper;
30-
import com.sun.javafx.scene.control.Properties;
31-
import com.sun.javafx.scene.control.behavior.BehaviorBase;
32-
import com.sun.javafx.scene.traversal.ParentTraversalEngine;
28+
import static com.sun.javafx.scene.control.skin.Utils.boundedSize;
29+
3330
import javafx.animation.Animation.Status;
3431
import javafx.animation.KeyFrame;
3532
import javafx.animation.KeyValue;
@@ -46,11 +43,11 @@
4643
import javafx.event.EventHandler;
4744
import javafx.geometry.BoundingBox;
4845
import javafx.geometry.Bounds;
46+
import javafx.geometry.Insets;
4947
import javafx.geometry.Orientation;
5048
import javafx.scene.AccessibleAttribute;
5149
import javafx.scene.Cursor;
5250
import javafx.scene.Node;
53-
import javafx.scene.control.Button;
5451
import javafx.scene.control.Control;
5552
import javafx.scene.control.ScrollBar;
5653
import javafx.scene.control.ScrollPane;
@@ -62,12 +59,15 @@
6259
import javafx.scene.layout.StackPane;
6360
import javafx.scene.shape.Rectangle;
6461
import javafx.util.Duration;
65-
import com.sun.javafx.util.Utils;
66-
import com.sun.javafx.scene.control.behavior.ScrollPaneBehavior;
67-
import static com.sun.javafx.scene.control.skin.Utils.*;
68-
import javafx.geometry.Insets;
6962

70-
import java.util.function.Consumer;
63+
import com.sun.javafx.scene.NodeHelper;
64+
import com.sun.javafx.scene.ParentHelper;
65+
import com.sun.javafx.scene.control.ListenerHelper;
66+
import com.sun.javafx.scene.control.Properties;
67+
import com.sun.javafx.scene.control.behavior.BehaviorBase;
68+
import com.sun.javafx.scene.control.behavior.ScrollPaneBehavior;
69+
import com.sun.javafx.scene.traversal.ParentTraversalEngine;
70+
import com.sun.javafx.util.Utils;
7171

7272
/**
7373
* Default skin implementation for the {@link ScrollPane} control.
@@ -265,16 +265,13 @@ public ScrollPaneSkin(final ScrollPane control) {
265265

266266
// install default input map for the ScrollPane control
267267
behavior = new ScrollPaneBehavior(control);
268-
// control.setInputMap(behavior.getInputMap());
269268

270269
initialize();
271270

272271
// Register listeners
273-
Consumer<ObservableValue<?>> viewportSizeHintConsumer = e -> {
274-
// change affects pref size, so requestLayout on control
275-
getSkinnable().requestLayout();
276-
};
277-
registerChangeListener(control.contentProperty(), e -> {
272+
ListenerHelper lh = ListenerHelper.get(this);
273+
274+
lh.addChangeListener(control.contentProperty(), (ev) -> {
278275
if (scrollNode != getSkinnable().getContent()) {
279276
if (scrollNode != null) {
280277
scrollNode.layoutBoundsProperty().removeListener(weakNodeListener);
@@ -292,32 +289,35 @@ public ScrollPaneSkin(final ScrollPane control) {
292289
}
293290
getSkinnable().requestLayout();
294291
});
295-
registerChangeListener(control.fitToWidthProperty(), e -> {
296-
getSkinnable().requestLayout();
297-
viewRect.requestLayout();
298-
});
299-
registerChangeListener(control.fitToHeightProperty(), e -> {
300-
getSkinnable().requestLayout();
301-
viewRect.requestLayout();
302-
});
303-
registerChangeListener(control.hbarPolicyProperty(), e -> {
304-
// change might affect pref size, so requestLayout on control
305-
getSkinnable().requestLayout();
306-
});
307-
registerChangeListener(control.vbarPolicyProperty(), e -> {
308-
// change might affect pref size, so requestLayout on control
309-
getSkinnable().requestLayout();
310-
});
311-
registerChangeListener(control.hvalueProperty(), e -> hsb.setValue(getSkinnable().getHvalue()));
312-
registerChangeListener(control.hmaxProperty(), e -> hsb.setMax(getSkinnable().getHmax()));
313-
registerChangeListener(control.hminProperty(), e -> hsb.setMin(getSkinnable().getHmin()));
314-
registerChangeListener(control.vvalueProperty(), e -> vsb.setValue(getSkinnable().getVvalue()));
315-
registerChangeListener(control.vmaxProperty(), e -> vsb.setMax(getSkinnable().getVmax()));
316-
registerChangeListener(control.vminProperty(), e -> vsb.setMin(getSkinnable().getVmin()));
317-
registerChangeListener(control.prefViewportWidthProperty(), viewportSizeHintConsumer);
318-
registerChangeListener(control.prefViewportHeightProperty(), viewportSizeHintConsumer);
319-
registerChangeListener(control.minViewportWidthProperty(), viewportSizeHintConsumer);
320-
registerChangeListener(control.minViewportHeightProperty(), viewportSizeHintConsumer);
292+
293+
lh.addChangeListener(
294+
() -> {
295+
getSkinnable().requestLayout();
296+
viewRect.requestLayout();
297+
},
298+
control.fitToWidthProperty(),
299+
control.fitToHeightProperty()
300+
);
301+
302+
lh.addChangeListener(control.hvalueProperty(), e -> hsb.setValue(getSkinnable().getHvalue()));
303+
lh.addChangeListener(control.hmaxProperty(), e -> hsb.setMax(getSkinnable().getHmax()));
304+
lh.addChangeListener(control.hminProperty(), e -> hsb.setMin(getSkinnable().getHmin()));
305+
lh.addChangeListener(control.vvalueProperty(), e -> vsb.setValue(getSkinnable().getVvalue()));
306+
lh.addChangeListener(control.vmaxProperty(), e -> vsb.setMax(getSkinnable().getVmax()));
307+
lh.addChangeListener(control.vminProperty(), e -> vsb.setMin(getSkinnable().getVmin()));
308+
309+
lh.addChangeListener(
310+
() -> {
311+
// change affects pref size, so requestLayout on control
312+
getSkinnable().requestLayout();
313+
},
314+
control.hbarPolicyProperty(),
315+
control.vbarPolicyProperty(),
316+
control.prefViewportWidthProperty(),
317+
control.prefViewportHeightProperty(),
318+
control.minViewportWidthProperty(),
319+
control.minViewportHeightProperty()
320+
);
321321
}
322322

323323

@@ -387,12 +387,13 @@ public String getName() {
387387
**************************************************************************/
388388

389389
/** {@inheritDoc} */
390-
@Override public void dispose() {
391-
super.dispose();
392-
390+
@Override
391+
public void dispose() {
393392
if (behavior != null) {
394393
behavior.dispose();
395394
}
395+
396+
super.dispose();
396397
}
397398

398399
/**
@@ -666,8 +667,10 @@ private void initialize() {
666667
}
667668
};
668669

669-
hsb.addEventFilter(MouseEvent.MOUSE_PRESSED, barHandler);
670-
vsb.addEventFilter(MouseEvent.MOUSE_PRESSED, barHandler);
670+
ListenerHelper lh = ListenerHelper.get(this);
671+
672+
lh.addEventFilter(hsb, MouseEvent.MOUSE_PRESSED, barHandler);
673+
lh.addEventFilter(vsb, MouseEvent.MOUSE_PRESSED, barHandler);
671674

672675
corner = new StackPane();
673676
corner.getStyleClass().setAll("corner");
@@ -708,30 +711,27 @@ private void initialize() {
708711
getChildren().clear();
709712
getChildren().addAll(viewRect, vsb, hsb, corner);
710713

711-
/*
712-
** listeners, and assorted housekeeping
713-
*/
714-
InvalidationListener vsbListener = valueModel -> {
714+
// listeners, and assorted housekeeping
715+
716+
lh.addInvalidationListener(vsb.valueProperty(), (valueModel) -> {
715717
if (!Properties.IS_TOUCH_SUPPORTED) {
716718
posY = Utils.clamp(getSkinnable().getVmin(), vsb.getValue(), getSkinnable().getVmax());
717719
}
718720
else {
719721
posY = vsb.getValue();
720722
}
721723
updatePosY();
722-
};
723-
vsb.valueProperty().addListener(vsbListener);
724+
});
724725

725-
InvalidationListener hsbListener = valueModel -> {
726+
lh.addInvalidationListener(hsb.valueProperty(), (valueModel) -> {
726727
if (!Properties.IS_TOUCH_SUPPORTED) {
727728
posX = Utils.clamp(getSkinnable().getHmin(), hsb.getValue(), getSkinnable().getHmax());
728729
}
729730
else {
730731
posX = hsb.getValue();
731732
}
732733
updatePosX();
733-
};
734-
hsb.valueProperty().addListener(hsbListener);
734+
});
735735

736736
viewRect.setOnMousePressed(e -> {
737737
mouseDown = true;
@@ -744,7 +744,6 @@ private void initialize() {
744744
ovvalue = vsb.getValue();
745745
});
746746

747-
748747
viewRect.setOnDragDetected(e -> {
749748
if (Properties.IS_TOUCH_SUPPORTED) {
750749
startSBReleasedAnimation();
@@ -782,6 +781,7 @@ posX > getSkinnable().getHmax() || posX < getSkinnable().getHmin()) && !touchDet
782781
startContentsToViewport();
783782
}
784783
});
784+
785785
viewRect.setOnMouseDragged(e -> {
786786
if (Properties.IS_TOUCH_SUPPORTED) {
787787
startSBReleasedAnimation();
@@ -843,7 +843,6 @@ else if (newVVal < vsb.getMin()) {
843843
e.consume();
844844
});
845845

846-
847846
/*
848847
** don't allow the ScrollBar to handle the ScrollEvent,
849848
** In a ScrollPane a vertical scroll should scroll on the vertical only,
@@ -953,13 +952,13 @@ else if (newVVal < vsb.getMin()) {
953952
** there are certain animations that need to know if the touch is
954953
** happening.....
955954
*/
956-
getSkinnable().addEventHandler(TouchEvent.TOUCH_PRESSED, e -> {
955+
lh.addEventHandler(getSkinnable(), TouchEvent.TOUCH_PRESSED, e -> {
957956
touchDetected = true;
958957
startSBReleasedAnimation();
959958
e.consume();
960959
});
961960

962-
getSkinnable().addEventHandler(TouchEvent.TOUCH_RELEASED, e -> {
961+
lh.addEventHandler(getSkinnable(), TouchEvent.TOUCH_RELEASED, e -> {
963962
touchDetected = false;
964963
e.consume();
965964
});

Diff for: ‎modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinMemoryLeakTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,8 @@ public static Collection<Object[]> data() {
170170
MenuButton.class,
171171
Pagination.class,
172172
PasswordField.class,
173-
ScrollBar.class,
174-
ScrollPane.class,
173+
//ScrollBar.class,
174+
//ScrollPane.class,
175175
// @Ignore("8245145")
176176
Spinner.class,
177177
SplitMenuButton.class,

0 commit comments

Comments
 (0)