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

8241455: Memory leak on replacing selection/focusModel #148

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -29,9 +29,11 @@
import javafx.beans.property.ObjectPropertyBase;
import javafx.beans.property.SimpleObjectProperty;
import javafx.beans.value.ChangeListener;
import javafx.beans.value.WeakChangeListener;
import javafx.collections.FXCollections;
import javafx.collections.ListChangeListener;
import javafx.collections.ObservableList;
import javafx.collections.WeakListChangeListener;
import javafx.beans.property.ReadOnlyBooleanProperty;
import javafx.beans.property.ReadOnlyBooleanWrapper;
import javafx.event.ActionEvent;
@@ -503,6 +505,9 @@ public void hide() {
// package for testing
static class ChoiceBoxSelectionModel<T> extends SingleSelectionModel<T> {
private final ChoiceBox<T> choiceBox;
private ChangeListener<ObservableList<T>> itemsObserver;
private ListChangeListener<T> itemsContentObserver;
private WeakListChangeListener<T> weakItemsContentObserver;

public ChoiceBoxSelectionModel(final ChoiceBox<T> cb) {
if (cb == null) {
@@ -520,7 +525,7 @@ public ChoiceBoxSelectionModel(final ChoiceBox<T> cb) {
*/

// watching for changes to the items list content
final ListChangeListener<T> itemsContentObserver = c -> {
itemsContentObserver = c -> {
if (choiceBox.getItems() == null || choiceBox.getItems().isEmpty()) {
setSelectedIndex(-1);
} else if (getSelectedIndex() == -1 && getSelectedItem() != null) {
@@ -530,17 +535,18 @@ public ChoiceBoxSelectionModel(final ChoiceBox<T> cb) {
}
}
};
weakItemsContentObserver = new WeakListChangeListener<>(itemsContentObserver);
if (this.choiceBox.getItems() != null) {
this.choiceBox.getItems().addListener(itemsContentObserver);
this.choiceBox.getItems().addListener(weakItemsContentObserver);
}

// watching for changes to the items list
ChangeListener<ObservableList<T>> itemsObserver = (valueModel, oldList, newList) -> {
itemsObserver = (valueModel, oldList, newList) -> {
if (oldList != null) {
oldList.removeListener(itemsContentObserver);
oldList.removeListener(weakItemsContentObserver);
}
if (newList != null) {
newList.addListener(itemsContentObserver);
newList.addListener(weakItemsContentObserver);
}
setSelectedIndex(-1);
if (getSelectedItem() != null) {
@@ -550,7 +556,9 @@ public ChoiceBoxSelectionModel(final ChoiceBox<T> cb) {
}
}
};
this.choiceBox.itemsProperty().addListener(itemsObserver);
// TBD: use pattern as f.i. in listView selectionModel (invalidationListener to really
// get all changes - including list of same content - of the list-valued property)
this.choiceBox.itemsProperty().addListener(new WeakChangeListener<>(itemsObserver));
}

// API Implementation
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -41,6 +41,7 @@
import javafx.beans.value.WritableValue;
import javafx.collections.ListChangeListener;
import javafx.collections.ObservableList;
import javafx.collections.WeakListChangeListener;
import javafx.geometry.Side;
import javafx.scene.AccessibleAttribute;
import javafx.scene.AccessibleRole;
@@ -672,14 +673,16 @@ public boolean isSettable(TabPane n) {
static class TabPaneSelectionModel extends SingleSelectionModel<Tab> {
private final TabPane tabPane;

private ListChangeListener<Tab> itemsContentObserver;

public TabPaneSelectionModel(final TabPane t) {
if (t == null) {
throw new NullPointerException("TabPane can not be null");
}
this.tabPane = t;

// watching for changes to the items list content
final ListChangeListener<Tab> itemsContentObserver = c -> {
itemsContentObserver = c -> {
while (c.next()) {
for (Tab tab : c.getRemoved()) {
if (tab != null && !tabPane.getTabs().contains(tab)) {
@@ -710,7 +713,7 @@ public TabPaneSelectionModel(final TabPane t) {
}
};
if (this.tabPane.getTabs() != null) {
this.tabPane.getTabs().addListener(itemsContentObserver);
this.tabPane.getTabs().addListener(new WeakListChangeListener<>(itemsContentObserver));
}
}

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -3406,12 +3406,13 @@ public TreeTableViewFocusModel(final TreeTableView<S> treeTableView) {
TreeTablePosition<S,?> pos = new TreeTablePosition<>(treeTableView, focusRow, null);
setFocusedCell(pos);

treeTableView.showRootProperty().addListener(o -> {
showRootListener = obs -> {
if (isFocused(0)) {
focus(-1);
focus(0);
}
});
};
treeTableView.showRootProperty().addListener(new WeakInvalidationListener(showRootListener));

focusedCellProperty().addListener(o -> {
treeTableView.notifyAccessibleAttributeChanged(AccessibleAttribute.FOCUS_ITEM);
@@ -3425,6 +3426,8 @@ public TreeTableViewFocusModel(final TreeTableView<S> treeTableView) {
private final WeakChangeListener<TreeItem<S>> weakRootPropertyListener =
new WeakChangeListener<>(rootPropertyListener);

private final InvalidationListener showRootListener;

private void updateTreeEventListener(TreeItem<S> oldRoot, TreeItem<S> newRoot) {
if (oldRoot != null && weakTreeItemListener != null) {
oldRoot.removeEventHandler(TreeItem.<S>expandedItemCountChangeEvent(), weakTreeItemListener);
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2008, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -32,6 +32,8 @@

import javafx.application.Platform;
import javafx.beans.DefaultProperty;
import javafx.beans.InvalidationListener;
import javafx.beans.WeakInvalidationListener;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.DoubleProperty;
import javafx.beans.property.ObjectProperty;
@@ -1295,9 +1297,10 @@ public TreeViewBitSetSelectionModel(final TreeView<T> treeView) {

this.treeView = treeView;
this.treeView.rootProperty().addListener(weakRootPropertyListener);
this.treeView.showRootProperty().addListener(o -> {
showRootListener = o -> {
shiftSelection(0, treeView.isShowRoot() ? 1 : -1, null);
});
};
this.treeView.showRootProperty().addListener(new WeakInvalidationListener(showRootListener));

updateTreeEventListener(null, treeView.getRoot());

@@ -1310,6 +1313,7 @@ private void updateTreeEventListener(TreeItem<T> oldRoot, TreeItem<T> newRoot) {
}

if (newRoot != null) {
//PENDING why create a new weak eventHandler?
weakTreeItemListener = new WeakEventHandler<>(treeItemListener);
newRoot.addEventHandler(TreeItem.<T>expandedItemCountChangeEvent(), weakTreeItemListener);
}
@@ -1456,6 +1460,7 @@ private void updateTreeEventListener(TreeItem<T> oldRoot, TreeItem<T> newRoot) {

private WeakEventHandler<TreeModificationEvent<T>> weakTreeItemListener;

private InvalidationListener showRootListener;


/***********************************************************************
@@ -1595,12 +1600,13 @@ public TreeViewFocusModel(final TreeView<T> treeView) {
focus(0);
}

treeView.showRootProperty().addListener(o -> {
showRootListener = obs -> {
if (isFocused(0)) {
focus(-1);
focus(0);
}
});
};
treeView.showRootProperty().addListener(new WeakInvalidationListener(showRootListener));

focusedIndexProperty().addListener(o -> {
treeView.notifyAccessibleAttributeChanged(AccessibleAttribute.FOCUS_ITEM);
@@ -1614,6 +1620,8 @@ public TreeViewFocusModel(final TreeView<T> treeView) {
private final WeakChangeListener<TreeItem<T>> weakRootPropertyListener =
new WeakChangeListener<>(rootPropertyListener);

private final InvalidationListener showRootListener;

private void updateTreeEventListener(TreeItem<T> oldRoot, TreeItem<T> newRoot) {
if (oldRoot != null && weakTreeItemListener != null) {
oldRoot.removeEventHandler(TreeItem.<T>expandedItemCountChangeEvent(), weakTreeItemListener);
ProTip! Use n and p to navigate between commits in a pull request.