diff --git a/src/corelib/itemmodels/qitemselectionmodel.cpp b/src/corelib/itemmodels/qitemselectionmodel.cpp index 1f32519fd6..710f07c3bd 100644 --- a/src/corelib/itemmodels/qitemselectionmodel.cpp +++ b/src/corelib/itemmodels/qitemselectionmodel.cpp @@ -550,52 +550,55 @@ void QItemSelection::split(const QItemSelectionRange &range, void QItemSelectionModelPrivate::initModel(QAbstractItemModel *m) { - static constexpr auto connections = qOffsetStringArray( - QT_STRINGIFY_SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)), - QT_STRINGIFY_SLOT(_q_rowsAboutToBeRemoved(QModelIndex,int,int)), - QT_STRINGIFY_SIGNAL(columnsAboutToBeRemoved(QModelIndex,int,int)), - QT_STRINGIFY_SLOT(_q_columnsAboutToBeRemoved(QModelIndex,int,int)), - QT_STRINGIFY_SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)), - QT_STRINGIFY_SLOT(_q_rowsAboutToBeInserted(QModelIndex,int,int)), - QT_STRINGIFY_SIGNAL(columnsAboutToBeInserted(QModelIndex,int,int)), - QT_STRINGIFY_SLOT(_q_columnsAboutToBeInserted(QModelIndex,int,int)), - QT_STRINGIFY_SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), - QT_STRINGIFY_SLOT(_q_layoutAboutToBeChanged()), - QT_STRINGIFY_SIGNAL(columnsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), - QT_STRINGIFY_SLOT(_q_layoutAboutToBeChanged()), - QT_STRINGIFY_SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)), - QT_STRINGIFY_SLOT(_q_layoutChanged()), - QT_STRINGIFY_SIGNAL(columnsMoved(QModelIndex,int,int,QModelIndex,int)), - QT_STRINGIFY_SLOT(_q_layoutChanged()), - QT_STRINGIFY_SIGNAL(layoutAboutToBeChanged(QList,QAbstractItemModel::LayoutChangeHint)), - QT_STRINGIFY_SLOT(_q_layoutAboutToBeChanged(QList,QAbstractItemModel::LayoutChangeHint)), - QT_STRINGIFY_SIGNAL(layoutChanged(QList,QAbstractItemModel::LayoutChangeHint)), - QT_STRINGIFY_SLOT(_q_layoutChanged(QList,QAbstractItemModel::LayoutChangeHint)), - QT_STRINGIFY_SIGNAL(modelReset()), - QT_STRINGIFY_SLOT(reset()), - QT_STRINGIFY_SIGNAL(destroyed(QObject*)), - QT_STRINGIFY_SLOT(_q_modelDestroyed()) - ); - + Q_Q(QItemSelectionModel); if (model == m) return; - Q_Q(QItemSelectionModel); - if (model.value()) { - for (int i = 0; i < connections.count(); i += 2) - QObject::disconnect(model.value(), connections.at(i), q, connections.at(i + 1)); - q->reset(); - } + if (model) + disconnectModel(); // Caller has to call notify(), unless calling during construction (the common case). model.setValueBypassingBindings(m); if (model.value()) { - for (int i = 0; i < connections.count(); i += 2) - QObject::connect(model.value(), connections.at(i), q, connections.at(i + 1)); + connections = std::array { + QObjectPrivate::connect(model, &QAbstractItemModel::rowsAboutToBeRemoved, + this, &QItemSelectionModelPrivate::rowsAboutToBeRemoved), + QObjectPrivate::connect(model, &QAbstractItemModel::columnsAboutToBeRemoved, + this, &QItemSelectionModelPrivate::columnsAboutToBeRemoved), + QObjectPrivate::connect(model, &QAbstractItemModel::rowsAboutToBeInserted, + this, &QItemSelectionModelPrivate::rowsAboutToBeInserted), + QObjectPrivate::connect(model, &QAbstractItemModel::columnsAboutToBeInserted, + this, &QItemSelectionModelPrivate::columnsAboutToBeInserted), + QObjectPrivate::connect(model, &QAbstractItemModel::rowsAboutToBeMoved, + this, &QItemSelectionModelPrivate::triggerLayoutToBeChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::columnsAboutToBeMoved, + this, &QItemSelectionModelPrivate::triggerLayoutToBeChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::rowsMoved, + this, &QItemSelectionModelPrivate::triggerLayoutChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::columnsMoved, + this, &QItemSelectionModelPrivate::triggerLayoutChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::layoutAboutToBeChanged, + this, &QItemSelectionModelPrivate::layoutAboutToBeChanged), + QObjectPrivate::connect(model, &QAbstractItemModel::layoutChanged, + this, &QItemSelectionModelPrivate::layoutChanged), + QObject::connect(model, &QAbstractItemModel::modelReset, + q, &QItemSelectionModel::reset), + QObjectPrivate::connect(model, &QAbstractItemModel::destroyed, + this, &QItemSelectionModelPrivate::modelDestroyed)}; } } +void QItemSelectionModelPrivate::disconnectModel() +{ + Q_Q(QItemSelectionModel); + for (auto &connection : connections) { + QObject::disconnect(connection); + connection = QMetaObject::Connection(); + } + q->reset(); +} + /*! \internal @@ -638,7 +641,7 @@ QItemSelection QItemSelectionModelPrivate::expandSelection(const QItemSelection /*! \internal */ -void QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(const QModelIndex &parent, +void QItemSelectionModelPrivate::rowsAboutToBeRemoved(const QModelIndex &parent, int start, int end) { Q_Q(QItemSelectionModel); @@ -721,7 +724,7 @@ void QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(const QModelIndex &pare /*! \internal */ -void QItemSelectionModelPrivate::_q_columnsAboutToBeRemoved(const QModelIndex &parent, +void QItemSelectionModelPrivate::columnsAboutToBeRemoved(const QModelIndex &parent, int start, int end) { Q_Q(QItemSelectionModel); @@ -758,7 +761,7 @@ void QItemSelectionModelPrivate::_q_columnsAboutToBeRemoved(const QModelIndex &p Split selection ranges if columns are about to be inserted in the middle. */ -void QItemSelectionModelPrivate::_q_columnsAboutToBeInserted(const QModelIndex &parent, +void QItemSelectionModelPrivate::columnsAboutToBeInserted(const QModelIndex &parent, int start, int end) { Q_UNUSED(end); @@ -788,7 +791,7 @@ void QItemSelectionModelPrivate::_q_columnsAboutToBeInserted(const QModelIndex & Split selection ranges if rows are about to be inserted in the middle. */ -void QItemSelectionModelPrivate::_q_rowsAboutToBeInserted(const QModelIndex &parent, +void QItemSelectionModelPrivate::rowsAboutToBeInserted(const QModelIndex &parent, int start, int end) { Q_Q(QItemSelectionModel); @@ -829,7 +832,8 @@ void QItemSelectionModelPrivate::_q_rowsAboutToBeInserted(const QModelIndex &par preparation for the layoutChanged() signal, where the indexes can be merged again. */ -void QItemSelectionModelPrivate::_q_layoutAboutToBeChanged(const QList &, QAbstractItemModel::LayoutChangeHint hint) +void QItemSelectionModelPrivate::layoutAboutToBeChanged(const QList &, + QAbstractItemModel::LayoutChangeHint hint) { savedPersistentIndexes.clear(); savedPersistentCurrentIndexes.clear(); @@ -976,7 +980,7 @@ static QItemSelection mergeIndexes(const QList &indexes) /*! \internal - Sort predicate function for QItemSelectionModelPrivate::_q_layoutChanged(), + Sort predicate function for QItemSelectionModelPrivate::layoutChanged(), sorting by parent first in addition to operator<(). This is to prevent fragmentation of the selection by grouping indexes with the same row, column of different parents next to each other, which may happen when a selection @@ -994,7 +998,7 @@ static bool qt_PersistentModelIndexLessThan(const QPersistentModelIndex &i1, con Merge the selected indexes into selection ranges again. */ -void QItemSelectionModelPrivate::_q_layoutChanged(const QList &, QAbstractItemModel::LayoutChangeHint hint) +void QItemSelectionModelPrivate::layoutChanged(const QList &, QAbstractItemModel::LayoutChangeHint hint) { // special case for when all indexes are selected if (tableSelected && tableColCount == model->columnCount(tableParent) @@ -1078,9 +1082,10 @@ void QItemSelectionModelPrivate::_q_layoutChanged(const QListranges is non-empty when the source model is reset the persistent indexes // it contains will be invalid. We can't clear them in a modelReset slot because that might already // be too late if another model observer is connected to the same modelReset slot and is invoked first - // it might call select() on this selection model before any such QItemSelectionModelPrivate::_q_modelReset() slot + // it might call select() on this selection model before any such QItemSelectionModelPrivate::modelReset() slot // is invoked, so it would not be cleared yet. We clear it invalid ranges in it here. d->ranges.removeIf(QtFunctionObjects::IsNotValid()); @@ -1883,7 +1888,6 @@ void QItemSelectionModel::setModel(QAbstractItemModel *model) d->model.removeBindingUnlessInWrapper(); if (d->model == model) return; - d->initModel(model); d->model.notify(); } diff --git a/src/corelib/itemmodels/qitemselectionmodel.h b/src/corelib/itemmodels/qitemselectionmodel.h index 4237e7f74f..a19923a90b 100644 --- a/src/corelib/itemmodels/qitemselectionmodel.h +++ b/src/corelib/itemmodels/qitemselectionmodel.h @@ -165,13 +165,6 @@ protected: private: Q_DISABLE_COPY(QItemSelectionModel) - Q_PRIVATE_SLOT(d_func(), void _q_columnsAboutToBeRemoved(const QModelIndex&, int, int)) - Q_PRIVATE_SLOT(d_func(), void _q_rowsAboutToBeRemoved(const QModelIndex&, int, int)) - Q_PRIVATE_SLOT(d_func(), void _q_columnsAboutToBeInserted(const QModelIndex&, int, int)) - Q_PRIVATE_SLOT(d_func(), void _q_rowsAboutToBeInserted(const QModelIndex&, int, int)) - Q_PRIVATE_SLOT(d_func(), void _q_layoutAboutToBeChanged(const QList &parents = QList(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoHint)) - Q_PRIVATE_SLOT(d_func(), void _q_layoutChanged(const QList &parents = QList(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoHint)) - Q_PRIVATE_SLOT(d_func(), void _q_modelDestroyed()) }; Q_DECLARE_OPERATORS_FOR_FLAGS(QItemSelectionModel::SelectionFlags) diff --git a/src/corelib/itemmodels/qitemselectionmodel_p.h b/src/corelib/itemmodels/qitemselectionmodel_p.h index 0f5fd30b28..d3138971d6 100644 --- a/src/corelib/itemmodels/qitemselectionmodel_p.h +++ b/src/corelib/itemmodels/qitemselectionmodel_p.h @@ -18,6 +18,7 @@ #include "qitemselectionmodel.h" #include "private/qobject_p.h" #include "private/qproperty_p.h" +#include QT_REQUIRE_CONFIG(itemmodel); @@ -36,13 +37,23 @@ public: void initModel(QAbstractItemModel *model); - void _q_rowsAboutToBeRemoved(const QModelIndex &parent, int start, int end); - void _q_columnsAboutToBeRemoved(const QModelIndex &parent, int start, int end); - void _q_rowsAboutToBeInserted(const QModelIndex &parent, int start, int end); - void _q_columnsAboutToBeInserted(const QModelIndex &parent, int start, int end); - void _q_layoutAboutToBeChanged(const QList &parents = QList(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoLayoutChangeHint); - void _q_layoutChanged(const QList &parents = QList(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoLayoutChangeHint); - void _q_modelDestroyed(); + void rowsAboutToBeRemoved(const QModelIndex &parent, int start, int end); + void columnsAboutToBeRemoved(const QModelIndex &parent, int start, int end); + void rowsAboutToBeInserted(const QModelIndex &parent, int start, int end); + void columnsAboutToBeInserted(const QModelIndex &parent, int start, int end); + void layoutAboutToBeChanged(const QList &parents, QAbstractItemModel::LayoutChangeHint hint); + void triggerLayoutToBeChanged() + { + layoutAboutToBeChanged(QList(), QAbstractItemModel::NoLayoutChangeHint); + } + + void layoutChanged(const QList &parents, QAbstractItemModel::LayoutChangeHint hint); + void triggerLayoutChanged() + { + layoutChanged(QList(), QAbstractItemModel::NoLayoutChangeHint); + } + + void modelDestroyed(); inline void remove(QList &r) { @@ -59,7 +70,8 @@ public: } void setModel(QAbstractItemModel *mod) { q_func()->setModel(mod); } - void modelChanged(QAbstractItemModel *mod) { q_func()->modelChanged(mod); } + void disconnectModel(); + void modelChanged(QAbstractItemModel *mod) { emit q_func()->modelChanged(mod); } Q_OBJECT_COMPAT_PROPERTY_WITH_ARGS(QItemSelectionModelPrivate, QAbstractItemModel *, model, &QItemSelectionModelPrivate::setModel, &QItemSelectionModelPrivate::modelChanged, nullptr) @@ -76,6 +88,7 @@ public: bool tableSelected; QPersistentModelIndex tableParent; int tableColCount, tableRowCount; + std::array connections; }; QT_END_NAMESPACE diff --git a/tests/auto/corelib/itemmodels/qitemselectionmodel/tst_qitemselectionmodel.cpp b/tests/auto/corelib/itemmodels/qitemselectionmodel/tst_qitemselectionmodel.cpp index 2d09b01c26..a14336e119 100644 --- a/tests/auto/corelib/itemmodels/qitemselectionmodel/tst_qitemselectionmodel.cpp +++ b/tests/auto/corelib/itemmodels/qitemselectionmodel/tst_qitemselectionmodel.cpp @@ -80,7 +80,10 @@ private slots: void QTBUG93305(); + void testSignalsDisconnection(); + private: + static void messageHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg); QAbstractItemModel *model; QItemSelectionModel *selection; }; @@ -2917,5 +2920,37 @@ void tst_QItemSelectionModel::QTBUG93305() QCOMPARE(spy.size(), 4); } +static void (*oldMessageHandler)(QtMsgType, const QMessageLogContext&, const QString&); +static bool signalError = false; + +// detect disconnect warning: +// qt.core.qobject.connect: QObject::disconnect: No such signal +void tst_QItemSelectionModel::messageHandler(QtMsgType type, const QMessageLogContext &context, const QString &msg) +{ + Q_ASSERT(oldMessageHandler); + + if (type == QtWarningMsg + && QString(context.category) == "qt.core.qobject.connect" + && msg.contains("No such")) { + signalError = true; + } + + return oldMessageHandler(type, context, msg); +} + +void tst_QItemSelectionModel::testSignalsDisconnection() +{ + oldMessageHandler = qInstallMessageHandler(messageHandler); + auto resetMessageHandler = qScopeGuard([] { qInstallMessageHandler(oldMessageHandler); }); + auto *newModel = new QStandardItemModel(model); + selection->setModel(newModel); + QSignalSpy spy(newModel, &QObject::destroyed); + delete newModel; + QTRY_COMPARE(spy.count(), 1); + qDebug() << spy; + selection->setModel(nullptr); + QVERIFY(!signalError); +} + QTEST_MAIN(tst_QItemSelectionModel) #include "tst_qitemselectionmodel.moc"