AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QSFPM: Remove data manipulation from move handlers

Browse Source 
Similar to the fix in the parent commit, incorrect updating of the
internal data structures during layout changes can lead to dangling
pointers being dereferenced later.  Moves are treated as layoutChanges
by this proxy by forwarding to the appropriate method.  However, data is
incorrectly cleared prior to that forwarding.  Remove that, and let the
layoutChange handling take appropriate action.

Change-Id: Iee951e37152328a4e6a5fb8e5385c32a2fe4c0bd
Reviewed-by: David Faure <david.faure@kdab.com>
This commit is contained in:
Stephen Kelly 2016-12-19 21:13:57 +00:00
parent 3bd0fd8f97
commit 0874861bcc
2 changed files with 58 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
src/corelib/itemmodels/qsortfilterproxymodel.cpp
View File

@ -1418,49 +1418,27 @@ void QSortFilterProxyModelPrivate::_q_sourceRowsRemoved(
void QSortFilterProxyModelPrivate::_q_sourceRowsAboutToBeMoved(
const QModelIndex &sourceParent, int /* sourceStart */, int /* sourceEnd */, const QModelIndex &destParent, int /* dest */)
{
Q_Q(QSortFilterProxyModel);
// Because rows which are contiguous in the source model might not be contiguous
// in the proxy due to sorting, the best thing we can do here is be specific about what
// parents are having their children changed.
// Optimize: Emit move signals if the proxy is not sorted. Will need to account for rows
// being filtered out though.
saved_persistent_indexes.clear();
QList<QPersistentModelIndex> parents;
parents << q->mapFromSource(sourceParent);
parents << sourceParent;
if (sourceParent != destParent)
parents << q->mapFromSource(destParent);
emit q->layoutAboutToBeChanged(parents);
if (persistent.indexes.isEmpty())
return;
saved_persistent_indexes = store_persistent_indexes();
parents << destParent;
_q_sourceLayoutAboutToBeChanged(parents, QAbstractItemModel::NoLayoutChangeHint);
}
void QSortFilterProxyModelPrivate::_q_sourceRowsMoved(
const QModelIndex &sourceParent, int /* sourceStart */, int /* sourceEnd */, const QModelIndex &destParent, int /* dest */)
{
Q_Q(QSortFilterProxyModel);
// Optimize: We only need to clear and update the persistent indexes which are children of
// sourceParent or destParent
qDeleteAll(source_index_mapping);
source_index_mapping.clear();
update_persistent_indexes(saved_persistent_indexes);
saved_persistent_indexes.clear();
if (dynamic_sortfilter && update_source_sort_column()) {
//update_source_sort_column might have created wrong mapping so we have to clear it again
qDeleteAll(source_index_mapping);
source_index_mapping.clear();
}
QList<QPersistentModelIndex> parents;
parents << q->mapFromSource(sourceParent);
parents << sourceParent;
if (sourceParent != destParent)
parents << q->mapFromSource(destParent);
emit q->layoutChanged(parents);
parents << destParent;
_q_sourceLayoutChanged(parents, QAbstractItemModel::NoLayoutChangeHint);
}
void QSortFilterProxyModelPrivate::_q_sourceColumnsAboutToBeInserted(
@ -1522,42 +1500,21 @@ void QSortFilterProxyModelPrivate::_q_sourceColumnsRemoved(
void QSortFilterProxyModelPrivate::_q_sourceColumnsAboutToBeMoved(
const QModelIndex &sourceParent, int /* sourceStart */, int /* sourceEnd */, const QModelIndex &destParent, int /* dest */)
{
Q_Q(QSortFilterProxyModel);
saved_persistent_indexes.clear();
QList<QPersistentModelIndex> parents;
parents << q->mapFromSource(sourceParent);
parents << sourceParent;
if (sourceParent != destParent)
parents << q->mapFromSource(destParent);
emit q->layoutAboutToBeChanged(parents);
if (persistent.indexes.isEmpty())
return;
saved_persistent_indexes = store_persistent_indexes();
parents << destParent;
_q_sourceLayoutAboutToBeChanged(parents, QAbstractItemModel::NoLayoutChangeHint);
}
void QSortFilterProxyModelPrivate::_q_sourceColumnsMoved(
const QModelIndex &sourceParent, int /* sourceStart */, int /* sourceEnd */, const QModelIndex &destParent, int /* dest */)
{
Q_Q(QSortFilterProxyModel);
qDeleteAll(source_index_mapping);
source_index_mapping.clear();
update_persistent_indexes(saved_persistent_indexes);
saved_persistent_indexes.clear();
if (dynamic_sortfilter && update_source_sort_column()) {
qDeleteAll(source_index_mapping);
source_index_mapping.clear();
}
QList<QPersistentModelIndex> parents;
parents << q->mapFromSource(sourceParent);
parents << sourceParent;
if (sourceParent != destParent)
parents << q->mapFromSource(destParent);
emit q->layoutChanged(parents);
parents << destParent;
_q_sourceLayoutChanged(parents, QAbstractItemModel::NoLayoutChangeHint);
}
/*!

46
tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
View File

@ -146,6 +146,7 @@ private slots:
void filterHint();
void sourceLayoutChangeLeavesValidPersistentIndexes();
void rowMoveLeavesValidPersistentIndexes();
protected:
void buildHierarchy(const QStringList &data, QAbstractItemModel *model);
@ -4307,5 +4308,50 @@ void tst_QSortFilterProxyModel::sourceLayoutChangeLeavesValidPersistentIndexes()
QVERIFY(persistentIndex.parent().isValid());
}
void tst_QSortFilterProxyModel::rowMoveLeavesValidPersistentIndexes()
{
DynamicTreeModel model;
Q_SET_OBJECT_NAME(model);
QList<int> ancestors;
for (auto i = 0; i < 5; ++i)
{
Q_UNUSED(i);
ModelInsertCommand insertCommand(&model);
insertCommand.setAncestorRowNumbers(ancestors);
insertCommand.setStartRow(0);
insertCommand.setEndRow(0);
insertCommand.doCommand();
ancestors.push_back(0);
}
QSortFilterProxyModel proxy1;
proxy1.setSourceModel(&model);
Q_SET_OBJECT_NAME(proxy1);
proxy1.setFilterRegExp("1|2");
auto item5 = model.match(model.index(0, 0), Qt::DisplayRole, "5", 1, Qt::MatchRecursive).first();
auto item3 = model.match(model.index(0, 0), Qt::DisplayRole, "3", 1, Qt::MatchRecursive).first();
Q_ASSERT(item5.isValid());
Q_ASSERT(item3.isValid());
QPersistentModelIndex persistentIndex = proxy1.match(proxy1.index(0, 0), Qt::DisplayRole, "2", 1, Qt::MatchRecursive).first();
ModelMoveCommand moveCommand(&model, 0);
moveCommand.setAncestorRowNumbers(QList<int>{0, 0, 0, 0});
moveCommand.setStartRow(0);
moveCommand.setEndRow(0);
moveCommand.setDestRow(0);
moveCommand.setDestAncestors(QList<int>{0, 0, 0});
moveCommand.doCommand();
// Calling parent() causes the internalPointer to be used.
// Before fixing QTBUG-47711 (moveRows case), that could be
// a dangling pointer.
QVERIFY(persistentIndex.parent().isValid());
}
QTEST_MAIN(tst_QSortFilterProxyModel)
#include "tst_qsortfilterproxymodel.moc"