Skip to content
Permalink
Browse files

8245282: Button/Combo Behavior: memory leak on dispose

Reviewed-by: arapte
  • Loading branch information
Jeanette Winzenburg authored and arapte committed Jun 2, 2020
1 parent a78b3fb commit 853ac78ad5099ebda00dc401b3ead67cafa59aa8
@@ -26,6 +26,8 @@

import com.sun.javafx.PlatformUtil;
import com.sun.javafx.scene.control.inputmap.KeyBinding;

import javafx.beans.InvalidationListener;
import javafx.beans.Observable;
import javafx.scene.control.ButtonBase;
import com.sun.javafx.scene.control.inputmap.InputMap;
@@ -56,6 +58,7 @@
*/
private boolean keyDown;

private InvalidationListener focusListener = this::focusChanged;


/***************************************************************************
@@ -89,7 +92,7 @@ public ButtonBehavior(C control) {
);

// Button also cares about focus
control.focusedProperty().addListener(this::focusChanged);
control.focusedProperty().addListener(focusListener);
}


@@ -105,10 +108,9 @@ public ButtonBehavior(C control) {
}

@Override public void dispose() {
// TODO specify contract of dispose and post-condition for getNode()
getNode().focusedProperty().removeListener(focusListener);
super.dispose();

// TODO
getNode().focusedProperty().removeListener(this::focusChanged);
}


@@ -26,6 +26,8 @@
package com.sun.javafx.scene.control.behavior;

import com.sun.javafx.scene.control.inputmap.InputMap;

import javafx.beans.InvalidationListener;
import javafx.beans.Observable;
import javafx.event.EventHandler;
import javafx.event.EventTarget;
@@ -47,6 +49,7 @@
public class ComboBoxBaseBehavior<T> extends BehaviorBase<ComboBoxBase<T>> {

private final InputMap<ComboBoxBase<T>> inputMap;
private InvalidationListener focusListener = this::focusChanged;

/***************************************************************************
* *
@@ -102,7 +105,7 @@ public ComboBoxBaseBehavior(final ComboBoxBase<T> comboBox) {
enterReleased.setAutoConsume(false);

// ComboBoxBase also cares about focus
comboBox.focusedProperty().addListener(this::focusChanged);
comboBox.focusedProperty().addListener(focusListener);

// Only add this if we're on an embedded platform that supports 5-button navigation
if (Utils.isTwoLevelFocus()) {
@@ -112,7 +115,7 @@ public ComboBoxBaseBehavior(final ComboBoxBase<T> comboBox) {

@Override public void dispose() {
if (tlFocus != null) tlFocus.dispose();
getNode().focusedProperty().removeListener(this::focusChanged);
getNode().focusedProperty().removeListener(focusListener);
super.dispose();
}

@@ -41,22 +41,12 @@
import static org.junit.Assert.*;
import static test.com.sun.javafx.scene.control.infrastructure.ControlSkinFactory.*;

import javafx.scene.control.Button;
import javafx.scene.control.CheckBox;
import javafx.scene.control.ColorPicker;
import javafx.scene.control.ComboBox;
import javafx.scene.control.Control;
import javafx.scene.control.DatePicker;
import javafx.scene.control.Hyperlink;
import javafx.scene.control.ListView;
import javafx.scene.control.MenuButton;
import javafx.scene.control.PasswordField;
import javafx.scene.control.RadioButton;
import javafx.scene.control.SplitMenuButton;
import javafx.scene.control.TableView;
import javafx.scene.control.TextArea;
import javafx.scene.control.TextField;
import javafx.scene.control.ToggleButton;
import javafx.scene.control.TreeTableView;
import javafx.scene.control.TreeView;
import test.com.sun.javafx.scene.control.infrastructure.ControlSkinFactory;
@@ -86,39 +76,19 @@ public void testMemoryLeakDisposeBehavior() {
//---------------- parameterized

// Note: name property not supported before junit 4.11
@Parameterized.Parameters //(name = "{index}: {0} ")
@Parameterized.Parameters // (name = "{index}: {0} ")
public static Collection<Object[]> data() {
List<Class<Control>> controlClasses = getControlClassesWithBehavior();
// FIXME as part of JDK-8241364
// The behaviors of these controls are leaking
// step 1: file issues (where not yet done), add informal ignore to entry
// step 2: fix and remove from list
List<Class<? extends Control>> leakingClasses = List.of(
// @Ignore("8245282")
Button.class,
// @Ignore("8245282")
CheckBox.class,
// @Ignore("8245282")
ColorPicker.class,
// @Ignore("8245282")
ComboBox.class,
// @Ignore("8245282")
DatePicker.class,
// @Ignore("8245282")
Hyperlink.class,
ListView.class,
// @Ignore("8245282")
MenuButton.class,
PasswordField.class,
// @Ignore("8245282")
RadioButton.class,
// @Ignore("8245282")
SplitMenuButton.class,
TableView.class,
TextArea.class,
TextField.class,
// @Ignore("8245282")
ToggleButton.class,
TreeTableView.class,
TreeView.class
);

0 comments on commit 853ac78

Please sign in to comment.