From dbb46ce9b0597390d7a52207ec18b53a5fe5608b Mon Sep 17 00:00:00 2001 From: John Preston Date: Tue, 26 May 2020 18:32:38 +0400 Subject: [PATCH] Let [Photo|Document]Media outlive message view. --- Telegram/SourceFiles/data/data_document.cpp | 8 ++------ Telegram/SourceFiles/data/data_photo.cpp | 8 ++------ Telegram/SourceFiles/data/data_reply_preview.cpp | 6 +++++- Telegram/SourceFiles/data/data_session.cpp | 15 +++++++++++++++ Telegram/SourceFiles/data/data_session.h | 5 +++++ .../history/view/history_view_element.cpp | 4 ++-- .../history/view/media/history_view_document.cpp | 2 +- .../history/view/media/history_view_gif.cpp | 10 +++++++--- .../view/media/history_view_media_grouped.cpp | 2 +- .../view/media/history_view_media_unwrapped.cpp | 2 -- .../view/media/history_view_media_unwrapped.h | 2 +- .../history/view/media/history_view_photo.cpp | 3 ++- .../history/view/media/history_view_sticker.cpp | 4 +++- .../view/media/history_view_theme_document.cpp | 2 +- .../history/view/media/history_view_web_page.cpp | 2 +- 15 files changed, 48 insertions(+), 27 deletions(-) diff --git a/Telegram/SourceFiles/data/data_document.cpp b/Telegram/SourceFiles/data/data_document.cpp index 2e8c60d3fd..665cec00ce 100644 --- a/Telegram/SourceFiles/data/data_document.cpp +++ b/Telegram/SourceFiles/data/data_document.cpp @@ -1601,13 +1601,9 @@ void DocumentData::collectLocalData(not_null local) { _owner->cache().copyIfEmpty(local->cacheKey(), cacheKey()); if (const auto localMedia = local->activeMediaView()) { - const auto media = createMediaView(); + auto media = createMediaView(); media->collectLocalData(localMedia.get()); - - // Keep DocumentMedia alive for some more time. - // NB! This allows DocumentMedia to outlive Main::Session! - // In case this is a problem this code should be rewritten. - crl::on_main(&session(), [media] {}); + _owner->keepAlive(std::move(media)); } if (!local->_location.inMediaCache() && !local->_location.isEmpty()) { _location = local->_location; diff --git a/Telegram/SourceFiles/data/data_photo.cpp b/Telegram/SourceFiles/data/data_photo.cpp index bbfbc57e95..18380065a7 100644 --- a/Telegram/SourceFiles/data/data_photo.cpp +++ b/Telegram/SourceFiles/data/data_photo.cpp @@ -230,13 +230,9 @@ void PhotoData::collectLocalData(not_null local) { } } if (const auto localMedia = local->activeMediaView()) { - const auto media = createMediaView(); + auto media = createMediaView(); media->collectLocalData(localMedia.get()); - - // Keep DocumentMedia alive for some more time. - // NB! This allows DocumentMedia to outlive Main::Session! - // In case this is a problem this code should be rewritten. - crl::on_main(&session(), [media] {}); + _owner->keepAlive(std::move(media)); } } diff --git a/Telegram/SourceFiles/data/data_reply_preview.cpp b/Telegram/SourceFiles/data/data_reply_preview.cpp index 989048eadb..b98737fec3 100644 --- a/Telegram/SourceFiles/data/data_reply_preview.cpp +++ b/Telegram/SourceFiles/data/data_reply_preview.cpp @@ -79,7 +79,7 @@ Image *ReplyPreview::image(Data::FileOrigin origin) { prepare(image, option | Images::Option::Blurred); } } - if (thumbnail || !_document->hasThumbnail()) { + if (_good || !_document->hasThumbnail()) { _checked = true; _documentMedia = nullptr; } @@ -101,6 +101,10 @@ Image *ReplyPreview::image(Data::FileOrigin origin) { prepare(blurred, Images::Option::Blurred); } } + if (_good) { + _checked = true; + _photoMedia = nullptr; + } } } return _image.get(); diff --git a/Telegram/SourceFiles/data/data_session.cpp b/Telegram/SourceFiles/data/data_session.cpp index f5d926dc23..8e1370a1d0 100644 --- a/Telegram/SourceFiles/data/data_session.cpp +++ b/Telegram/SourceFiles/data/data_session.cpp @@ -268,6 +268,18 @@ void Session::clear() { _photos.clear(); } +void Session::keepAlive(std::shared_ptr media) { + // NB! This allows PhotoMedia to outlive Main::Session! + // In case this is a problem this code should be rewritten. + crl::on_main(&session(), [media = std::move(media)]{}); +} + +void Session::keepAlive(std::shared_ptr media) { + // NB! This allows DocumentMedia to outlive Main::Session! + // In case this is a problem this code should be rewritten. + crl::on_main(&session(), [media = std::move(media)] {}); +} + not_null Session::peer(PeerId id) { const auto i = _peers.find(id); if (i != _peers.cend()) { @@ -3335,6 +3347,9 @@ void Session::registerItemView(not_null view) { } void Session::unregisterItemView(not_null view) { + Expects(!_playingVideoFiles.contains(view)); + Expects(!_heavyViewParts.contains(view)); + const auto i = _views.find(view->data()); if (i != end(_views)) { auto &list = i->second; diff --git a/Telegram/SourceFiles/data/data_session.h b/Telegram/SourceFiles/data/data_session.h index 3039aa17cb..bc4d7e3012 100644 --- a/Telegram/SourceFiles/data/data_session.h +++ b/Telegram/SourceFiles/data/data_session.h @@ -63,6 +63,8 @@ class CloudThemes; class Streaming; class MediaRotation; class Histories; +class DocumentMedia; +class PhotoMedia; class Session final { public: @@ -110,6 +112,9 @@ public: void clear(); + void keepAlive(std::shared_ptr media); + void keepAlive(std::shared_ptr media); + void startExport(PeerData *peer = nullptr); void startExport(const MTPInputPeer &singlePeer); void suggestStartExport(TimeId availableAt); diff --git a/Telegram/SourceFiles/history/view/history_view_element.cpp b/Telegram/SourceFiles/history/view/history_view_element.cpp index d017865c06..50fea5fe79 100644 --- a/Telegram/SourceFiles/history/view/history_view_element.cpp +++ b/Telegram/SourceFiles/history/view/history_view_element.cpp @@ -607,7 +607,7 @@ auto Element::verticalRepaintRange() const -> VerticalRepaintRange { } void Element::checkHeavyPart() { - if (_media && !_media->hasHeavyPart()) { + if (!_media || !_media->hasHeavyPart()) { history()->owner().unregisterHeavyViewPart(this); } } @@ -746,7 +746,7 @@ void Element::clickHandlerPressedChanged( Element::~Element() { // Delete media while owner still exists. - _media = nullptr; + base::take(_media); if (_data->mainView() == this) { _data->clearMainView(); } diff --git a/Telegram/SourceFiles/history/view/media/history_view_document.cpp b/Telegram/SourceFiles/history/view/media/history_view_document.cpp index 6084b7bfe1..da10341e9c 100644 --- a/Telegram/SourceFiles/history/view/media/history_view_document.cpp +++ b/Telegram/SourceFiles/history/view/media/history_view_document.cpp @@ -86,7 +86,7 @@ Document::Document( Document::~Document() { if (_dataMedia) { - _dataMedia = nullptr; + _data->owner().keepAlive(base::take(_dataMedia)); _parent->checkHeavyPart(); } } diff --git a/Telegram/SourceFiles/history/view/media/history_view_gif.cpp b/Telegram/SourceFiles/history/view/media/history_view_gif.cpp index 777dde4129..4125b35722 100644 --- a/Telegram/SourceFiles/history/view/media/history_view_gif.cpp +++ b/Telegram/SourceFiles/history/view/media/history_view_gif.cpp @@ -101,7 +101,9 @@ Gif::~Gif() { _data->owner().streaming().keepAlive(_data); setStreamed(nullptr); } - _dataMedia = nullptr; + if (_dataMedia) { + _data->owner().keepAlive(base::take(_dataMedia)); + } _parent->checkHeavyPart(); } } @@ -479,7 +481,8 @@ void Gif::draw(Painter &p, const QRect &r, TextSelection selection, crl::time ms auto over = _animation->a_thumbOver.value(1.); p.setBrush(anim::brush(st::msgDateImgBg, st::msgDateImgBgOver, over)); } else { - auto over = ClickHandler::showAsActive(_data->loading() ? _cancell : _savel); + const auto over = ClickHandler::showAsActive( + (_data->loading() || _data->uploading()) ? _cancell : _savel); p.setBrush(over ? st::msgDateImgBgOver : st::msgDateImgBg); } p.setOpacity(radialOpacity * p.opacity()); @@ -1013,7 +1016,8 @@ void Gif::drawGrouped( auto over = _animation->a_thumbOver.value(1.); p.setBrush(anim::brush(st::msgDateImgBg, st::msgDateImgBgOver, over)); } else { - auto over = ClickHandler::showAsActive(_data->loading() ? _cancell : _savel); + auto over = ClickHandler::showAsActive( + (_data->loading() || _data->uploading()) ? _cancell : _savel); p.setBrush(over ? st::msgDateImgBgOver : st::msgDateImgBg); } p.setOpacity(radialOpacity * p.opacity()); diff --git a/Telegram/SourceFiles/history/view/media/history_view_media_grouped.cpp b/Telegram/SourceFiles/history/view/media/history_view_media_grouped.cpp index d6c7760ee0..413b89036d 100644 --- a/Telegram/SourceFiles/history/view/media/history_view_media_grouped.cpp +++ b/Telegram/SourceFiles/history/view/media/history_view_media_grouped.cpp @@ -63,7 +63,7 @@ GroupedMedia::GroupedMedia( GroupedMedia::~GroupedMedia() { // Destroy all parts while the media object is still not destroyed. - base::take(_parts).clear(); + base::take(_parts); } QSize GroupedMedia::countOptimalSize() { diff --git a/Telegram/SourceFiles/history/view/media/history_view_media_unwrapped.cpp b/Telegram/SourceFiles/history/view/media/history_view_media_unwrapped.cpp index 1f574e8fba..4b974cf5bc 100644 --- a/Telegram/SourceFiles/history/view/media/history_view_media_unwrapped.cpp +++ b/Telegram/SourceFiles/history/view/media/history_view_media_unwrapped.cpp @@ -24,8 +24,6 @@ constexpr auto kMaxForwardedBarLines = 4; } // namespace -UnwrappedMedia::Content::~Content() = default; - UnwrappedMedia::UnwrappedMedia( not_null parent, std::unique_ptr content) diff --git a/Telegram/SourceFiles/history/view/media/history_view_media_unwrapped.h b/Telegram/SourceFiles/history/view/media/history_view_media_unwrapped.h index ea56b0ec54..5e38073dab 100644 --- a/Telegram/SourceFiles/history/view/media/history_view_media_unwrapped.h +++ b/Telegram/SourceFiles/history/view/media/history_view_media_unwrapped.h @@ -47,7 +47,7 @@ public: [[nodiscard]] virtual bool alwaysShowOutTimestamp() { return false; } - virtual ~Content() = 0; + virtual ~Content() = default; }; UnwrappedMedia( diff --git a/Telegram/SourceFiles/history/view/media/history_view_photo.cpp b/Telegram/SourceFiles/history/view/media/history_view_photo.cpp index 737105f060..04bcc1d962 100644 --- a/Telegram/SourceFiles/history/view/media/history_view_photo.cpp +++ b/Telegram/SourceFiles/history/view/media/history_view_photo.cpp @@ -14,6 +14,7 @@ https://github.com/telegramdesktop/tdesktop/blob/master/LEGAL #include "history/view/history_view_element.h" #include "history/view/history_view_cursor_state.h" #include "history/view/media/history_view_media_common.h" +#include "main/main_session.h" #include "ui/image/image.h" #include "ui/grouped_layout.h" #include "data/data_session.h" @@ -54,7 +55,7 @@ Photo::Photo( Photo::~Photo() { if (_dataMedia) { - _dataMedia = nullptr; + _data->owner().keepAlive(base::take(_dataMedia)); _parent->checkHeavyPart(); } } diff --git a/Telegram/SourceFiles/history/view/media/history_view_sticker.cpp b/Telegram/SourceFiles/history/view/media/history_view_sticker.cpp index 85bc420f78..02c0dcc20a 100644 --- a/Telegram/SourceFiles/history/view/media/history_view_sticker.cpp +++ b/Telegram/SourceFiles/history/view/media/history_view_sticker.cpp @@ -67,7 +67,9 @@ Sticker::Sticker( Sticker::~Sticker() { if (_lottie || _dataMedia) { unloadLottie(); - _dataMedia = nullptr; + if (_dataMedia) { + _data->owner().keepAlive(base::take(_dataMedia)); + } _parent->checkHeavyPart(); } } diff --git a/Telegram/SourceFiles/history/view/media/history_view_theme_document.cpp b/Telegram/SourceFiles/history/view/media/history_view_theme_document.cpp index 3840b7c7c3..7eacc1ec71 100644 --- a/Telegram/SourceFiles/history/view/media/history_view_theme_document.cpp +++ b/Telegram/SourceFiles/history/view/media/history_view_theme_document.cpp @@ -42,7 +42,7 @@ ThemeDocument::ThemeDocument( ThemeDocument::~ThemeDocument() { if (_dataMedia) { - _dataMedia = nullptr; + _data->owner().keepAlive(base::take(_dataMedia)); _parent->checkHeavyPart(); } } diff --git a/Telegram/SourceFiles/history/view/media/history_view_web_page.cpp b/Telegram/SourceFiles/history/view/media/history_view_web_page.cpp index 7f12c2c00e..5135c3c486 100644 --- a/Telegram/SourceFiles/history/view/media/history_view_web_page.cpp +++ b/Telegram/SourceFiles/history/view/media/history_view_web_page.cpp @@ -820,7 +820,7 @@ QString WebPage::displayedSiteName() const { WebPage::~WebPage() { history()->owner().unregisterWebPageView(_data, _parent); if (_photoMedia) { - _photoMedia = nullptr; + history()->owner().keepAlive(base::take(_photoMedia)); _parent->checkHeavyPart(); } }