8359599: Calling refresh() for all virtualized controls recreates all cells instead of refreshing the cells

Marius Hanl · Marius Hanl · commit 02756a810c54 · 2025-11-05T22:37:49.000Z
Reviewed-by: angorya, kcr, jhendrikx, jvos
diff --git a/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/Properties.java b/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/Properties.java
@@ -82,12 +82,12 @@ public static String getColorPickerString(String key) {
 
     /***************************************************************************
      *
-     * ListView, TableView
+     * Virtualized controls
      *
      **************************************************************************/
 
-    public static final String REFRESH = "refreshKey";
     public static final String RECREATE = "recreateKey";
+    public static final String REBUILD = "rebuildKey";
 
 
 
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/ListView.java
@@ -1032,16 +1032,14 @@ public final ObjectProperty<EventHandler<ScrollToEvent<Integer>>> onScrollToProp
     }
 
     /**
-     * Calling {@code refresh()} forces the ListView control to recreate and
-     * repopulate the cells necessary to populate the visual bounds of the control.
-     * In other words, this forces the ListView to update what it is showing to
-     * the user. This is useful in cases where the underlying data source has
-     * changed in a way that is not observed by the ListView itself.
+     * Forces the ListView to update what it is showing to the user.
+     * This is useful in cases where the underlying data source has changed in a way
+     * that is not observed by the ListView itself.
      *
      * @since JavaFX 8u60
      */
     public void refresh() {
-        getProperties().put(Properties.RECREATE, Boolean.TRUE);
+        getProperties().put(Properties.REBUILD, Boolean.TRUE);
     }
 
 
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java
@@ -1763,16 +1763,14 @@ public void sort() {
     }
 
     /**
-     * Calling {@code refresh()} forces the TableView control to recreate and
-     * repopulate the cells necessary to populate the visual bounds of the control.
-     * In other words, this forces the TableView to update what it is showing to
-     * the user. This is useful in cases where the underlying data source has
-     * changed in a way that is not observed by the TableView itself.
+     * Forces the TableView to update what it is showing to the user.
+     * This is useful in cases where the underlying data source has changed in a way
+     * that is not observed by the TableView itself.
      *
      * @since JavaFX 8u60
      */
     public void refresh() {
-        getProperties().put(Properties.RECREATE, Boolean.TRUE);
+        getProperties().put(Properties.REBUILD, Boolean.TRUE);
     }
 
 
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java
@@ -2079,16 +2079,14 @@ public void sort() {
     }
 
     /**
-     * Calling {@code refresh()} forces the TreeTableView control to recreate and
-     * repopulate the cells necessary to populate the visual bounds of the control.
-     * In other words, this forces the TreeTableView to update what it is showing to
-     * the user. This is useful in cases where the underlying data source has
-     * changed in a way that is not observed by the TreeTableView itself.
+     * Forces the TreeTableView to update what it is showing to the user.
+     * This is useful in cases where the underlying data source has changed in a way
+     * that is not observed by the TreeTableView itself.
      *
      * @since JavaFX 8u60
      */
     public void refresh() {
-        getProperties().put(Properties.RECREATE, Boolean.TRUE);
+        getProperties().put(Properties.REBUILD, Boolean.TRUE);
     }
 
 
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java
@@ -1109,16 +1109,14 @@ public int getTreeItemLevel(TreeItem<?> node) {
     }
 
     /**
-     * Calling {@code refresh()} forces the TreeView control to recreate and
-     * repopulate the cells necessary to populate the visual bounds of the control.
-     * In other words, this forces the TreeView to update what it is showing to
-     * the user. This is useful in cases where the underlying data source has
-     * changed in a way that is not observed by the TreeView itself.
+     * Forces the TreeView to update what it is showing to the user.
+     * This is useful in cases where the underlying data source has changed in a way
+     * that is not observed by the TreeView itself.
      *
      * @since JavaFX 8u60
      */
     public void refresh() {
-        getProperties().put(Properties.RECREATE, Boolean.TRUE);
+        getProperties().put(Properties.REBUILD, Boolean.TRUE);
     }
 
 
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListViewSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ListViewSkin.java
@@ -101,7 +101,6 @@ public class ListViewSkin<T> extends VirtualContainerBase<ListView<T>, ListCell<
 
     private ObservableList<T> listViewItems;
 
