8295806: TableViewSkin: memory leak when changing skin

Reviewed-by: aghaisas
openjdk · Dec 5, 2022 · bb98d88 · bb98d88
1 parent 2c18c18
commit bb98d88
Show file tree

Hide file tree

Showing 7 changed files with 143 additions and 105 deletions.
diff --git a/...avafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableViewBehavior.java b/...avafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableViewBehavior.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2022, 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
@@ -77,7 +77,7 @@ public TableViewBehavior(TableView<T> control) {
         control.selectionModelProperty().addListener(weakSelectionModelListener);
         TableViewSelectionModel<T> sm = control.getSelectionModel();
         if (sm != null) {
-            sm.getSelectedCells().addListener(selectedCellsListener);
+            sm.getSelectedCells().addListener(weakSelectedCellsListener);
         }
 
         // Only add this if we're on an embedded platform that supports 5-button navigation
@@ -86,8 +86,12 @@ public TableViewBehavior(TableView<T> control) {
         }
     }
 
-    @Override public void dispose() {
-        if (tlFocus != null) tlFocus.dispose();
+    @Override
+    public void dispose() {
+        if (tlFocus != null) {
+            tlFocus.dispose();
+            tlFocus = null;
+        }
         super.dispose();
     }
 

diff --git a/...x.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableViewBehaviorBase.java b/...x.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableViewBehaviorBase.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2022, 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
@@ -405,6 +405,24 @@ protected void setAnchor(int row, TableColumnBase col) {
     private Runnable onFocusLeftCell;
     public void setOnFocusLeftCell(Runnable r) { onFocusLeftCell = r; }
 
+    @Override
+    public void dispose() {
+        onScrollPageUp = null;
+        onScrollPageDown = null;
+        onFocusPreviousRow = null;
+        onFocusNextRow = null;
+        onSelectPreviousRow = null;
+        onSelectNextRow = null;
+        onMoveToFirstCell = null;
+        onMoveToLastCell = null;
+        onSelectRightCell = null;
+        onSelectLeftCell = null;
+        onFocusRightCell = null;
+        onFocusLeftCell = null;
+
+        super.dispose();
+    }
+
     public void mousePressed(MouseEvent e) {
 //        // FIXME can't assume (yet) cells.get(0) is necessarily the lead cell
 //        ObservableList<? extends TablePositionBase> cells = getSelectedCells();

diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2022, 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
@@ -60,7 +60,6 @@ public class TableRowSkin<T> extends TableRowSkinBase<T, TableRow<T>, TableCell<
      *                                                                         *
      **************************************************************************/
 
-    private TableViewSkin<T> tableViewSkin;
     private final BehaviorBase<TableRow<T>> behavior;
 
 
@@ -83,13 +82,8 @@ public TableRowSkin(TableRow<T> control) {
 
         // install default input map for the TableRow control
         behavior = new TableRowBehavior<>(control);
-//        control.setInputMap(behavior.getInputMap());
-
-        updateTableViewSkin();
 
         registerChangeListener(control.tableViewProperty(), e -> {
-            updateTableViewSkin();
-
             for (int i = 0, max = cells.size(); i < max; i++) {
                 Node n = cells.get(i);
                 if (n instanceof TableCell) {
@@ -229,16 +223,12 @@ private TableView<T> getTableView() {
         return getSkinnable().getTableView();
     }
 
-    private void updateTableViewSkin() {
+    // test-only
+    TableViewSkin<T> getTableViewSkin() {
         TableView<T> tableView = getSkinnable().getTableView();
         if (tableView != null && tableView.getSkin() instanceof TableViewSkin) {
-            tableViewSkin = (TableViewSkin)tableView.getSkin();
+            return (TableViewSkin)tableView.getSkin();
         }
+        return null;
     }
-
-    // test-only
-    TableViewSkin<T> getTableViewSkin() {
-        return tableViewSkin;
-    }
-
 }
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkin.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2022, 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
@@ -43,6 +43,8 @@
 import javafx.scene.control.TableView;
 import javafx.scene.control.TableView.TableViewSelectionModel;
 import javafx.scene.input.MouseEvent;
+
+import com.sun.javafx.scene.control.ListenerHelper;
 import com.sun.javafx.scene.control.behavior.TableViewBehavior;
 
 /**
@@ -81,11 +83,12 @@ public TableViewSkin(final TableView<T> control) {
 
         // install default input map for the TableView control
         behavior = new TableViewBehavior<>(control);
-//        control.setInputMap(behavior.getInputMap());
 
         flow.setFixedCellSize(control.getFixedCellSize());
         flow.setCellFactory(flow -> createCell());
 
+        ListenerHelper lh = ListenerHelper.get(this);
+
         EventHandler<MouseEvent> ml = event -> {
             // This ensures that the table maintains the focus, even when the vbar
             // and hbar controls inside the flow are clicked. Without this, the
@@ -96,8 +99,8 @@ public TableViewSkin(final TableView<T> control) {
                 control.requestFocus();
             }
         };
-        flow.getVbar().addEventFilter(MouseEvent.MOUSE_PRESSED, ml);
-        flow.getHbar().addEventFilter(MouseEvent.MOUSE_PRESSED, ml);
+        lh.addEventFilter(flow.getVbar(), MouseEvent.MOUSE_PRESSED, ml);
+        lh.addEventFilter(flow.getHbar(), MouseEvent.MOUSE_PRESSED, ml);
 
         // init the behavior 'closures'
         behavior.setOnFocusPreviousRow(() -> onFocusAboveCell());
@@ -113,7 +116,9 @@ public TableViewSkin(final TableView<T> control) {
         behavior.setOnFocusLeftCell(() -> onFocusLeftCell());
         behavior.setOnFocusRightCell(() -> onFocusRightCell());
 
-        registerChangeListener(control.fixedCellSizeProperty(), e -> flow.setFixedCellSize(getSkinnable().getFixedCellSize()));
+        lh.addChangeListener(control.fixedCellSizeProperty(), (ev) -> {
+            flow.setFixedCellSize(getSkinnable().getFixedCellSize());
+        });
 
         updateItemCount();
     }
@@ -127,12 +132,13 @@ public TableViewSkin(final TableView<T> control) {
      **************************************************************************/
 
     /** {@inheritDoc} */
-    @Override public void dispose() {
-        super.dispose();
-
+    @Override
+    public void dispose() {
         if (behavior != null) {
             behavior.dispose();
         }
+
+        super.dispose();
     }
 
     /** {@inheritDoc} */