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

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2025, 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
@@ -95,7 +95,6 @@ public TableRowSkin(TableRow<T> control) {
         setupTableViewListeners();
     }
 
-    // FIXME: replace listener to fixedCellSize with direct lookup - JDK-8277000
     private void setupTableViewListeners() {
         TableView<T> tableView = getSkinnable().getTableView();
         if (tableView == null) {
@@ -104,31 +103,22 @@ private void setupTableViewListeners() {
                 setupTableViewListeners();
             });
         } else {
-            registerChangeListener(tableView.fixedCellSizeProperty(), e -> {
-                VirtualFlow<TableRow<T>> virtualFlow = getVirtualFlow();
-                if (virtualFlow != null) {
-                    unregisterChangeListeners(virtualFlow.widthProperty());
-                }
-
-                updateCachedFixedSize();
-            });
-
-            updateCachedFixedSize();
+            VirtualFlow<TableRow<T>> virtualFlow = getVirtualFlow();
+            if (virtualFlow != null) {
+                registerChangeListener(virtualFlow.widthProperty(), _ -> requestLayoutWhenFixedCellSizeSet());
+            }
         }
     }
 
-    private void updateCachedFixedSize() {
-        TableView<T> tableView = getSkinnable().getTableView();
-        if (tableView != null) {
-            fixedCellSize = tableView.getFixedCellSize();
-            fixedCellSizeEnabled = fixedCellSize > 0;
-
-            if (fixedCellSizeEnabled) {
-                VirtualFlow<TableRow<T>> virtualFlow = getVirtualFlow();
-                if (virtualFlow != null) {
-                    registerChangeListener(virtualFlow.widthProperty(), e -> getSkinnable().requestLayout());
-                }
-            }
+    /**
+     * When we have a fixed cell size set, we must request layout when the width of the virtual flow changed,
+     * because we might need to add or remove cells that are now visible or not anymore.
+     * <br>
+     * See also: JDK-8144500 and JDK-8185887.
+     */
+    private void requestLayoutWhenFixedCellSizeSet() {
+        if (getFixedCellSize() > 0) {
+            getSkinnable().requestLayout();
         }
     }
 
@@ -237,6 +227,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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2025, 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
@@ -40,6 +40,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;
@@ -115,10 +116,6 @@ public abstract class TableRowSkinBase<T,
 
     boolean isDirty = false;
 
-    // FIXME: replace cached values with direct lookup - JDK-8277000
-    double fixedCellSize;
-    boolean fixedCellSizeEnabled;
-
 
     /* *************************************************************************
      *                                                                         *
@@ -304,14 +301,15 @@ protected ObjectProperty<Node> graphicProperty() {
         if (index < 0/* || row >= itemsProperty().get().size()*/) return;
 
         VirtualFlow<C> virtualFlow = getVirtualFlow();
+        double fixedCellSize = getFixedCellSize();
         for (int column = 0, max = cells.size(); column < max; column++) {
             R tableCell = cells.get(column);
             TableColumnBase<T, ?> tableColumn = getTableColumn(tableCell);
 
             width = snapSizeX(tableColumn.getWidth());
 
             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
@@ -413,6 +411,10 @@ protected ObjectProperty<Node> graphicProperty() {
         }
     }
 
+    double getFixedCellSize() {
+        return Region.USE_COMPUTED_SIZE;
+    }
+
     int getIndentationLevel(C control) {
         return 0;
     }
@@ -512,7 +514,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;
         }
 
@@ -545,7 +548,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;
         }
 
@@ -575,7 +579,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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2025, 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
@@ -106,7 +106,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) {
@@ -119,32 +118,22 @@ private void setupTreeTableViewListeners() {
                 updateLeafColumns();
             });
 
-            registerChangeListener(getTreeTableView().fixedCellSizeProperty(), e -> {
-                VirtualFlow<TreeTableRow<T>> virtualFlow = getVirtualFlow();
-                if (virtualFlow != null) {
-                    unregisterChangeListeners(virtualFlow.widthProperty());
-                }
-
-                updateCachedFixedSize();
-            });
-            updateCachedFixedSize();
+            VirtualFlow<TreeTableRow<T>> virtualFlow = getVirtualFlow();
+            if (virtualFlow != null) {
+                registerChangeListener(virtualFlow.widthProperty(), _ -> requestLayoutWhenFixedCellSizeSet());
+            }
         }
     }
 
-    private void updateCachedFixedSize() {
-        if (getSkinnable() != null) {
-            TreeTableView<T> t = getSkinnable().getTreeTableView();
-            if (t != null) {
-                fixedCellSize = t.getFixedCellSize();
-                fixedCellSizeEnabled = fixedCellSize > 0.0;
-
-                if (fixedCellSizeEnabled) {
-                    VirtualFlow<TreeTableRow<T>> virtualFlow = getTableViewSkin().getVirtualFlow();
-                    if (virtualFlow != null) {
-                        registerChangeListener(virtualFlow.widthProperty(), ev -> getSkinnable().requestLayout());
-                    }
-                }
-            }
+    /**
+     * When we have a fixed cell size set, we must request layout when the width of the virtual flow changed,
+     * because we might need to add or remove cells that are now visible or not anymore.
+     * <br>
+     * See also: JDK-8144500 and JDK-8185887.
+     */
+    private void requestLayoutWhenFixedCellSizeSet() {
+        if (getFixedCellSize() > 0) {
+            getSkinnable().requestLayout();
         }
     }
 
@@ -319,6 +308,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() {
         disclosureNodeDirty = false;
 

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, 2025, 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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2025, 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
@@ -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);
@@ -672,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.
      */