8277000: Tree-/TableRowSkin: replace listener to fixedCellSize by live lookup

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java

@@ -32,7 +32,6 @@
 import com.sun.javafx.scene.control.behavior.BehaviorBase;
 import com.sun.javafx.scene.control.behavior.TableRowBehavior;
 
-import javafx.beans.property.DoubleProperty;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.scene.AccessibleAttribute;
@@ -93,34 +92,25 @@ public TableRowSkin(TableRow<T> control) {
             }
         });
 
-        setupTreeTableViewListeners();
+        setupTableViewListeners();
     }
 
-    // FIXME: replace listener to fixedCellSize with direct lookup - JDK-8277000
-    private void setupTreeTableViewListeners() {
+    private void setupTableViewListeners() {
         TableView<T> tableView = getSkinnable().getTableView();
         if (tableView == null) {
             registerInvalidationListener(getSkinnable().tableViewProperty(), e -> {
                 unregisterInvalidationListeners(getSkinnable().tableViewProperty());
-                setupTreeTableViewListeners();
+                setupTableViewListeners();
             });
         } else {
-            DoubleProperty fixedCellSizeProperty = tableView.fixedCellSizeProperty();
-            if (fixedCellSizeProperty != null) {
-                registerChangeListener(fixedCellSizeProperty, e -> {
-                    fixedCellSize = fixedCellSizeProperty.get();
-                    fixedCellSizeEnabled = fixedCellSize > 0;
-                });
-                fixedCellSize = fixedCellSizeProperty.get();
-                fixedCellSizeEnabled = fixedCellSize > 0;
-
+            if (getFixedCellSize() > 0) {
                 // JDK-8144500:
                 // When in fixed cell size mode, we must listen to the width of the virtual flow, so
                 // that when it changes, we can appropriately add / remove cells that may or may not
                 // be required (because we remove all cells that are not visible).
                 VirtualFlow<TableRow<T>> virtualFlow = getVirtualFlow();
                 if (virtualFlow != null) {
-                    registerChangeListener(virtualFlow.widthProperty(), e -> tableView.requestLayout());
+                    registerChangeListener(virtualFlow.widthProperty(), e -> getTableView().requestLayout());
                 }
             }
         }
@@ -231,6 +221,12 @@ private TableView<T> getTableView() {
         return getSkinnable().getTableView();
     }
 
+    @Override
+    double getFixedCellSize() {
+        TableView<T> tableView = getTableView();
+        return tableView != null ? tableView.getFixedCellSize() : super.getFixedCellSize();
+    }
+
     // test-only
     TableViewSkin<T> getTableViewSkin() {
         TableView<T> tableView = getSkinnable().getTableView();

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java

@@ -41,6 +41,7 @@
 import javafx.scene.Node;
 import javafx.scene.Parent;
 import javafx.scene.control.*;
+import javafx.scene.layout.Region;
 import javafx.util.Duration;
 
 import com.sun.javafx.tk.Toolkit;
@@ -129,10 +130,6 @@ public abstract class TableRowSkinBase<T,
     boolean isDirty = false;
     boolean updateCells = false;
 
-    // FIXME: replace cached values with direct lookup - JDK-8277000
-    double fixedCellSize;
-    boolean fixedCellSizeEnabled;
-
 
     /* *************************************************************************
      *                                                                         *
@@ -318,12 +315,13 @@ protected ObjectProperty<Node> graphicProperty() {
         int index = control.getIndex();
         if (index < 0/* || row >= itemsProperty().get().size()*/) return;
 
+        double fixedCellSize = getFixedCellSize();
         for (int column = 0, max = cells.size(); column < max; column++) {
             R tableCell = cells.get(column);
             TableColumnBase<T, ?> tableColumn = getTableColumn(tableCell);
 
             boolean isVisible = true;
-            if (fixedCellSizeEnabled) {
+            if (fixedCellSize > 0) {
                 // we determine if the cell is visible, and if not we have the
                 // ability to take it out of the scenegraph to help improve
                 // performance. However, we only do this when there is a
@@ -342,7 +340,7 @@ protected ObjectProperty<Node> graphicProperty() {
             }
 
             if (isVisible) {
-                if (fixedCellSizeEnabled && tableCell.getParent() == null) {
+                if (fixedCellSize > 0 && tableCell.getParent() == null) {
                     getChildren().add(tableCell);
                 }
                 // Note: prefWidth() has to be called only after the tableCell is added to the tableRow, if it wasn't
@@ -420,18 +418,20 @@ protected ObjectProperty<Node> graphicProperty() {
                 tableCell.requestLayout();
             } else {
                 width = tableCell.prefWidth(height);
-                if (fixedCellSizeEnabled) {
-                    // we only add/remove to the scenegraph if the fixed cell
-                    // length support is enabled - otherwise we keep all
-                    // TableCells in the scenegraph
-                    getChildren().remove(tableCell);
-                }
+                // we only add/remove to the scenegraph if the fixed cell
+                // length support is enabled - otherwise we keep all
+                // TableCells in the scenegraph
+                getChildren().remove(tableCell);
             }
 
             x += width;
         }
     }
 
+    double getFixedCellSize() {
+        return Region.USE_COMPUTED_SIZE;
+    }
+
     int getIndentationLevel(C control) {
         return 0;
     }
@@ -519,7 +519,7 @@ void updateCells(boolean resetChildren) {
         }
 
         // update children of each row
-        if (fixedCellSizeEnabled) {
+        if (getFixedCellSize() > 0) {
             // we leave the adding / removing up to the layoutChildren method mostly, but here we remove any children
             // cells that refer to columns that are removed or not visible.
             List<Node> toRemove = new ArrayList<>();
@@ -559,7 +559,8 @@ VirtualFlow<C> getVirtualFlow() {
 
     /** {@inheritDoc} */
     @Override protected double computePrefHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
-        if (fixedCellSizeEnabled) {
+        double fixedCellSize = getFixedCellSize();
+        if (fixedCellSize > 0) {
             return fixedCellSize;
         }
 
@@ -592,7 +593,8 @@ VirtualFlow<C> getVirtualFlow() {
 
     /** {@inheritDoc} */
     @Override protected double computeMinHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
-        if (fixedCellSizeEnabled) {
+        double fixedCellSize = getFixedCellSize();
+        if (fixedCellSize > 0) {
             return fixedCellSize;
         }
 
@@ -622,7 +624,8 @@ VirtualFlow<C> getVirtualFlow() {
 
     /** {@inheritDoc} */
     @Override protected double computeMaxHeight(double width, double topInset, double rightInset, double bottomInset, double leftInset) {
-        if (fixedCellSizeEnabled) {
+        double fixedCellSize = getFixedCellSize();
+        if (fixedCellSize > 0) {
             return fixedCellSize;
         }
         return super.computeMaxHeight(width, topInset, rightInset, bottomInset, leftInset);

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java

@@ -97,10 +97,6 @@ public TreeTableRowSkin(TreeTableRow<T> control) {
 
         ListenerHelper lh = ListenerHelper.get(this);
 
-        lh.addChangeListener(control.indexProperty(), (ev) -> {
-            updateCells = true;
-        });
-
         lh.addChangeListener(control.treeItemProperty(), (ev) -> {
             updateTreeItem();
             // There used to be an isDirty = true statement here, but this was
@@ -111,7 +107,6 @@ public TreeTableRowSkin(TreeTableRow<T> control) {
         setupTreeTableViewListeners();
     }
 
-    // FIXME: replace listener to fixedCellSize with direct lookup - JDK-8277000
     private void setupTreeTableViewListeners() {
         TreeTableView<T> treeTableView = getSkinnable().getTreeTableView();
         if (treeTableView == null) {
@@ -130,40 +125,19 @@ private void setupTreeTableViewListeners() {
                 }
             });
 
-            DoubleProperty fixedCellSizeProperty = getTreeTableView().fixedCellSizeProperty();
-            if (fixedCellSizeProperty != null) {
-                registerChangeListener(fixedCellSizeProperty, (x) -> {
-                    updateCachedFixedSize();
-                });
-                updateCachedFixedSize();
-
+            if (getFixedCellSize() > 0) {
                 // JDK-8144500:
                 // When in fixed cell size mode, we must listen to the width of the virtual flow, so
                 // that when it changes, we can appropriately add / remove cells that may or may not
                 // be required (because we remove all cells that are not visible).
                 VirtualFlow<TreeTableRow<T>> virtualFlow = getVirtualFlow();
                 if (virtualFlow != null) {
-                    registerChangeListener(getVirtualFlow().widthProperty(), (x) -> {
-                        if (getSkinnable() != null) {
-                            TreeTableView<T> t = getSkinnable().getTreeTableView();
-                            t.requestLayout();
-                        }
-                    });
+                    registerChangeListener(getVirtualFlow().widthProperty(), e -> getTreeTableView().requestLayout());
                 }
             }
         }
     }
 
-    private void updateCachedFixedSize() {
-        if (getSkinnable() != null) {
-            TreeTableView<T> t = getSkinnable().getTreeTableView();
-            if (t != null) {
-                fixedCellSize = t.getFixedCellSize();
-                fixedCellSizeEnabled = fixedCellSize > 0.0;
-            }
-        }
-    }
-
     /* *************************************************************************
      *                                                                         *
      * Listeners                                                               *
@@ -351,6 +325,12 @@ private TreeTableView<T> getTreeTableView() {
         return getSkinnable().getTreeTableView();
     }
 
+    @Override
+    double getFixedCellSize() {
+        TreeTableView<T> treeTableView = getTreeTableView();
+        return treeTableView != null ? treeTableView.getFixedCellSize() : super.getFixedCellSize();
+    }
+
     private void updateDisclosureNodeAndGraphic() {
         if (getSkinnable().isEmpty()) {
             getChildren().remove(graphic);

modules/javafx.controls/src/shims/java/javafx/scene/control/skin/TableSkinShim.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, 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
@@ -117,13 +117,11 @@ public static VirtualFlow<?> getVirtualFlow(TreeTableView<?> table) {
 //----------------- Tree/TableRowSkin state
 
     public static <T> boolean isFixedCellSizeEnabled(TableRow<T> tableRow) {
-        TableRowSkin<T> skin = (TableRowSkin<T>) tableRow.getSkin();
-        return skin.fixedCellSizeEnabled;
+        return tableRow.getTableView().getFixedCellSize() > 0;
     }
 
     public static <T> boolean isFixedCellSizeEnabled(TreeTableRow<T> tableRow) {
-        TreeTableRowSkin<T> skin = (TreeTableRowSkin<T>) tableRow.getSkin();
-        return skin.fixedCellSizeEnabled;
+        return tableRow.getTreeTableView().getFixedCellSize() > 0;
     }
 
     public static <T> boolean isDirty(TableRow<T> tableRow) {

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java

@@ -121,7 +121,6 @@ public class SkinCleanupTest {
     /**
      * Test access to fixedCellSize via lookup (not listener)
      */
-    @Disabled("JDK-8277000")
     @Test
     public void testTreeTableRowFixedCellSizeListener() {
         TreeTableView<Person> tableView = createPersonTreeTable(false);
@@ -271,6 +270,37 @@ public void testTreeTableRowFixedCellSizeEnabled() {
         assertTrue(isFixedCellSizeEnabled(tableRow), "fixed cell size enabled");
     }
 
+    @Test
+    public void testTreeTableRowVirtualFlowWidthListenerReplaceSkin() {
+        TreeTableView<Person> tableView = createPersonTreeTable(false);
+        tableView.setFixedCellSize(24);
+        showControl(tableView, true);
+        VirtualFlow<?> flow = getVirtualFlow(tableView);
+        TreeTableRow<?> tableRow = (TreeTableRow<?>) getCell(tableView, 1);
+        replaceSkin(tableRow);
+        Toolkit.getToolkit().firePulse();
+        TreeTableRowSkin<?> rowSkin = (TreeTableRowSkin<?>) tableRow.getSkin();
+        assertNotNull(
+                unregisterChangeListeners(rowSkin, flow.widthProperty()),
+                "row skin must have listener to virtualFlow width");
+    }
+
+    /**
+     * Sanity test: listener to flow's width is registered.
+     */
+    @Test
+    public void testTreeTableRowVirtualFlowWidthListener() {
+        TreeTableView<Person> tableView = createPersonTreeTable(false);
+        tableView.setFixedCellSize(24);
+        showControl(tableView, true);
+        VirtualFlow<?> flow = getVirtualFlow(tableView);
+        TreeTableRow<?> tableRow = (TreeTableRow<?>) getCell(tableView, 1);
+        TreeTableRowSkin<?> rowSkin = (TreeTableRowSkin<?>) tableRow.getSkin();
+        assertNotNull(
+                unregisterChangeListeners(rowSkin, flow.widthProperty()),
+                "row skin must have listener to virtualFlow width");
+    }
+
     @Test
     public void testTreeTableRowTracksVirtualFlowReplaceSkin() {
         TreeTableView<Person> tableView = createPersonTreeTable(false);
@@ -550,7 +580,6 @@ private TreeTableView<Person> createPersonTreeTable(boolean installSkin) {
     /**
      * Test access to fixedCellSize via lookup (not listener)
      */
-    @Disabled("JDK-8277000")
     @Test
     public void testTableRowFixedCellSizeListener() {
         TableView<Person> tableView = createPersonTable(false);
@@ -644,6 +673,7 @@ public void testTableRowFixedCellSizeEnabled() {
     @Test
     public void testTableRowVirtualFlowWidthListenerReplaceSkin() {
         TableView<Person> tableView = createPersonTable(false);
+        tableView.setFixedCellSize(24);
         showControl(tableView, true);
         VirtualFlow<?> flow = getVirtualFlow(tableView);
         TableRow<?> tableRow = (TableRow<?>) getCell(tableView, 1);
@@ -661,6 +691,7 @@ public void testTableRowVirtualFlowWidthListenerReplaceSkin() {
     @Test
     public void testTableRowVirtualFlowWidthListener() {
         TableView<Person> tableView = createPersonTable(false);
+        tableView.setFixedCellSize(24);
         showControl(tableView, true);
         VirtualFlow<?> flow = getVirtualFlow(tableView);
         TableRow<?> tableRow = (TableRow<?>) getCell(tableView, 1);
@@ -670,6 +701,31 @@ public void testTableRowVirtualFlowWidthListener() {
                 "row skin must have listener to virtualFlow width");
     }
 
+    @Test
+    public void testTableRowTracksVirtualFlowReplaceSkin() {
+        TableView<Person> tableView = createPersonTable(false);
+        showControl(tableView, true);
+        VirtualFlow<?> flow = getVirtualFlow(tableView);
+        TableRow<?> tableRow = (TableRow<?>) getCell(tableView, 1);
+        replaceSkin(tableRow);
+        Toolkit.getToolkit().firePulse();
+        TableRowSkin<?> rowSkin = (TableRowSkin<?>) tableRow.getSkin();
+        checkFollowsWidth(flow, (Region) rowSkin.getNode());
+    }
+
+    /**
+     * Sanity test checks that tree table row skin tracks the virtual flow width.
+     */
+    @Test
+    public void testTableRowTracksVirtualFlowWidth() {
+        TableView<Person> tableView = createPersonTable(false);
+        showControl(tableView, true);
+        VirtualFlow<?> flow = getVirtualFlow(tableView);
+        TableRow<?> tableRow = (TableRow<?>) getCell(tableView, 1);
+        TableRowSkin<?> rowSkin = (TableRowSkin<?>) tableRow.getSkin();
+        checkFollowsWidth(flow, (Region) rowSkin.getNode());
+    }
+
     /**
      * Sanity: children don't pile up with fixed cell size.
      */