Skip to content

Commit 560ef17

Browse files
Jeanette Winzenburgarapte
Jeanette Winzenburg
authored and
arapte
committed
8241455: Memory leak on replacing selection/focusModel
Reviewed-by: arapte, aghaisas
1 parent 5906521 commit 560ef17

File tree

5 files changed

+337
-18
lines changed

5 files changed

+337
-18
lines changed

‎modules/javafx.controls/src/main/java/javafx/scene/control/ChoiceBox.java

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2010, 2020, 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
@@ -29,9 +29,11 @@
2929
import javafx.beans.property.ObjectPropertyBase;
3030
import javafx.beans.property.SimpleObjectProperty;
3131
import javafx.beans.value.ChangeListener;
32+
import javafx.beans.value.WeakChangeListener;
3233
import javafx.collections.FXCollections;
3334
import javafx.collections.ListChangeListener;
3435
import javafx.collections.ObservableList;
36+
import javafx.collections.WeakListChangeListener;
3537
import javafx.beans.property.ReadOnlyBooleanProperty;
3638
import javafx.beans.property.ReadOnlyBooleanWrapper;
3739
import javafx.event.ActionEvent;
@@ -503,6 +505,9 @@ public void hide() {
503505
// package for testing
504506
static class ChoiceBoxSelectionModel<T> extends SingleSelectionModel<T> {
505507
private final ChoiceBox<T> choiceBox;
508+
private ChangeListener<ObservableList<T>> itemsObserver;
509+
private ListChangeListener<T> itemsContentObserver;
510+
private WeakListChangeListener<T> weakItemsContentObserver;
506511

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

522527
// watching for changes to the items list content
523-
final ListChangeListener<T> itemsContentObserver = c -> {
528+
itemsContentObserver = c -> {
524529
if (choiceBox.getItems() == null || choiceBox.getItems().isEmpty()) {
525530
setSelectedIndex(-1);
526531
} else if (getSelectedIndex() == -1 && getSelectedItem() != null) {
@@ -530,17 +535,18 @@ public ChoiceBoxSelectionModel(final ChoiceBox<T> cb) {
530535
}
531536
}
532537
};
538+
weakItemsContentObserver = new WeakListChangeListener<>(itemsContentObserver);
533539
if (this.choiceBox.getItems() != null) {
534-
this.choiceBox.getItems().addListener(itemsContentObserver);
540+
this.choiceBox.getItems().addListener(weakItemsContentObserver);
535541
}
536542

537543
// watching for changes to the items list
538-
ChangeListener<ObservableList<T>> itemsObserver = (valueModel, oldList, newList) -> {
544+
itemsObserver = (valueModel, oldList, newList) -> {
539545
if (oldList != null) {
540-
oldList.removeListener(itemsContentObserver);
546+
oldList.removeListener(weakItemsContentObserver);
541547
}
542548
if (newList != null) {
543-
newList.addListener(itemsContentObserver);
549+
newList.addListener(weakItemsContentObserver);
544550
}
545551
setSelectedIndex(-1);
546552
if (getSelectedItem() != null) {
@@ -550,7 +556,9 @@ public ChoiceBoxSelectionModel(final ChoiceBox<T> cb) {
550556
}
551557
}
552558
};
553-
this.choiceBox.itemsProperty().addListener(itemsObserver);
559+
// TBD: use pattern as f.i. in listView selectionModel (invalidationListener to really
560+
// get all changes - including list of same content - of the list-valued property)
561+
this.choiceBox.itemsProperty().addListener(new WeakChangeListener<>(itemsObserver));
554562
}
555563

556564
// API Implementation

‎modules/javafx.controls/src/main/java/javafx/scene/control/TabPane.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2011, 2020, 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
@@ -41,6 +41,7 @@
4141
import javafx.beans.value.WritableValue;
4242
import javafx.collections.ListChangeListener;
4343
import javafx.collections.ObservableList;
44+
import javafx.collections.WeakListChangeListener;
4445
import javafx.geometry.Side;
4546
import javafx.scene.AccessibleAttribute;
4647
import javafx.scene.AccessibleRole;
@@ -672,14 +673,16 @@ public StyleableProperty<Number> getStyleableProperty(TabPane n) {
672673
static class TabPaneSelectionModel extends SingleSelectionModel<Tab> {
673674
private final TabPane tabPane;
674675

676+
private ListChangeListener<Tab> itemsContentObserver;
677+
675678
public TabPaneSelectionModel(final TabPane t) {
676679
if (t == null) {
677680
throw new NullPointerException("TabPane can not be null");
678681
}
679682
this.tabPane = t;
680683

681684
// watching for changes to the items list content
682-
final ListChangeListener<Tab> itemsContentObserver = c -> {
685+
itemsContentObserver = c -> {
683686
while (c.next()) {
684687
for (Tab tab : c.getRemoved()) {
685688
if (tab != null && !tabPane.getTabs().contains(tab)) {
@@ -710,7 +713,7 @@ public TabPaneSelectionModel(final TabPane t) {
710713
}
711714
};
712715
if (this.tabPane.getTabs() != null) {
713-
this.tabPane.getTabs().addListener(itemsContentObserver);
716+
this.tabPane.getTabs().addListener(new WeakListChangeListener<>(itemsContentObserver));
714717
}
715718
}
716719

‎modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2012, 2020, 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
@@ -3406,12 +3406,13 @@ public TreeTableViewFocusModel(final TreeTableView<S> treeTableView) {
34063406
TreeTablePosition<S,?> pos = new TreeTablePosition<>(treeTableView, focusRow, null);
34073407
setFocusedCell(pos);
34083408

3409-
treeTableView.showRootProperty().addListener(o -> {
3409+
showRootListener = obs -> {
34103410
if (isFocused(0)) {
34113411
focus(-1);
34123412
focus(0);
34133413
}
3414-
});
3414+
};
3415+
treeTableView.showRootProperty().addListener(new WeakInvalidationListener(showRootListener));
34153416

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

3429+
private final InvalidationListener showRootListener;
3430+
34283431
private void updateTreeEventListener(TreeItem<S> oldRoot, TreeItem<S> newRoot) {
34293432
if (oldRoot != null && weakTreeItemListener != null) {
34303433
oldRoot.removeEventHandler(TreeItem.<S>expandedItemCountChangeEvent(), weakTreeItemListener);

‎modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2008, 2020, 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
@@ -32,6 +32,8 @@
3232

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

12961298
this.treeView = treeView;
12971299
this.treeView.rootProperty().addListener(weakRootPropertyListener);
1298-
this.treeView.showRootProperty().addListener(o -> {
1300+
showRootListener = o -> {
12991301
shiftSelection(0, treeView.isShowRoot() ? 1 : -1, null);
1300-
});
1302+
};
1303+
this.treeView.showRootProperty().addListener(new WeakInvalidationListener(showRootListener));
13011304

13021305
updateTreeEventListener(null, treeView.getRoot());
13031306

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

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

14571461
private WeakEventHandler<TreeModificationEvent<T>> weakTreeItemListener;
14581462

1463+
private InvalidationListener showRootListener;
14591464

14601465

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

1598-
treeView.showRootProperty().addListener(o -> {
1603+
showRootListener = obs -> {
15991604
if (isFocused(0)) {
16001605
focus(-1);
16011606
focus(0);
16021607
}
1603-
});
1608+
};
1609+
treeView.showRootProperty().addListener(new WeakInvalidationListener(showRootListener));
16041610

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

1623+
private final InvalidationListener showRootListener;
1624+
16171625
private void updateTreeEventListener(TreeItem<T> oldRoot, TreeItem<T> newRoot) {
16181626
if (oldRoot != null && weakTreeItemListener != null) {
16191627
oldRoot.removeEventHandler(TreeItem.<T>expandedItemCountChangeEvent(), weakTreeItemListener);

0 commit comments

Comments
 (0)