Port of QItemSelectionModel::model to new property system

The property 'model' is ported to a bindable property.
The properties hasSelection, selection, selectedIndexes,
and currentIndex are left for later patches.

Task-number: QTBUG-85520
Change-Id: Ia424ce99fc80c3d807c634c21d161a3ad94b27d2
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>

src/corelib/itemmodels/qitemselectionmodel.cpp

@@ -38,6 +38,8 @@
 ****************************************************************************/
 
 #include "qitemselectionmodel.h"
+#include "qitemselectionmodel_p.h"
+
 #include <private/qitemselectionmodel_p.h>
 #include <private/qduplicatetracker_p.h>
 #include <qdebug.h>
@@ -603,6 +605,8 @@ void QItemSelectionModelPrivate::initModel(QAbstractItemModel *m)
           SLOT(_q_layoutChanged(QList<QPersistentModelIndex>,QAbstractItemModel::LayoutChangeHint)) },
         { SIGNAL(modelReset()),
           SLOT(reset()) },
+        { SIGNAL(destroyed(QObject*)),
+           SLOT(_q_modelDestroyed()) },
         { nullptr, nullptr }
     };
 
@@ -610,15 +614,18 @@ void QItemSelectionModelPrivate::initModel(QAbstractItemModel *m)
         return;
 
     Q_Q(QItemSelectionModel);
