status-im
/
status-desktop
mirror of https://github.com/status-im/status-desktop.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix(ConcatModel): Changed behavior on submodel destruction to not emit signals 

The general rule should be to avoid any actions on destruction different
than setting relevant pointers to null. Other actions like emiting
signals are potentially dangerous when the whole objects hierarchy is
under destruction.

Closes: #13065
This commit is contained in:
Michał Cieślak 2023-12-21 09:59:19 +01:00 committed by Michał
parent 790add8ea5
commit 6fe2067c22
3 changed files with 21 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
ui/StatusQ/include/StatusQ/concatmodel.h
View File

@ -3,6 +3,7 @@
#include <QAbstractListModel>
#include <QQmlListProperty>
#include <QQmlParserStatus>
#include <QPointer>
#include <unordered_map>
@ -27,13 +28,11 @@ public:
signals:
void modelAboutToBeChanged();
void modelChanged(bool deleted);
void modelChanged();
void markerRoleValueChanged();
private:
void onModelDestroyed();
QAbstractItemModel* m_model = nullptr;
QPointer<QAbstractItemModel> m_model;
QString m_markerRoleValue;
};

22
ui/StatusQ/src/concatmodel.cpp
View File

@ -90,18 +90,11 @@ void SourceModel::setModel(QAbstractItemModel* model)
if (m_model == model)
return;
if (model)
connect(model, &QObject::destroyed, this, &SourceModel::onModelDestroyed);
if (m_model)
disconnect(m_model, &QObject::destroyed, this, &SourceModel::onModelDestroyed);
emit modelAboutToBeChanged();
m_model = model;
emit modelChanged(false);
emit modelChanged();
}
/*!
\qmlproperty any StatusQ::SourceModel::model
@ -133,12 +126,6 @@ const QString& SourceModel::markerRoleValue() const
return m_markerRoleValue;
}
void SourceModel::onModelDestroyed()
{
m_model = nullptr;
emit modelChanged(true);
}
ConcatModel::ConcatModel(QObject* parent)
: QAbstractListModel{parent}
@ -350,15 +337,10 @@ void ConcatModel::componentComplete()
});
connect(source, &SourceModel::modelChanged, this,
[this, source, i](bool deleted)
[this, source, i]()
{
auto previousRowCount = m_rowCounts[i];
if (deleted) {
auto prefix = countPrefix(i);
beginRemoveRows({}, prefix, prefix + previousRowCount - 1);
}
if (previousRowCount) {
m_rowCounts[i] = 0;
endRemoveRows();

49
ui/StatusQ/tests/tst_ConcatModel.cpp
View File

@ -1379,43 +1379,26 @@ private slots:
QSignalSpy rowsAboutToBeRemovedSpy(&model, &ConcatModel::rowsAboutToBeRemoved);
QSignalSpy rowsRemovedSpy(&model, &ConcatModel::rowsRemoved);
// checking validity inside rowsAboutToBeRemoved signal
{
QObject context;
connect(&model, &ConcatModel::rowsAboutToBeRemoved, &context,
[this, &model, &roles] {
QCOMPARE(model.rowCount(), 6);
sourceModel2.reset();
QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "key")), 1);
QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "key")), 2);
QCOMPARE(model.data(model.index(2, 0), roleForName(roles, "key")), {});
QCOMPARE(model.data(model.index(3, 0), roleForName(roles, "key")), {});
QCOMPARE(model.data(model.index(4, 0), roleForName(roles, "key")), 5);
QCOMPARE(model.data(model.index(5, 0), roleForName(roles, "key")), 6);
QCOMPARE(model.rowCount(), 6);
QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(2, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(3, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(4, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(5, 0), roleForName(roles, "whichModel")), "");
});
QCOMPARE(rowsAboutToBeRemovedSpy.count(), 0);
QCOMPARE(rowsRemovedSpy.count(), 0);
sourceModel2.reset();
}
QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "key")), 1);
QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "key")), 2);
QCOMPARE(model.data(model.index(2, 0), roleForName(roles, "key")), {});
QCOMPARE(model.data(model.index(3, 0), roleForName(roles, "key")), {});
QCOMPARE(model.data(model.index(4, 0), roleForName(roles, "key")), 5);
QCOMPARE(model.data(model.index(5, 0), roleForName(roles, "key")), 6);
QCOMPARE(model.rowCount(), 4);
QCOMPARE(rowsAboutToBeRemovedSpy.count(), 1);
QCOMPARE(rowsRemovedSpy.count(), 1);
QCOMPARE(rowsAboutToBeRemovedSpy.at(0).at(0), QModelIndex{});
QCOMPARE(rowsAboutToBeRemovedSpy.at(0).at(1), 2);
QCOMPARE(rowsAboutToBeRemovedSpy.at(0).at(2), 3);
QCOMPARE(rowsRemovedSpy.at(0).at(0), QModelIndex{});
QCOMPARE(rowsRemovedSpy.at(0).at(1), 2);
QCOMPARE(rowsRemovedSpy.at(0).at(2), 3);
QCOMPARE(model.data(model.index(0, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(1, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(2, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(3, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(4, 0), roleForName(roles, "whichModel")), "");
QCOMPARE(model.data(model.index(5, 0), roleForName(roles, "whichModel")), "");
}
void removalTest()