TableView: don't emit rows and columns changed while rebuilding

We should be careful about emitting signals while we do a rebuild.
The reason is that the application might listen to those signals
and change Flickable properties (especially contentX/Y) behind
TableView's back.

A bug is seen from this if the application is setting a very
large contentY upon receiving rowsChanged(). In that case
TableView will not detect that it should do a "fast-flick"
directly to the new contentY, but instead start to
loadAndUnloadVisibleEdges() until it reaches the current
viewport. And this can take a really long time, and therefore
block the UI.

This patch will wait to emit rows/columnsChanged until we're
done rebuilding. At that point, it's safe to change properties
such as contentX/Y from the application.

The main usecase this patch is solving, is to be able to
always position the viewport on the last row as new rows
are added to the model. This can now be done by listening
to onRowsChanged or onContentHeightChanged, and position
the viewport at the end. The included auto test will
therefore test this exact use case.

Pick-to: 6.5 6.2
Fixes: QTBUG-118897
Change-Id: I6124fbd0e7097a2bbb89c887fe594c3028726aa7
Reviewed-by: Santhosh Kumar <santhosh.kumar.selvaraj@qt.io>
(cherry picked from commit bff7aca8695e4cacc252d0235dc70cf3a46c032e)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

5 files changed, 184 insertions, 16 deletions

diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 8376e092af..5c1355de9c 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -2773,21 +2773,6 @@ qreal QQuickTableViewPrivate::sizeHintForRow(int row) const
     return rowHeight;
 }
 
-void QQuickTableViewPrivate::updateTableSize()
-{
-    // tableSize is the same as row and column count, and will always
-    // be the same as the number of rows and columns in the model.
-    Q_Q(QQuickTableView);
-
-    const QSize prevTableSize = tableSize;
-    tableSize = calculateTableSize();
-
-    if (prevTableSize.width() != tableSize.width())
-        emit q->columnsChanged();
-    if (prevTableSize.height() != tableSize.height())
-        emit q->rowsChanged();
-}
-
 QSize QQuickTableViewPrivate::calculateTableSize()
 {
     QSize size(0, 0);
@@ -3330,6 +3315,7 @@ void QQuickTableViewPrivate::processRebuildTable()
                 Q_TABLEVIEW_UNREACHABLE(rebuildOptions);
         }
 
+        tableSizeBeforeRebuild = tableSize;
         edgesBeforeRebuild = loadedItems.isEmpty() ? QMargins()
             : QMargins(q->leftColumn(), q->topRow(), q->rightColumn(), q->bottomRow());
     }