-    if (model) {
+    if (model.value()) {
         for (const Cx *cx = &connections[0]; cx->signal; cx++)
-            QObject::disconnect(model, cx->signal, q, cx->slot);
+            QObject::disconnect(model.value(), cx->signal, q, cx->slot);
         q->reset();
     }
-    model = m;
+
-    if (model) {
+    // Caller has to call notify(), unless calling during construction (the common case).
+    model.setValueBypassingBindings(m);
+
+    if (model.value()) {
         for (const Cx *cx = &connections[0]; cx->signal; cx++)
-            QObject::connect(model, cx->signal, q, cx->slot);
+            QObject::connect(model.value(), cx->signal, q, cx->slot);
     }
 }
 
@@ -674,12 +681,16 @@ void QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(const QModelIndex &pare
     if (currentIndex.isValid() && parent == currentIndex.parent()
         && currentIndex.row() >= start && currentIndex.row() <= end) {
         QModelIndex old = currentIndex;
-        if (start > 0) // there are rows left above the change
+        if (start > 0) {
+            // there are rows left above the change
             currentIndex = model->index(start - 1, old.column(), parent);
-        else if (model && end < model->rowCount(parent) - 1) // there are rows left below the change
+        } else if (model.value() && end < model->rowCount(parent) - 1) {
+            // there are rows left below the change
             currentIndex = model->index(end + 1, old.column(), parent);
-        else // there are no rows left in the table
+        } else {
+            // there are no rows left in the table
             currentIndex = QModelIndex();
+        }
         emit q->currentChanged(currentIndex, old);
         emit q->currentRowChanged(currentIndex, old);
         if (currentIndex.column() != old.column())
@@ -744,12 +755,16 @@ void QItemSelectionModelPrivate::_q_columnsAboutToBeRemoved(const QModelIndex &p
     if (currentIndex.isValid() && parent == currentIndex.parent()
         && currentIndex.column() >= start && currentIndex.column() <= end) {
         QModelIndex old = currentIndex;
-        if (start > 0) // there are columns to the left of the change
+        if (start > 0) {
+            // there are columns to the left of the change
             currentIndex = model->index(old.row(), start - 1, parent);
-        else if (model && end < model->columnCount() - 1) // there are columns to the right of the change
+        } else if (model.value() && end < model->columnCount() - 1) {
+            // there are columns to the right of the change
             currentIndex = model->index(old.row(), end + 1, parent);
-        else // there are no columns left in the table
+        } else {
+            // there are no columns left in the table
             currentIndex = QModelIndex();
+        }
         emit q->currentChanged(currentIndex, old);
         if (currentIndex.row() != old.row())
             emit q->currentRowChanged(currentIndex, old);
@@ -1050,6 +1065,39 @@ void QItemSelectionModelPrivate::_q_layoutChanged(const QList<QPersistentModelIn
     }
 }
 
+/*!
+    \internal
+
+    Called when the used model gets destroyed.
+
+    It is impossible to have a correct implementation here.
+    In the following situation, there are two contradicting rules:
+
+    \code
+    QProperty<QAbstractItemModel *> leader(mymodel);
+    QItemSelectionModel myItemSelectionModel;
+    myItemSelectionModel.bindableModel().setBinding([&](){ return leader.value(); }
+    delete mymodel;
+    QAbstractItemModel *returnedModel = myItemSelectionModel.model();
+    \endcode
+
+    What should returnedModel be in this situation?
+
+    Rules for bindable properties say that myItemSelectionModel.model()
+    should return the same as leader.value(), namely the pointer to the now deleted model.
+
+    However, backward compatibility requires myItemSelectionModel.model() to return a
+    nullptr, because that was done in the past after the model used was deleted.
+
+    We decide to break the new rule, imposed by bindable properties, and not break the old
+    rule, because that may break existing code.
+*/
+void QItemSelectionModelPrivate::_q_modelDestroyed()
+{
+    model.setValueBypassingBindings(nullptr);
+    model.notify();
+}
+
 /*!
     \class QItemSelectionModel
     \inmodule QtCore
@@ -1238,7 +1286,7 @@ struct IsNotValid {
 void QItemSelectionModel::select(const QItemSelection &selection, QItemSelectionModel::SelectionFlags command)
 {
     Q_D(QItemSelectionModel);
-    if (!d->model) {
+    if (!d->model.value()) {
         qWarning("QItemSelectionModel: Selecting when no model has been set will result in a no-op.");
         return;
     }
@@ -1343,7 +1391,7 @@ void QItemSelectionModel::clearSelection()
 void QItemSelectionModel::setCurrentIndex(const QModelIndex &index, QItemSelectionModel::SelectionFlags command)
 {
     Q_D(QItemSelectionModel);
-    if (!d->model) {
+    if (!d->model.value()) {
         qWarning("QItemSelectionModel: Setting the current index when no model has been set will result in a no-op.");
         return;
     }
@@ -1425,7 +1473,7 @@ bool QItemSelectionModel::isSelected(const QModelIndex &index) const
 bool QItemSelectionModel::isRowSelected(int row, const QModelIndex &parent) const
 {
     Q_D(const QItemSelectionModel);
-    if (!d->model)
+    if (!d->model.value())
         return false;
     if (parent.isValid() && d->model != parent.model())
         return false;
@@ -1500,7 +1548,7 @@ bool QItemSelectionModel::isRowSelected(int row, const QModelIndex &parent) cons
 bool QItemSelectionModel::isColumnSelected(int column, const QModelIndex &parent) const
 {
     Q_D(const QItemSelectionModel);
-    if (!d->model)
+    if (!d->model.value())
         return false;
     if (parent.isValid() && d->model != parent.model())
         return false;
@@ -1574,7 +1622,7 @@ bool QItemSelectionModel::isColumnSelected(int column, const QModelIndex &parent
 bool QItemSelectionModel::rowIntersectsSelection(int row, const QModelIndex &parent) const
 {
     Q_D(const QItemSelectionModel);
-    if (!d->model)
+    if (!d->model.value())
         return false;
     if (parent.isValid() && d->model != parent.model())
          return false;
@@ -1610,7 +1658,7 @@ bool QItemSelectionModel::rowIntersectsSelection(int row, const QModelIndex &par
 bool QItemSelectionModel::columnIntersectsSelection(int column, const QModelIndex &parent) const
 {
     Q_D(const QItemSelectionModel);
-    if (!d->model)
+    if (!d->model.value())
         return false;
     if (parent.isValid() && d->model != parent.model())
         return false;
@@ -1794,7 +1842,7 @@ const QItemSelection QItemSelectionModel::selection() const
 */
 QAbstractItemModel *QItemSelectionModel::model()
 {
-    return d_func()->model;
+    return d_func()->model.value();
 }
 
 /*!
@@ -1802,7 +1850,12 @@ QAbstractItemModel *QItemSelectionModel::model()
 */
 const QAbstractItemModel *QItemSelectionModel::model() const
 {
-    return d_func()->model;
+    return d_func()->model.value();
+}
+
+QBindable<QAbstractItemModel *> QItemSelectionModel::bindableModel()
+{
+    return &d_func()->model;
 }
 
 /*!
@@ -1815,11 +1868,12 @@ const QAbstractItemModel *QItemSelectionModel::model() const
 void QItemSelectionModel::setModel(QAbstractItemModel *model)
 {
     Q_D(QItemSelectionModel);
+    d->model.removeBindingUnlessInWrapper();
     if (d->model == model)
         return;
 
     d->initModel(model);
-    emit modelChanged(model);
+    d->model.notify();
 }
 
 /*!

src/corelib/itemmodels/qitemselectionmodel.h

@@ -120,7 +120,7 @@ class QItemSelectionModelPrivate;
 class Q_CORE_EXPORT QItemSelectionModel : public QObject
 {
     Q_OBJECT
-    Q_PROPERTY(QAbstractItemModel *model READ model WRITE setModel NOTIFY modelChanged)
+    Q_PROPERTY(QAbstractItemModel *model READ model WRITE setModel NOTIFY modelChanged BINDABLE bindableModel)
     Q_PROPERTY(bool hasSelection READ hasSelection NOTIFY selectionChanged STORED false DESIGNABLE false)
     Q_PROPERTY(QModelIndex currentIndex READ currentIndex NOTIFY currentChanged STORED false DESIGNABLE false)
     Q_PROPERTY(QItemSelection selection READ selection NOTIFY selectionChanged STORED false DESIGNABLE false)
@@ -169,6 +169,7 @@ public:
 
     const QAbstractItemModel *model() const;
     QAbstractItemModel *model();
+    QBindable<QAbstractItemModel *> bindableModel();
 
     void setModel(QAbstractItemModel *model);
 
@@ -201,6 +202,7 @@ private:
     Q_PRIVATE_SLOT(d_func(), void _q_rowsAboutToBeInserted(const QModelIndex&, int, int))
     Q_PRIVATE_SLOT(d_func(), void _q_layoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents = QList<QPersistentModelIndex>(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoHint))
     Q_PRIVATE_SLOT(d_func(), void _q_layoutChanged(const QList<QPersistentModelIndex> &parents = QList<QPersistentModelIndex>(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoHint))
+    Q_PRIVATE_SLOT(d_func(), void _q_modelDestroyed())
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QItemSelectionModel::SelectionFlags)

src/corelib/itemmodels/qitemselectionmodel_p.h

@@ -52,6 +52,7 @@
 //
 
 #include "private/qobject_p.h"
+#include "private/qproperty_p.h"
 
 QT_REQUIRE_CONFIG(itemmodel);
 
@@ -62,8 +63,7 @@ class QItemSelectionModelPrivate: public QObjectPrivate
     Q_DECLARE_PUBLIC(QItemSelectionModel)
 public:
     QItemSelectionModelPrivate()
-      : model(nullptr),
+      : currentCommand(QItemSelectionModel::NoUpdate),
-        currentCommand(QItemSelectionModel::NoUpdate),
         tableSelected(false), tableColCount(0), tableRowCount(0) {}
 
     QItemSelection expandSelection(const QItemSelection &selection,
@@ -77,6 +77,7 @@ public:
     void _q_columnsAboutToBeInserted(const QModelIndex &parent, int start, int end);
     void _q_layoutAboutToBeChanged(const QList<QPersistentModelIndex> &parents = QList<QPersistentModelIndex>(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoLayoutChangeHint);
     void _q_layoutChanged(const QList<QPersistentModelIndex> &parents = QList<QPersistentModelIndex>(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoLayoutChangeHint);
+    void _q_modelDestroyed();
 
     inline void remove(QList<QItemSelectionRange> &r)
     {
@@ -92,7 +93,12 @@ public:
             currentSelection.clear();
     }
 
-    QPointer<QAbstractItemModel> model;
+    void setModel(QAbstractItemModel *mod) { q_func()->setModel(mod); }
+    void modelChanged(QAbstractItemModel *mod) { q_func()->modelChanged(mod); }
+    Q_OBJECT_COMPAT_PROPERTY_WITH_ARGS(QItemSelectionModelPrivate, QAbstractItemModel *, model,
+                                       &QItemSelectionModelPrivate::setModel,
+                                       &QItemSelectionModelPrivate::modelChanged, nullptr)
+
     QItemSelection ranges;
     QItemSelection currentSelection;
     QPersistentModelIndex currentIndex;

tests/auto/corelib/itemmodels/qitemselectionmodel/CMakeLists.txt

@@ -7,6 +7,7 @@
 qt_internal_add_test(tst_qitemselectionmodel
     SOURCES
         tst_qitemselectionmodel.cpp
-    PUBLIC_LIBRARIES
+    LIBRARIES
         Qt::Gui
+        Qt::TestPrivate
 )

tests/auto/corelib/itemmodels/qitemselectionmodel/tst_qitemselectionmodel.cpp

@@ -27,6 +27,7 @@
 ****************************************************************************/
 
 #include <QTest>
+#include <QtTest/private/qpropertytesthelper_p.h>
 #include <QSignalSpy>
 
 #include <QtGui/QtGui>
@@ -85,6 +86,8 @@ private slots:
     void deselectRemovedMiddleRange();
     void setModel();
 
+    void bindableModel();
+
     void testDifferentModels();
 
     void testValidRangesInSelectionsAfterReset();
@@ -2435,6 +2438,29 @@ void tst_QItemSelectionModel::setModel()
     QVERIFY(sel.selection().isEmpty());
 }
 
+void tst_QItemSelectionModel::bindableModel()
+{
+    QItemSelectionModel sel;
+    QVERIFY(!sel.model());
+
+    std::unique_ptr<QStringListModel> firstModel(
+            new QStringListModel(QStringList { "Some", "random", "content" }));
+    std::unique_ptr<QStringListModel> changedModel(
+            new QStringListModel(QStringList { "Other", "random", "content" }));
+
+    QTestPrivate::testReadWritePropertyBasics<QItemSelectionModel, QAbstractItemModel *>(
+            sel, firstModel.get(), changedModel.get(), "model");
+    if (QTest::currentTestFailed()) {
+        qDebug("Failed property test for QItemSelectionModel::model");
+        return;
+    }
+
+    // check that model is set to nullptr when the object pointed to is deleted:
+    sel.setModel(firstModel.get());
+    firstModel.reset();
+    QCOMPARE(sel.model(), nullptr);
+}
+
 void tst_QItemSelectionModel::testDifferentModels()
 {
     QStandardItemModel model1;