TableView: don't rebuild everything if row count changed

As it stood, TableView would rebuild the whole table if
we got a call to forceLayout before a pending
rowsInsertedCallback was delivered. This was far too
harsh, as we a RebuildOption::All will throw away all
existing delegates, and reset the contentItem back
to zero.

The correct thing to do, is to do the same that we
already do in rowsInsertedCallback; just schedule
RebuildOption::ViewportOnly. But since we don't
know what else might have changed, we now also need
to take the previous "else" case path in the function.

Pick-to: 6.5 6.2
Task-number: QTBUG-118897
Change-Id: I23fe30c7d9e6abf32a6565c18bd9249de459636c
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: Santhosh Kumar <santhosh.kumar.selvaraj@qt.io>
(cherry picked from commit a4ed13098d3995ab59e66d3ff5011e2b2014978f)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

2 files changed, 23 insertions, 17 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 5c1355de9c..d986f59d3a 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -2477,21 +2477,22 @@ void QQuickTableViewPrivate::forceLayout(bool immediate)
 
     const QSize actualTableSize = calculateTableSize();
     if (tableSize != actualTableSize) {
-        // This can happen if the app is calling forceLayout while
-        // the model is updated, but before we're notified about it.
-        rebuildOptions = RebuildOption::All;
-    } else {
-        // Resizing a column (or row) can result in the table going from being
-        // e.g completely inside the viewport to go outside. And in the latter
-        // case, the user needs to be able to scroll the viewport, also if
-        // flags such as Flickable.StopAtBounds is in use. So we need to
-        // update contentWidth/Height to support that case.
-        rebuildOptions = RebuildOption::LayoutOnly
-                | RebuildOption::CalculateNewContentWidth
-                | RebuildOption::CalculateNewContentHeight
-                | checkForVisibilityChanges();
+        // The table size will have changed if forceLayout is called after
+        // the row count in the model has changed, but before we received
+        // a rowsInsertedCallback about it (and vice versa for columns).
+        rebuildOptions |= RebuildOption::ViewportOnly;
     }
 
+    // Resizing a column (or row) can result in the table going from being
+    // e.g completely inside the viewport to go outside. And in the latter
+    // case, the user needs to be able to scroll the viewport, also if
+    // flags such as Flickable.StopAtBounds is in use. So we need to
+    // update contentWidth/Height to support that case.
+    rebuildOptions |= RebuildOption::LayoutOnly
+            | RebuildOption::CalculateNewContentWidth
+            | RebuildOption::CalculateNewContentHeight
+            | checkForVisibilityChanges();
+
     scheduleRebuildTable(rebuildOptions);
 
     if (immediate) {
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index c4dc59693f..215966e4f0 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -104,7 +104,7 @@ private slots:
     void isColumnLoadedAndIsRowLoaded();
     void checkForceLayoutFunction();
     void checkForceLayoutEndUpDoingALayout();
-    void checkForceLayoutDuringModelChange();
+    void checkForceLayoutInbetweenAddingRowsToModel();
     void checkForceLayoutWhenAllItemsAreHidden();
     void checkLayoutChangedSignal();
     void checkContentWidthAndHeight();
@@ -827,10 +827,11 @@ void tst_QQuickTableView::checkForceLayoutEndUpDoingALayout()
     QCOMPARE(tableView->contentHeight(), (9 * (newDelegateSize + rowSpacing)) - rowSpacing);
 }
 
-void tst_QQuickTableView::checkForceLayoutDuringModelChange()
+void tst_QQuickTableView::checkForceLayoutInbetweenAddingRowsToModel()
 {
-    // Check that TableView doesn't assert if we call
-    // forceLayout() in the middle of a model change.
+    // Check that TableView doesn't assert if we call forceLayout() while waiting
+    // for a callback from the model that the row count has changed. Also make sure
+    // that we don't move the contentItem while doing so.
     LOAD_TABLEVIEW("plaintableview.qml");
 
     const int initialRowCount = 10;
@@ -845,9 +846,13 @@ void tst_QQuickTableView::checkForceLayoutDuringModelChange()
 
     WAIT_UNTIL_POLISHED;
 
+    const int contentY = 10;
+    tableView->setContentY(contentY);
     QCOMPARE(tableView->rows(), initialRowCount);
+    QCOMPARE(tableView->contentY(), contentY);
     model.addRow(0);
     QCOMPARE(tableView->rows(), initialRowCount + 1);
+    QCOMPARE(tableView->contentY(), contentY);
 }
 
 void tst_QQuickTableView::checkForceLayoutWhenAllItemsAreHidden()