From 13afbf7ec6e80a50fe9f6b04f31962de1d0bd0c7 Mon Sep 17 00:00:00 2001 From: Anthony Bilinski Date: Tue, 29 Jan 2019 02:43:49 -0800 Subject: [PATCH] fix(groups): avoid having to lookup peer pk Caused race where peer plays audio, then is removed from group, then we process audio played signal and lookup their peerId in core where it doesn't exist. Now Group will effectively contain the peer until the peer list changed slot is processed. Partial fix for #5511 --- src/core/core.cpp | 8 ++++---- src/core/core.h | 2 +- src/core/coreav.cpp | 2 +- src/model/group.cpp | 13 ------------- src/model/group.h | 2 -- src/widget/widget.cpp | 4 ++-- src/widget/widget.h | 2 +- 7 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index 8461c7758..6d5b82254 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -1175,10 +1175,10 @@ QString Core::getGroupPeerName(int groupId, int peerId) const QByteArray name(length, Qt::Uninitialized); uint8_t* namePtr = reinterpret_cast(name.data()); bool success = tox_conference_peer_get_name(tox.get(), groupId, peerId, namePtr, &error); - if (!parsePeerQueryError(error) || !success) { - qWarning() << "getGroupPeerName: Unknown error"; + if (!parsePeerQueryError(error)) { return QString{}; } + assert(success); return ToxString(name).getQString(); } @@ -1193,10 +1193,10 @@ ToxPk Core::getGroupPeerPk(int groupId, int peerId) const uint8_t friendPk[TOX_PUBLIC_KEY_SIZE] = {0x00}; Tox_Err_Conference_Peer_Query error; bool success = tox_conference_peer_get_public_key(tox.get(), groupId, peerId, friendPk, &error); - if (!parsePeerQueryError(error) || !success) { - qWarning() << "getGroupPeerToxId: Unknown error"; + if (!parsePeerQueryError(error)) { return ToxPk{}; } + assert(success); return ToxPk(friendPk); } diff --git a/src/core/core.h b/src/core/core.h index 26da227b5..d2598463c 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -199,7 +199,7 @@ signals: void groupPeerlistChanged(int groupnumber); void groupPeerNameChanged(int groupnumber, int peernumber, const QString& newName); void groupTitleChanged(int groupnumber, const QString& author, const QString& title); - void groupPeerAudioPlaying(int groupnumber, int peernumber); + void groupPeerAudioPlaying(int groupnumber, ToxPk peerPk); void groupSentFailed(int groupId); void actionSentResult(uint32_t friendId, const QString& action, int success); diff --git a/src/core/coreav.cpp b/src/core/coreav.cpp index cc18634f8..4ea090e57 100644 --- a/src/core/coreav.cpp +++ b/src/core/coreav.cpp @@ -485,7 +485,7 @@ void CoreAV::groupCallCallback(void* tox, uint32_t group, uint32_t peer, const i return; } - emit c->groupPeerAudioPlaying(group, peer); + emit c->groupPeerAudioPlaying(group, peerPk); CoreAV* cav = c->getAv(); diff --git a/src/model/group.cpp b/src/model/group.cpp index bd549b188..e1b715c1b 100644 --- a/src/model/group.cpp +++ b/src/model/group.cpp @@ -82,19 +82,6 @@ QString Group::getDisplayedName() const return getName(); } -/** - * @brief performs a peerId to ToxPk lookup - * @param peerId peerId to lookup - * @return ToxPk if peerId found - * @note should not be used, reference peers by their ToxPk instead - * @todo remove this function - */ -const ToxPk Group::resolvePeerId(int peerId) const -{ - const Core* core = Core::getInstance(); - return core->getGroupPeerPk(groupId, peerId); -} - void Group::regeneratePeerList() { const Core* core = Core::getInstance(); diff --git a/src/model/group.h b/src/model/group.h index c856b2558..dd11e68e4 100644 --- a/src/model/group.h +++ b/src/model/group.h @@ -53,8 +53,6 @@ public: void setTitle(const QString& author, const QString& newTitle); QString getName() const; QString getDisplayedName() const override; - - const ToxPk resolvePeerId(int peerId) const; QString resolveToxId(const ToxPk& id) const; void setSelfName(const QString& name); QString getSelfName() const; diff --git a/src/widget/widget.cpp b/src/widget/widget.cpp index 5f661ae0b..2b811d6b7 100644 --- a/src/widget/widget.cpp +++ b/src/widget/widget.cpp @@ -1810,7 +1810,7 @@ void Widget::onGroupTitleChanged(int groupnumber, const QString& author, const Q widget->searchName(ui->searchContactText->text(), filterGroups(filter)); } -void Widget::onGroupPeerAudioPlaying(int groupnumber, int peernumber) +void Widget::onGroupPeerAudioPlaying(int groupnumber, ToxPk peerPk) { Group* g = GroupList::findGroup(groupnumber); if (!g) { @@ -1819,7 +1819,7 @@ void Widget::onGroupPeerAudioPlaying(int groupnumber, int peernumber) auto form = groupChatForms[g->getId()].data(); // TODO(sudden6): switch to ToxPk here - form->peerAudioPlaying(g->resolvePeerId(peernumber)); + form->peerAudioPlaying(peerPk); } void Widget::removeGroup(Group* g, bool fake) diff --git a/src/widget/widget.h b/src/widget/widget.h index 4129d4e65..6954c6fbf 100644 --- a/src/widget/widget.h +++ b/src/widget/widget.h @@ -175,7 +175,7 @@ public slots: void onGroupPeerlistChanged(int groupnumber); void onGroupPeerNameChanged(int groupnumber, int peernumber, const QString& newName); void onGroupTitleChanged(int groupnumber, const QString& author, const QString& title); - void onGroupPeerAudioPlaying(int groupnumber, int peernumber); + void onGroupPeerAudioPlaying(int groupnumber, ToxPk peerPk); void onGroupSendFailed(int groupId); void onFriendTypingChanged(int friendId, bool isTyping); void nextContact();