-    private boolean needCellsRebuilt = true;
     private boolean needCellsReconfigured = false;
 
     private int itemCount = -1;
@@ -117,10 +116,9 @@ public class ListViewSkin<T> extends VirtualContainerBase<ListView<T>, ListCell<
 
     private MapChangeListener<Object, Object> propertiesMapListener = c -> {
         if (! c.wasAdded()) return;
-        if (Properties.RECREATE.equals(c.getKey())) {
-            needCellsRebuilt = true;
-            getSkinnable().requestLayout();
-            getSkinnable().getProperties().remove(Properties.RECREATE);
+        if (Properties.REBUILD.equals(c.getKey())) {
+            requestRebuildCells();
+            getSkinnable().getProperties().remove(Properties.REBUILD);
         }
     };
 
@@ -230,7 +228,7 @@ public ListViewSkin(final ListView<T> control) {
         control.itemsProperty().addListener(weakItemsChangeListener);
 
         final ObservableMap<Object, Object> properties = control.getProperties();
-        properties.remove(Properties.RECREATE);
+        properties.remove(Properties.REBUILD);
         properties.addListener(weakPropertiesMapListener);
 
         // Register listeners
@@ -281,13 +279,10 @@ public ListViewSkin(final ListView<T> control) {
                                             final double w, final double h) {
         super.layoutChildren(x, y, w, h);
 
-        if (needCellsRebuilt) {
-            flow.rebuildCells();
-        } else if (needCellsReconfigured) {
+        if (needCellsReconfigured) {
             flow.reconfigureCells();
         }
 
-        needCellsRebuilt = false;
         needCellsReconfigured = false;
 
         if (getItemCount() == 0) {
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkinBase.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableViewSkinBase.java
@@ -152,7 +152,6 @@ public abstract class TableViewSkinBase<M, S, C extends Control, I extends Index
 
     private int visibleColCount;
 
-    boolean needCellsRecreated = true;
     boolean needCellsReconfigured = false;
 
     private int itemCount = -1;
@@ -297,19 +296,18 @@ public TableViewSkinBase(final C control) {
         });
 
         final ObservableMap<Object, Object> properties = control.getProperties();
-        properties.remove(Properties.REFRESH);
         properties.remove(Properties.RECREATE);
+        properties.remove(Properties.REBUILD);
         lh.addMapChangeListener(properties, (c) -> {
             if (!c.wasAdded()) {
                 return;
             }
-            if (Properties.REFRESH.equals(c.getKey())) {
-                refreshView();
-                getSkinnable().getProperties().remove(Properties.REFRESH);
-            } else if (Properties.RECREATE.equals(c.getKey())) {
-                needCellsRecreated = true;
-                refreshView();
+            if (Properties.RECREATE.equals(c.getKey())) {
+                requestRecreateCells();
                 getSkinnable().getProperties().remove(Properties.RECREATE);
+            } else if (Properties.REBUILD.equals(c.getKey())) {
+                requestRebuildCells();
+                getSkinnable().getProperties().remove(Properties.REBUILD);
             }
         });
 
@@ -425,13 +423,10 @@ public void dispose() {
 
         super.layoutChildren(x, y, w, h);
 
-        if (needCellsRecreated) {
-            flow.recreateCells();
-        } else if (needCellsReconfigured) {
+        if (needCellsReconfigured) {
             flow.reconfigureCells();
         }
 
-        needCellsRecreated = false;
         needCellsReconfigured = false;
 
         final double baselineOffset = table.getLayoutBounds().getHeight() / 2;
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeViewSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeViewSkin.java
@@ -102,9 +102,9 @@ public class TreeViewSkin<T> extends VirtualContainerBase<TreeView<T>, TreeCell<
 
     private MapChangeListener<Object, Object> propertiesMapListener = c -> {
         if (! c.wasAdded()) return;
-        if (Properties.RECREATE.equals(c.getKey())) {
+        if (Properties.REBUILD.equals(c.getKey())) {
             requestRebuildCells();
-            getSkinnable().getProperties().remove(Properties.RECREATE);
+            getSkinnable().getProperties().remove(Properties.REBUILD);
         }
     };
 
@@ -184,7 +184,7 @@ public TreeViewSkin(final TreeView control) {
         flow.getHbar().addEventFilter(MouseEvent.MOUSE_PRESSED, ml);
 
         final ObservableMap<Object, Object> properties = control.getProperties();
-        properties.remove(Properties.RECREATE);
+        properties.remove(Properties.REBUILD);
         properties.addListener(propertiesMapListener);
 
         // init the behavior 'closures'
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualContainerBase.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualContainerBase.java
@@ -191,4 +191,8 @@ void requestRebuildCells() {
         flow.rebuildCells();
     }
 
+    void requestRecreateCells() {
+        flow.recreateCells();
+    }
+
 }
diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java
@@ -43,6 +43,7 @@
 import java.util.List;
 import java.util.ListIterator;
 import java.util.NoSuchElementException;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 import javafx.application.Platform;
 import javafx.beans.binding.Bindings;
@@ -2726,6 +2727,65 @@ public void fixListViewCrash_JDK_8303680() {
         // But we wan't to ensure, that the VirtualFlow "Doesn't crash" - which was the case before.
     }
 
+    @Test
+    void testRefreshShouldNotResetCells() {
+        final AtomicInteger creationCounter = new AtomicInteger();
+
+        ListView<Person> listView = new ListView<>();
+        listView.setItems(FXCollections.observableArrayList(new Person("name")));
+        listView.setCellFactory(_ -> {
+            creationCounter.incrementAndGet();
+            return new ListCell<>();
+        });
+
+        stageLoader = new StageLoader(listView);
+        Toolkit.getToolkit().firePulse();
+
+        assertTrue(creationCounter.get() > 0);
+        creationCounter.set(0);
+
+        listView.refresh();
+        Toolkit.getToolkit().firePulse();
+
+        assertEquals(0, creationCounter.get());
+    }
+
+    @Test
+    void testRefreshShouldReflectChangeInCell() {
+        String initialName = "Initial";
+        Person person = new Person(initialName);
+
+        ListView<Person> listView = new ListView<>();
+        listView.setItems(FXCollections.observableArrayList(person));
+        listView.setCellFactory(_ -> new ListCell<>() {
+            @Override
+            protected void updateItem(Person item, boolean empty) {
+                super.updateItem(item, empty);
+
+                if (empty) {
+                    setText(null);
+                } else {
+                    setText(item.getFirstName());
+                }
+            }
+        });
+
+        stageLoader = new StageLoader(listView);
+        Toolkit.getToolkit().firePulse();
+
+        String newName = "Other Name";
+        person.setFirstName(newName);
+
+        IndexedCell<?> cell = VirtualFlowTestUtils.getCell(listView, 0);
+        assertEquals(initialName, cell.getText());
+
+        listView.refresh();
+        Toolkit.getToolkit().firePulse();
+
+        cell = VirtualFlowTestUtils.getCell(listView, 0);
+        assertEquals(newName, cell.getText());
+    }
+
     private static double toViewportLength(double prefHeight) {
         // it would be better to calculate this from listView but there is no API for this
         return prefHeight - 2;
diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java
@@ -6427,4 +6427,68 @@ public void testScrollingXIsSnapped() {
 
         assertEquals(-snappedNewValue, rootHeader.getLayoutX(), 0);
     }
+
+    @Test
+    void testRefreshShouldNotResetCells() {
+        final AtomicInteger cellCreationCounter = new AtomicInteger();
+        final AtomicInteger rowCreationCounter = new AtomicInteger();
+
+        TableColumn<Person, String> firstNameCol = new TableColumn<>("First Name");
+        firstNameCol.setCellValueFactory(new PropertyValueFactory<>("firstName"));
+
+        firstNameCol.setCellFactory(_ -> {
+            cellCreationCounter.incrementAndGet();
+            return new TableCell<>();
+        });
+
+        TableView<Person> table = new TableView<>();
+        table.setRowFactory(_ -> {
+            rowCreationCounter.incrementAndGet();
+            return new TableRow<>();
+        });
+        table.setItems(FXCollections.observableArrayList(new Person("name")));
+        table.getColumns().add(firstNameCol);
+
+        stageLoader = new StageLoader(table);
+        Toolkit.getToolkit().firePulse();
+
+        assertTrue(cellCreationCounter.get() > 0);
+        assertTrue(rowCreationCounter.get() > 0);
+        rowCreationCounter.set(0);
+        cellCreationCounter.set(0);
+
+        table.refresh();
+        Toolkit.getToolkit().firePulse();
+
+        assertEquals(0, rowCreationCounter.get());
+        assertEquals(0, cellCreationCounter.get());
+    }
+
+    @Test
+    void testRefreshShouldReflectChangeInCell() {
+        String initialName = "Initial";
+        Person person = new Person(initialName);
+
+        TableColumn<Person, String> firstNameCol = new TableColumn<>("First Name");
+        firstNameCol.setCellValueFactory(e -> new SimpleStringProperty(e.getValue().getFirstName()));
+
+        TableView<Person> table = new TableView<>();
+        table.setItems(FXCollections.observableArrayList(person));
+        table.getColumns().add(firstNameCol);
+
+        stageLoader = new StageLoader(table);
+        Toolkit.getToolkit().firePulse();
+
+        String newName = "Other Name";
+        person.setFirstName(newName);
+
+        IndexedCell<?> cell = VirtualFlowTestUtils.getCell(table, 0, 0);
+        assertEquals(initialName, cell.getText());
+
+        table.refresh();
+        Toolkit.getToolkit().firePulse();
+
+        cell = VirtualFlowTestUtils.getCell(table, 0, 0);
+        assertEquals(newName, cell.getText());
+    }
 }
diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java
diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java
diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java
diff --git a/tests/performance/control/src/table/TableRefreshTest.java b/tests/performance/control/src/table/TableRefreshTest.java

Original file line number	Diff line number	Diff line change
`@@ -82,12 +82,12 @@ public static String getColorPickerString(String key) {`
`82`	`82`
`83`	`83`	`/***************************************************************************`
`84`	`84`	`*`
`85`		`- * ListView, TableView`
	`85`	`+ * Virtualized controls`
`86`	`86`	`*`
`87`	`87`	`**************************************************************************/`
`88`	`88`
`89`		`- public static final String REFRESH = "refreshKey";`
`90`	`89`	`public static final String RECREATE = "recreateKey";`
	`90`	`+ public static final String REBUILD = "rebuildKey";`
`91`	`91`
`92`	`92`
`93`	`93`
Original file line number	Diff line number	Diff line change
`@@ -1032,16 +1032,14 @@ public final ObjectProperty<EventHandler<ScrollToEvent<Integer>>> onScrollToProp`
`1032`	`1032`	`}`
`1033`	`1033`
`1034`	`1034`	`/**`
`1035`		`- * Calling {@code refresh()} forces the ListView control to recreate and`
`1036`		`- * repopulate the cells necessary to populate the visual bounds of the control.`
`1037`		`- * In other words, this forces the ListView to update what it is showing to`
`1038`		`- * the user. This is useful in cases where the underlying data source has`
`1039`		`- * changed in a way that is not observed by the ListView itself.`
	`1035`	`+ * Forces the ListView to update what it is showing to the user.`
	`1036`	`+ * This is useful in cases where the underlying data source has changed in a way`
	`1037`	`+ * that is not observed by the ListView itself.`
`1040`	`1038`	`*`
`1041`	`1039`	`* @since JavaFX 8u60`
`1042`	`1040`	`*/`
`1043`	`1041`	`public void refresh() {`
`1044`		`- getProperties().put(Properties.RECREATE, Boolean.TRUE);`
	`1042`	`+ getProperties().put(Properties.REBUILD, Boolean.TRUE);`
`1045`	`1043`	`}`
`1046`	`1044`
`1047`	`1045`
Original file line number	Diff line number	Diff line change
`@@ -1763,16 +1763,14 @@ public void sort() {`
`1763`	`1763`	`}`
`1764`	`1764`
`1765`	`1765`	`/**`
`1766`		`- * Calling {@code refresh()} forces the TableView control to recreate and`
`1767`		`- * repopulate the cells necessary to populate the visual bounds of the control.`
`1768`		`- * In other words, this forces the TableView to update what it is showing to`
`1769`		`- * the user. This is useful in cases where the underlying data source has`
`1770`		`- * changed in a way that is not observed by the TableView itself.`
	`1766`	`+ * Forces the TableView to update what it is showing to the user.`
	`1767`	`+ * This is useful in cases where the underlying data source has changed in a way`
	`1768`	`+ * that is not observed by the TableView itself.`
`1771`	`1769`	`*`
`1772`	`1770`	`* @since JavaFX 8u60`
`1773`	`1771`	`*/`
`1774`	`1772`	`public void refresh() {`
`1775`		`- getProperties().put(Properties.RECREATE, Boolean.TRUE);`
	`1773`	`+ getProperties().put(Properties.REBUILD, Boolean.TRUE);`
`1776`	`1774`	`}`
`1777`	`1775`
`1778`	`1776`
Original file line number	Diff line number	Diff line change
`@@ -2079,16 +2079,14 @@ public void sort() {`
`2079`	`2079`	`}`
`2080`	`2080`
`2081`	`2081`	`/**`
`2082`		`- * Calling {@code refresh()} forces the TreeTableView control to recreate and`
`2083`		`- * repopulate the cells necessary to populate the visual bounds of the control.`
`2084`		`- * In other words, this forces the TreeTableView to update what it is showing to`
`2085`		`- * the user. This is useful in cases where the underlying data source has`
`2086`		`- * changed in a way that is not observed by the TreeTableView itself.`
	`2082`	`+ * Forces the TreeTableView to update what it is showing to the user.`
	`2083`	`+ * This is useful in cases where the underlying data source has changed in a way`
	`2084`	`+ * that is not observed by the TreeTableView itself.`
`2087`	`2085`	`*`
`2088`	`2086`	`* @since JavaFX 8u60`
`2089`	`2087`	`*/`
`2090`	`2088`	`public void refresh() {`
`2091`		`- getProperties().put(Properties.RECREATE, Boolean.TRUE);`
	`2089`	`+ getProperties().put(Properties.REBUILD, Boolean.TRUE);`
`2092`	`2090`	`}`
`2093`	`2091`
`2094`	`2092`
Original file line number	Diff line number	Diff line change
`@@ -1109,16 +1109,14 @@ public int getTreeItemLevel(TreeItem<?> node) {`
`1109`	`1109`	`}`
`1110`	`1110`
`1111`	`1111`	`/**`
`1112`		`- * Calling {@code refresh()} forces the TreeView control to recreate and`
`1113`		`- * repopulate the cells necessary to populate the visual bounds of the control.`
`1114`		`- * In other words, this forces the TreeView to update what it is showing to`
`1115`		`- * the user. This is useful in cases where the underlying data source has`
`1116`		`- * changed in a way that is not observed by the TreeView itself.`
	`1112`	`+ * Forces the TreeView to update what it is showing to the user.`
	`1113`	`+ * This is useful in cases where the underlying data source has changed in a way`
	`1114`	`+ * that is not observed by the TreeView itself.`
`1117`	`1115`	`*`
`1118`	`1116`	`* @since JavaFX 8u60`
`1119`	`1117`	`*/`
`1120`	`1118`	`public void refresh() {`
`1121`		`- getProperties().put(Properties.RECREATE, Boolean.TRUE);`
	`1119`	`+ getProperties().put(Properties.REBUILD, Boolean.TRUE);`
`1122`	`1120`	`}`
`1123`	`1121`
`1124`	`1122`
Original file line number	Diff line number	Diff line change
`@@ -191,4 +191,8 @@ void requestRebuildCells() {`
`191`	`191`	`flow.rebuildCells();`
`192`	`192`	`}`
`193`	`193`
	`194`	`+ void requestRecreateCells() {`
	`195`	`+ flow.recreateCells();`
	`196`	`+ }`
	`197`	`+`
`194`	`198`	`}`