Manage MessageContentModels properly so we don't leak memory.

- Manage MessageContentModels properly so we don't leak memory creating new ones every time the role is refreshed.
- Parent and reply MessageContentModels to their message to make sure they get cleaned up when the parent is deleted.
- Make sure ReactionModels are cleaned up on room change to stop that list just growing.
This commit is contained in:
James Graham
2024-07-31 17:57:11 +00:00
parent ea2f891533
commit 37d6033df4
5 changed files with 37 additions and 21 deletions

View File

@@ -31,8 +31,8 @@
using namespace Quotient;
MessageContentModel::MessageContentModel(NeoChatRoom *room, const Quotient::RoomEvent *event, bool isReply, bool isPending)
: QAbstractListModel(nullptr)
MessageContentModel::MessageContentModel(NeoChatRoom *room, const Quotient::RoomEvent *event, bool isReply, bool isPending, MessageContentModel *parent)
: QAbstractListModel(parent)
, m_room(room)
, m_eventId(event != nullptr ? event->id() : QString())
, m_eventSenderId(event != nullptr ? event->senderId() : QString())
@@ -43,8 +43,8 @@ MessageContentModel::MessageContentModel(NeoChatRoom *room, const Quotient::Room
initializeModel();
}
MessageContentModel::MessageContentModel(NeoChatRoom *room, const QString &eventId, bool isReply, bool isPending)
: QAbstractListModel(nullptr)
MessageContentModel::MessageContentModel(NeoChatRoom *room, const QString &eventId, bool isReply, bool isPending, MessageContentModel *parent)
: QAbstractListModel(parent)
, m_room(room)
, m_eventId(eventId)
, m_isPending(isPending)
@@ -58,15 +58,16 @@ void MessageContentModel::initializeModel()
Q_ASSERT(m_room != nullptr);
// Allow making a model for an event that is being downloaded but will appear later
// e.g. a reply, but we need an ID to know when it has arrived.
Q_ASSERT(!m_eventId.isEmpty());
// Also note that a pending event may not have an event ID yet but as long as we have an event
// pointer we can pass out the transaction ID until it is set.
Q_ASSERT(!m_eventId.isEmpty() || m_event != nullptr);
Quotient::connectUntil(m_room.get(), &NeoChatRoom::extraEventLoaded, this, [this](const QString &eventId) {
if (m_room != nullptr) {
if (eventId == m_eventId) {
m_event = loadEvent<RoomEvent>(m_room->getEvent(eventId)->fullJson());
Q_EMIT eventUpdated();
intiializeEvent(m_room->getEvent(eventId));
updateReplyModel();
resetContent();
resetModel();
return true;
}
}
@@ -164,7 +165,12 @@ void MessageContentModel::intiializeEvent(const QString &eventId)
void MessageContentModel::intiializeEvent(const Quotient::RoomEvent *event)
{
m_event = loadEvent<RoomEvent>(event->fullJson());
auto senderId = event->senderId();
// a pending event may not previously have had an event ID so update.
if (m_eventId.isEmpty()) {
m_eventId = m_event->id();
}
auto senderId = m_event->senderId();
// A pending event might not have a sender ID set yet but in that case it must
// be the local member.
if (senderId.isEmpty()) {
@@ -422,9 +428,9 @@ void MessageContentModel::updateReplyModel()
const auto replyEvent = m_room->findInTimeline(eventHandler.getReplyId());
if (replyEvent == m_room->historyEdge()) {
m_replyModel = new MessageContentModel(m_room, eventHandler.getReplyId(), true);
m_replyModel = new MessageContentModel(m_room, eventHandler.getReplyId(), true, false, this);
} else {
m_replyModel = new MessageContentModel(m_room, replyEvent->get(), true);
m_replyModel = new MessageContentModel(m_room, replyEvent->get(), true, false, this);
}
connect(m_replyModel, &MessageContentModel::eventUpdated, this, [this]() {

View File

@@ -75,8 +75,12 @@ public:
};
Q_ENUM(Roles)
explicit MessageContentModel(NeoChatRoom *room, const Quotient::RoomEvent *event, bool isReply = false, bool isPending = false);
MessageContentModel(NeoChatRoom *room, const QString &eventId, bool isReply = false, bool isPending = false);
explicit MessageContentModel(NeoChatRoom *room,
const Quotient::RoomEvent *event,
bool isReply = false,
bool isPending = false,
MessageContentModel *parent = nullptr);
MessageContentModel(NeoChatRoom *room, const QString &eventId, bool isReply = false, bool isPending = false, MessageContentModel *parent = nullptr);
bool showAuthor() const;
void setShowAuthor(bool showAuthor);

View File

@@ -88,7 +88,6 @@ void MessageEventModel::setRoom(NeoChatRoom *room)
// HACK: Reset the model to a null room first to make sure QML dismantles
// last room's objects before the room is actually changed
beginResetModel();
m_readMarkerModels.clear();
m_currentRoom->disconnect(this);
m_currentRoom = nullptr;
endResetModel();
@@ -96,6 +95,9 @@ void MessageEventModel::setRoom(NeoChatRoom *room)
// Don't clear the member objects until the model has been fully reset and all
// refs cleared.
m_memberObjects.clear();
m_contentModels.clear();
m_reactionModels.clear();
m_readMarkerModels.clear();
}
beginResetModel();
@@ -442,13 +444,8 @@ QVariant MessageEventModel::data(const QModelIndex &idx, int role) const
}
if (role == ContentModelRole) {
if (!evt.isStateEvent() && !evt.id().isEmpty()) {
return QVariant::fromValue<MessageContentModel *>(new MessageContentModel(m_currentRoom, &evt));
}
if (evt.isStateEvent()) {
if (evt.matrixType() == QStringLiteral("org.matrix.msc3672.beacon_info")) {
return QVariant::fromValue<MessageContentModel *>(new MessageContentModel(m_currentRoom, &evt));
}
if (m_contentModels.contains(evt.id())) {
return QVariant::fromValue<MessageContentModel *>(m_contentModels.at(evt.id()).get());
}
return {};
}
@@ -634,6 +631,12 @@ void MessageEventModel::createEventObjects(const Quotient::RoomEvent *event)
m_memberObjects[senderId] = std::unique_ptr<NeochatRoomMember>(new NeochatRoomMember(m_currentRoom, senderId));
}
if (!m_contentModels.contains(eventId)) {
if (!event->isStateEvent() || event->matrixType() == QStringLiteral("org.matrix.msc3672.beacon_info")) {
m_contentModels[eventId] = std::unique_ptr<MessageContentModel>(new MessageContentModel(m_currentRoom, event));
}
}
// ReadMarkerModel handles updates to add and remove markers, we only need to
// handle adding and removing whole models here.
if (m_readMarkerModels.contains(eventId)) {

View File

@@ -8,6 +8,7 @@
#include <QQmlEngine>
#include "linkpreviewer.h"
#include "messagecontentmodel.h"
#include "neochatroom.h"
#include "neochatroommember.h"
#include "pollhandler.h"
@@ -117,6 +118,7 @@ private:
KFormat m_format;
std::map<QString, std::unique_ptr<NeochatRoomMember>> m_memberObjects;
std::map<QString, std::unique_ptr<MessageContentModel>> m_contentModels;
QMap<QString, QSharedPointer<ReadMarkerModel>> m_readMarkerModels;
QMap<QString, QSharedPointer<ReactionModel>> m_reactionModels;