@@ -3398,6 +3384,10 @@ void QQuickTableViewPrivate::processRebuildTable()
     }
 
     if (rebuildState == RebuildState::Done) {
+        if (tableSizeBeforeRebuild.width() != tableSize.width())
+            emit q->columnsChanged();
+        if (tableSizeBeforeRebuild.height() != tableSize.height())
+            emit q->rowsChanged();
         if (edgesBeforeRebuild.left() != q->leftColumn())
             emit q->leftColumnChanged();
         if (edgesBeforeRebuild.right() != q->rightColumn())
@@ -3547,7 +3537,7 @@ void QQuickTableViewPrivate::calculateTopLeft(QPoint &topLeftCell, QPointF &topL
 
 void QQuickTableViewPrivate::loadInitialTable()
 {
-    updateTableSize();
+    tableSize = calculateTableSize();
 
     if (positionXAnimation.isRunning()) {
         positionXAnimation.stop();
diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h
index 9ba0713b6e..b1431af009 100644
--- a/src/quick/items/qquicktableview_p_p.h
+++ b/src/quick/items/qquicktableview_p_p.h
@@ -379,6 +379,7 @@ public:
     QPoint selectionEndCell = {-1, -1};
 
     QMargins edgesBeforeRebuild;
+    QSize tableSizeBeforeRebuild;
 
     int currentRow = -1;
     int currentColumn = -1;
diff --git a/tests/auto/quick/qquicktableview/data/positionlast.qml b/tests/auto/quick/qquicktableview/data/positionlast.qml
new file mode 100644
index 0000000000..fc6c36a00a
--- /dev/null
+++ b/tests/auto/quick/qquicktableview/data/positionlast.qml
@@ -0,0 +1,52 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+import QtQuick
+
+Item {
+    width: 300
+    height: 300
+
+    property alias tableView: tableView
+    property bool positionOnRowsChanged: false
+    property bool positionOnColumnsChanged: false
+    property bool positionOnContentHeightChanged: false
+    property bool positionOnContentWidthChanged: false
+
+    TableView {
+        id: tableView
+        anchors.fill: parent
+        clip: true
+
+        delegate: Rectangle {
+            implicitWidth: 100
+            implicitHeight: 100
+            Text {
+                anchors.centerIn: parent
+                text: "row:" + row + "\ncol:" + column
+                font.pixelSize: 10
+            }
+        }
+
+        onRowsChanged: {
+            if (positionOnRowsChanged)
+                positionViewAtRow(rows - 1, TableView.AlignBottom)
+        }
+
+        onColumnsChanged: {
+            if (positionOnColumnsChanged)
+                positionViewAtColumn(columns - 1, TableView.AlignRight)
+        }
+
+        onContentHeightChanged: {
+            if (positionOnContentHeightChanged)
+                positionViewAtRow(rows - 1, TableView.AlignBottom)
+        }
+
+        onContentWidthChanged: {
+            if (positionOnContentWidthChanged)
+                positionViewAtColumn(columns - 1, TableView.AlignRight)
+        }
+    }
+
+}
diff --git a/tests/auto/quick/qquicktableview/testmodel.h b/tests/auto/quick/qquicktableview/testmodel.h
index 71d58d6602..e8caeebac0 100644
--- a/tests/auto/quick/qquicktableview/testmodel.h
+++ b/tests/auto/quick/qquicktableview/testmodel.h
@@ -182,6 +182,11 @@ public:
         insertRow(row, QModelIndex());
     }
 
+    Q_INVOKABLE void addColumn(int column)
+    {
+        insertColumn(column, QModelIndex());
+    }
+
     Qt::ItemFlags flags(const QModelIndex &index) const override
     {
         Q_UNUSED(index)
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index 969bd2b62e..c4dc59693f 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -189,6 +189,10 @@ private slots:
     void positionViewAtCellForLargeCells_data();
     void positionViewAtCellForLargeCells();
     void positionViewAtCellForLargeCellsUsingSubrect();
+    void positionViewAtLastRow_data();
+    void positionViewAtLastRow();
+    void positionViewAtLastColumn_data();
+    void positionViewAtLastColumn();
     void itemAtCell_data();
     void itemAtCell();
     void leftRightTopBottomProperties_data();
@@ -4100,6 +4104,122 @@ void tst_QQuickTableView::positionViewAtCellForLargeCellsUsingSubrect()
     QCOMPARE(tableView->contentY(), -(tableView->height() - subRect.bottom()));
 }
 
+void tst_QQuickTableView::positionViewAtLastRow_data()
+{
+    QTest::addColumn<QString>("signalToTest");
+    QTest::addColumn<bool>("animate");
+
+    QTest::newRow("positionOnRowsChanged, animate=false") << "positionOnRowsChanged" << false;
+    QTest::newRow("positionOnRowsChanged, animate=true") << "positionOnRowsChanged" << true;
+    QTest::newRow("positionOnContentHeightChanged, animate=false") << "positionOnContentHeightChanged" << false;
+    QTest::newRow("positionOnContentHeightChanged, animate=true") << "positionOnContentHeightChanged" << true;
+}
+
+void tst_QQuickTableView::positionViewAtLastRow()
+{
+    // Check that we can make TableView always scroll to the
+    // last row in the model by positioning the view upon
+    // a rowsChanged callback
+    QFETCH(QString, signalToTest);
+    QFETCH(bool, animate);
+
+    LOAD_TABLEVIEW("positionlast.qml");
+
+    // Use a very large model to indirectly test that we "fast-flick" to
+    // the end at start-up (instead of loading and unloading rows, which
+    // would take forever).
+    TestModel model(2000000, 2000000);
+    tableView->setModel(QVariant::fromValue(&model));
+    tableView->setAnimate(animate);
+
+    view->rootObject()->setProperty(signalToTest.toUtf8().constData(), true);
+
+    WAIT_UNTIL_POLISHED;
+
+    const qreal delegateSize = 100.;
+    const qreal viewportRowCount = tableView->height() / delegateSize;
+
+    // Check that the viewport is positioned at the last row at start-up
+    QCOMPARE(tableView->rows(), model.rowCount());
+    QCOMPARE(tableView->bottomRow(), model.rowCount() - 1);
+    QCOMPARE(tableView->contentY(), (model.rowCount() - viewportRowCount) * delegateSize);
+
+    // Check that the viewport is positioned at the last
+    // row after more rows are added.
+    for (int row = 0; row < 2; ++row) {
+        model.addRow(model.rowCount() - 1);
+
+        WAIT_UNTIL_POLISHED;
+
+        if (tableView->animate()) {
+            QVERIFY(tableViewPrivate->positionYAnimation.isRunning());
+            QTRY_VERIFY(!tableViewPrivate->positionYAnimation.isRunning());
+        }
+
+        QCOMPARE(tableView->rows(), model.rowCount());
+        QCOMPARE(tableView->bottomRow(), model.rowCount() - 1);
+        QCOMPARE(tableView->contentY(), (model.rowCount() - viewportRowCount) * delegateSize);
+    }
+}
+
+void tst_QQuickTableView::positionViewAtLastColumn_data()
+{
+    QTest::addColumn<QString>("signalToTest");
+    QTest::addColumn<bool>("animate");
+
+    QTest::newRow("positionOnColumnsChanged, animate=false") << "positionOnColumnsChanged" << false;
+    QTest::newRow("positionOnColumnsChanged, animate=true") << "positionOnColumnsChanged" << true;
+    QTest::newRow("positionOnContentWidthChanged, animate=false") << "positionOnContentWidthChanged" << false;
+    QTest::newRow("positionOnContentWidthChanged, animate=true") << "positionOnContentWidthChanged" << true;
+}
+
+void tst_QQuickTableView::positionViewAtLastColumn()
+{
+    // Check that we can make TableView always scroll to the
+    // last column in the model by positioning the view upon
+    // a columnsChanged callback
+    QFETCH(QString, signalToTest);
+    QFETCH(bool, animate);
+
+    LOAD_TABLEVIEW("positionlast.qml");
+
+    // Use a very large model to indirectly test that we "fast-flick" to
+    // the end at start-up (instead of loading and unloading columns, which
+    // would take forever).
+    TestModel model(2000000, 2000000);
+    tableView->setModel(QVariant::fromValue(&model));
+    tableView->setAnimate(animate);
+
+    view->rootObject()->setProperty(signalToTest.toUtf8().constData(), true);
+
+    WAIT_UNTIL_POLISHED;
+
+    const qreal delegateSize = 100.;
+    const qreal viewportColumnCount = tableView->width() / delegateSize;
+
+    // Check that the viewport is positioned at the last column at start-up
+    QCOMPARE(tableView->columns(), model.columnCount());
+    QCOMPARE(tableView->rightColumn(), model.columnCount() - 1);
+    QCOMPARE(tableView->contentX(), (model.columnCount() - viewportColumnCount) * delegateSize);
+
+    // Check that the viewport is positioned at the last
+    // column after more columns are added.
+    for (int column = 0; column < 2; ++column) {
+        model.addColumn(model.columnCount() - 1);
+
+        WAIT_UNTIL_POLISHED;
+
+        if (tableView->animate()) {
+            QVERIFY(tableViewPrivate->positionXAnimation.isRunning());
+            QTRY_VERIFY(!tableViewPrivate->positionXAnimation.isRunning());
+        }
+
+        QCOMPARE(tableView->columns(), model.columnCount());
+        QCOMPARE(tableView->rightColumn(), model.columnCount() - 1);
+        QCOMPARE(tableView->contentX(), (model.columnCount() - viewportColumnCount) * delegateSize);
+    }
+}
+
 void tst_QQuickTableView::itemAtCell_data()
 {
     QTest::addColumn<QPoint>("cell");