Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8245282: Button/Combo Behavior: memory leak on dispose #226

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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;
Expand Down Expand Up @@ -56,6 +58,7 @@ public class ButtonBehavior<C extends ButtonBase> extends BehaviorBase<C> {
*/
private boolean keyDown;

private InvalidationListener focusListener = this::focusChanged;


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

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


Expand All @@ -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);
}


Expand Down
Expand Up @@ -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;
Expand All @@ -47,6 +49,7 @@
public class ComboBoxBaseBehavior<T> extends BehaviorBase<ComboBoxBase<T>> {

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

/***************************************************************************
* *
Expand Down Expand Up @@ -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()) {
Expand All @@ -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();
}

Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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
);
Expand Down