From 2868899d814e80679ac44aa08977b73b0f236692 Mon Sep 17 00:00:00 2001 From: John Preston Date: Wed, 3 Jan 2018 12:06:02 +0300 Subject: [PATCH] Fix possible assertion violation. Allow removing local HistoryItem's after the album was already sent. --- Telegram/SourceFiles/apiwrap.cpp | 10 +++++++++- Telegram/SourceFiles/app.cpp | 28 ++++++++++++++++---------- Telegram/SourceFiles/mainwidget.cpp | 31 +++++++++++++++++------------ 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/Telegram/SourceFiles/apiwrap.cpp b/Telegram/SourceFiles/apiwrap.cpp index 070ef68dd1..0933395f34 100644 --- a/Telegram/SourceFiles/apiwrap.cpp +++ b/Telegram/SourceFiles/apiwrap.cpp @@ -2960,7 +2960,15 @@ void ApiWrap::sendAlbumWithCancelled( const MessageGroupId &groupId) { const auto localId = item->fullId(); const auto albumIt = _sendingAlbums.find(groupId.raw()); - Assert(albumIt != _sendingAlbums.end()); + if (albumIt != _sendingAlbums.end()) { + // Sometimes we destroy item being sent already after the album + // was sent successfully. For example the message could be loaded + // from server (by messages.getHistory or updateNewMessage) and + // added to history and after that updateMessageID was received with + // the same message id, in this case we destroy a detached local + // item and sendAlbumWithCancelled is called for already sent album. + return; + } const auto &album = albumIt->second; const auto proj = [](const SendingAlbum::Item &item) { diff --git a/Telegram/SourceFiles/app.cpp b/Telegram/SourceFiles/app.cpp index bd97e7d5e0..b6d93ff5b3 100644 --- a/Telegram/SourceFiles/app.cpp +++ b/Telegram/SourceFiles/app.cpp @@ -1061,29 +1061,35 @@ namespace { } void feedWereDeleted(ChannelId channelId, const QVector &msgsIds) { - MsgsData *data = fetchMsgsData(channelId, false); + const auto data = fetchMsgsData(channelId, false); if (!data) return; - ChannelHistory *channelHistory = (channelId == NoChannel) ? 0 : App::historyLoaded(peerFromChannel(channelId))->asChannelHistory(); + const auto channelHistory = (channelId != NoChannel) + ? App::history(peerFromChannel(channelId))->asChannelHistory() + : nullptr; - QMap historiesToCheck; - for (QVector::const_iterator i = msgsIds.cbegin(), e = msgsIds.cend(); i != e; ++i) { - MsgsData::const_iterator j = data->constFind(i->v); + base::flat_set> historiesToCheck; + for (const auto msgId : msgsIds) { + auto j = data->constFind(msgId.v); if (j != data->cend()) { - History *h = (*j)->history(); + const auto h = (*j)->history(); (*j)->destroy(); - if (!h->lastMsg) historiesToCheck.insert(h, true); + if (!h->lastMsg) { + historiesToCheck.emplace(h); + } } else { if (channelHistory) { - if (channelHistory->unreadCount() > 0 && i->v >= channelHistory->inboxReadBefore) { - channelHistory->setUnreadCount(channelHistory->unreadCount() - 1); + if (channelHistory->unreadCount() > 0 + && msgId.v >= channelHistory->inboxReadBefore) { + channelHistory->setUnreadCount( + channelHistory->unreadCount() - 1); } } } } if (main()) { - for (QMap::const_iterator i = historiesToCheck.cbegin(), e = historiesToCheck.cend(); i != e; ++i) { - main()->checkPeerHistory(i.key()->peer); + for (const auto history : historiesToCheck) { + main()->checkPeerHistory(history->peer); } } } diff --git a/Telegram/SourceFiles/mainwidget.cpp b/Telegram/SourceFiles/mainwidget.cpp index 77c470993b..7a69bfd879 100644 --- a/Telegram/SourceFiles/mainwidget.cpp +++ b/Telegram/SourceFiles/mainwidget.cpp @@ -4925,24 +4925,29 @@ void MainWidget::feedUpdate(const MTPUpdate &update) { } break; case mtpc_updateMessageID: { - auto &d = update.c_updateMessageID(); - auto msg = App::histItemByRandom(d.vrandom_id.v); - if (msg.msg) { - if (auto msgRow = App::histItemById(msg)) { - if (App::histItemById(msg.channel, d.vid.v)) { - auto history = msgRow->history(); - auto wasLast = (history->lastMsg == msgRow); - msgRow->destroy(); + const auto &d = update.c_updateMessageID(); + if (const auto fullId = App::histItemByRandom(d.vrandom_id.v)) { + const auto channel = fullId.channel; + const auto newId = d.vid.v; + if (const auto local = App::histItemById(fullId)) { + const auto existing = App::histItemById(channel, newId); + if (existing && local->detached()) { + const auto history = local->history(); + const auto wasLast = (history->lastMsg == local); + local->destroy(); if (wasLast && !history->lastMsg) { checkPeerHistory(history->peer); } _history->peerMessagesUpdated(); } else { - App::historyUnregItem(msgRow); - Auth().messageIdChanging.notify({ msgRow, d.vid.v }, true); - msgRow->setId(d.vid.v); - App::historyRegItem(msgRow); - Auth().data().requestItemRepaint(msgRow); + if (existing) { + existing->destroy(); + } + App::historyUnregItem(local); + Auth().messageIdChanging.notify({ local, newId }, true); + local->setId(d.vid.v); + App::historyRegItem(local); + Auth().data().requestItemRepaint(local); } } App::historyUnregRandom(d.vrandom_id.v);