Show avatars in the completion popup

include/AvatarProvider.h

@@ -25,9 +25,12 @@
 class MatrixClient;
 class TimelineItem;
 
+//! Saved cache data per user.
 struct AvatarData
 {
+        //! The avatar image of the user.
         QImage img;
+        //! The url that was used to download the avatar.
         QUrl url;
 };
 
@@ -37,17 +40,22 @@ class AvatarProvider : public QObject
 
 public:
         static void init(QSharedPointer<MatrixClient> client);
-        static void resolve(const QString &userId, std::function<void(QImage)> callback);
+        //! The callback is called with the downloaded avatar for the given user
+        //! or the avatar is downloaded first and then saved for re-use.
+        static void resolve(const QString &userId,
+                            QObject *receiver,
+                            std::function<void(QImage)> callback);
+        //! Used to initialize the mapping user -> avatar url.
         static void setAvatarUrl(const QString &userId, const QUrl &url);
-
-        static void clear();
+        //! Remove all saved data.
+        static void clear() { avatars_.clear(); };
 
 private:
+        //! Update the cache with the downloaded avatar.
         static void updateAvatar(const QString &uid, const QImage &img);
 
         static QSharedPointer<MatrixClient> client_;
 
         using UserID = QString;
         static std::map<UserID, AvatarData> avatars_;
-        static std::map<UserID, std::vector<std::function<void(QImage)>>> toBeResolved_;
 };

include/MatrixClient.h

@@ -29,6 +29,7 @@ class DownloadMediaProxy : public QObject
 signals:
         void imageDownloaded(const QPixmap &data);
         void fileDownloaded(const QByteArray &data);
+        void avatarDownloaded(const QImage &img);
 };
 
 /*
@@ -59,14 +60,12 @@ public:
         void versions() noexcept;
         void fetchRoomAvatar(const QString &roomid, const QUrl &avatar_url);
         //! Download user's avatar.
-        void fetchUserAvatar(const QUrl &avatarUrl,
-                             std::function<void(QImage)> onSuccess,
-                             std::function<void(QString)> onError);
+        QSharedPointer<DownloadMediaProxy> fetchUserAvatar(const QUrl &avatarUrl);
         void fetchCommunityAvatar(const QString &communityId, const QUrl &avatarUrl);
         void fetchCommunityProfile(const QString &communityId);
         void fetchCommunityRooms(const QString &communityId);
-        DownloadMediaProxy *downloadImage(const QUrl &url);
-        DownloadMediaProxy *downloadFile(const QUrl &url);
+        QSharedPointer<DownloadMediaProxy> downloadImage(const QUrl &url);
+        QSharedPointer<DownloadMediaProxy> downloadFile(const QUrl &url);
         void messages(const QString &room_id, const QString &from_token, int limit = 30) noexcept;
         void uploadImage(const QString &roomid,
                          const QString &filename,

include/timeline/TimelineItem.h

@@ -182,7 +182,8 @@ TimelineItem::setupLocalWidgetLayout(Widget *widget,
                 headerLayout_->addLayout(widgetLayout_);
                 messageLayout_->addLayout(headerLayout_, 1);
 
-                AvatarProvider::resolve(userid, [this](const QImage &img) { setUserAvatar(img); });
+                AvatarProvider::resolve(
+                  userid, this, [this](const QImage &img) { setUserAvatar(img); });
         } else {
                 setupSimpleLayout();
 
@@ -230,7 +231,8 @@ TimelineItem::setupWidgetLayout(Widget *widget,
                 headerLayout_->addLayout(widgetLayout_);
                 messageLayout_->addLayout(headerLayout_, 1);
 
-                AvatarProvider::resolve(sender, [this](const QImage &img) { setUserAvatar(img); });
+                AvatarProvider::resolve(
+                  sender, this, [this](const QImage &img) { setUserAvatar(img); });
         } else {
                 setupSimpleLayout();
 

src/AvatarProvider.cc

@@ -21,7 +21,6 @@
 QSharedPointer<MatrixClient> AvatarProvider::client_;
 
 std::map<QString, AvatarData> AvatarProvider::avatars_;
-std::map<QString, std::vector<std::function<void(QImage)>>> AvatarProvider::toBeResolved_;
 
 void
 AvatarProvider::init(QSharedPointer<MatrixClient> client)
@@ -32,22 +31,14 @@ AvatarProvider::init(QSharedPointer<MatrixClient> client)
 void
 AvatarProvider::updateAvatar(const QString &uid, const QImage &img)
 {
-        if (toBeResolved_.find(uid) != toBeResolved_.end()) {
-                auto callbacks = toBeResolved_[uid];
-
-                // Update all the timeline items with the resolved avatar.
-                for (const auto &callback : callbacks)
-                        callback(img);
-
-                toBeResolved_.erase(uid);
-        }
-
         auto avatarData = &avatars_[uid];
         avatarData->img = img;
 }
 
 void
-AvatarProvider::resolve(const QString &userId, std::function<void(QImage)> callback)
+AvatarProvider::resolve(const QString &userId,
+                        QObject *receiver,
+                        std::function<void(QImage)> callback)
 {
         if (avatars_.find(userId) == avatars_.end())
                 return;
@@ -59,23 +50,19 @@ AvatarProvider::resolve(const QString &userId, std::function<void(QImage)> callb
                 return;
         }
 
-        // Add the current timeline item to the waiting list for this avatar.
-        if (toBeResolved_.find(userId) == toBeResolved_.end()) {
-                client_->fetchUserAvatar(avatars_[userId].url,
-                                         [userId](QImage image) { updateAvatar(userId, image); },
-                                         [userId](QString error) {
-                                                 qWarning()
-                                                   << error << ": failed to retrieve user avatar"
-                                                   << userId;
-                                         });
+        auto proxy = client_->fetchUserAvatar(avatars_[userId].url);
 
-                std::vector<std::function<void(QImage)>> items;
-                items.emplace_back(callback);
+        if (proxy == nullptr)
+                return;
 
-                toBeResolved_.emplace(userId, items);
-        } else {
-                toBeResolved_[userId].emplace_back(callback);
-        }
+        connect(proxy.data(),
+                &DownloadMediaProxy::avatarDownloaded,
+                receiver,
+                [userId, proxy, callback](const QImage &img) {
+                        proxy->deleteLater();
+                        updateAvatar(userId, img);
+                        callback(img);
+                });
 }
 
 void
@@ -86,10 +73,3 @@ AvatarProvider::setAvatarUrl(const QString &userId, const QUrl &url)
 
         avatars_.emplace(userId, data);
 }
-
-void
-AvatarProvider::clear()
-{
-        avatars_.clear();
-        toBeResolved_.clear();
-}

src/ChatPage.cc

@@ -579,11 +579,20 @@ ChatPage::updateOwnProfileInfo(const QUrl &avatar_url, const QString &display_na
         user_info_widget_->setUserId(userid);
         user_info_widget_->setDisplayName(display_name);
 
-        if (avatar_url.isValid())
-                client_->fetchUserAvatar(
-                  avatar_url,
-                  [this](QImage img) { user_info_widget_->setAvatar(img); },
-                  [](QString error) { qWarning() << error << ": failed to fetch own avatar"; });
+        if (avatar_url.isValid()) {
+                auto proxy = client_->fetchUserAvatar(avatar_url);
+                if (proxy == nullptr)
+                        return;
+
+                proxy->setParent(this);
+                connect(proxy.data(),
+                        &DownloadMediaProxy::avatarDownloaded,
+                        this,
+                        [this, proxy](const QImage &img) {
+                                proxy->deleteLater();
+                                user_info_widget_->setAvatar(img);
+                        });
+        }
 }
 
 void

src/MatrixClient.cc

@@ -709,16 +709,14 @@ MatrixClient::fetchCommunityRooms(const QString &communityId)
         });
 }
 
-void
-MatrixClient::fetchUserAvatar(const QUrl &avatarUrl,
-                              std::function<void(QImage)> onSuccess,
-                              std::function<void(QString)> onError)
+QSharedPointer<DownloadMediaProxy>
+MatrixClient::fetchUserAvatar(const QUrl &avatarUrl)
 {
         QList<QString> url_parts = avatarUrl.toString().split("mxc://");
 
         if (url_parts.size() != 2) {
-                qDebug() << "Invalid format for user avatar " << avatarUrl.toString();
-                return;
+                qDebug() << "Invalid format for user avatar:" << avatarUrl.toString();
+                return nullptr;
         }
 
         QUrlQuery query;
@@ -735,33 +733,42 @@ MatrixClient::fetchUserAvatar(const QUrl &avatarUrl,
         QNetworkRequest avatar_request(endpoint);
 
         auto reply = get(avatar_request);
-        connect(reply, &QNetworkReply::finished, this, [reply, onSuccess, onError]() {
+        auto proxy = QSharedPointer<DownloadMediaProxy>(
+          new DownloadMediaProxy, [this](auto proxy) { proxy->deleteLater(); });
+        connect(reply, &QNetworkReply::finished, this, [reply, proxy, avatarUrl]() {
                 reply->deleteLater();
 
                 int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
-                if (status == 0 || status >= 400)
-                        return onError(reply->errorString());
+                if (status == 0 || status >= 400) {
+                        qWarning() << reply->errorString() << avatarUrl;
+                        return;
+                }
 
                 auto data = reply->readAll();
 
-                if (data.size() == 0)
-                        return onError("received avatar with no data");
+                if (data.size() == 0) {
+                        qWarning() << "received avatar with no data:" << avatarUrl;
+                        return;
+                }
 
                 QImage img;
                 img.loadFromData(data);
 
-                onSuccess(std::move(img));
+                emit proxy->avatarDownloaded(img);
         });
+
+        return proxy;
 }
 
-DownloadMediaProxy *
+QSharedPointer<DownloadMediaProxy>
 MatrixClient::downloadImage(const QUrl &url)
 {
         QNetworkRequest image_request(url);
 
         auto reply = get(image_request);
-        auto proxy = new DownloadMediaProxy;
+        auto proxy = QSharedPointer<DownloadMediaProxy>(
+          new DownloadMediaProxy, [this](auto proxy) { proxy->deleteLater(); });
         connect(reply, &QNetworkReply::finished, this, [reply, proxy]() {
                 reply->deleteLater();
 
@@ -786,13 +793,14 @@ MatrixClient::downloadImage(const QUrl &url)
         return proxy;
 }
 
-DownloadMediaProxy *
+QSharedPointer<DownloadMediaProxy>
 MatrixClient::downloadFile(const QUrl &url)
 {
         QNetworkRequest fileRequest(url);
 
         auto reply = get(fileRequest);
-        auto proxy = new DownloadMediaProxy;
+        auto proxy = QSharedPointer<DownloadMediaProxy>(
+          new DownloadMediaProxy, [this](auto proxy) { proxy->deleteLater(); });
         connect(reply, &QNetworkReply::finished, this, [reply, proxy]() {
                 reply->deleteLater();
 

src/SuggestionsPopup.cpp

@@ -44,8 +44,8 @@ PopupItem::PopupItem(QWidget *parent, const QString &user_id)
         topLayout_->addWidget(avatar_);
         topLayout_->addWidget(userName_, 1);
 
-        /* AvatarProvider::resolve(user_id, [this](const QImage &img) { avatar_->setImage(img); });
-         */
+        AvatarProvider::resolve(
+          user_id, this, [this](const QImage &img) { avatar_->setImage(img); });
 }
 
 void

src/dialogs/ReadReceipts.cc

@@ -51,7 +51,8 @@ ReceiptItem::ReceiptItem(QWidget *parent, const QString &user_id, uint64_t times
         topLayout_->addWidget(avatar_);
         topLayout_->addLayout(textLayout_, 1);
 
-        AvatarProvider::resolve(user_id, [this](const QImage &img) { avatar_->setImage(img); });
+        AvatarProvider::resolve(
+          user_id, this, [this](const QImage &img) { avatar_->setImage(img); });
 }
 
 QString

src/timeline/TimelineItem.cc

@@ -126,7 +126,8 @@ TimelineItem::TimelineItem(mtx::events::MessageType ty,
 
                 messageLayout_->addLayout(headerLayout_, 1);
 
-                AvatarProvider::resolve(userid, [this](const QImage &img) { setUserAvatar(img); });
+                AvatarProvider::resolve(
+                  userid, this, [this](const QImage &img) { setUserAvatar(img); });
         } else {
                 generateBody(body);
                 setupSimpleLayout();
@@ -259,7 +260,8 @@ TimelineItem::TimelineItem(const mtx::events::RoomEvent<mtx::events::msg::Notice
 
                 messageLayout_->addLayout(headerLayout_, 1);
 
-                AvatarProvider::resolve(sender, [this](const QImage &img) { setUserAvatar(img); });
+                AvatarProvider::resolve(
+                  sender, this, [this](const QImage &img) { setUserAvatar(img); });
         } else {
                 generateBody(body);
                 setupSimpleLayout();
@@ -303,7 +305,8 @@ TimelineItem::TimelineItem(const mtx::events::RoomEvent<mtx::events::msg::Emote>
 
                 messageLayout_->addLayout(headerLayout_, 1);
 
-                AvatarProvider::resolve(sender, [this](const QImage &img) { setUserAvatar(img); });
+                AvatarProvider::resolve(
+                  sender, this, [this](const QImage &img) { setUserAvatar(img); });
         } else {
                 generateBody(emoteMsg);
                 setupSimpleLayout();
@@ -352,7 +355,8 @@ TimelineItem::TimelineItem(const mtx::events::RoomEvent<mtx::events::msg::Text>
 
                 messageLayout_->addLayout(headerLayout_, 1);
 
-                AvatarProvider::resolve(sender, [this](const QImage &img) { setUserAvatar(img); });
+                AvatarProvider::resolve(
+                  sender, this, [this](const QImage &img) { setUserAvatar(img); });
         } else {
                 generateBody(body);
                 setupSimpleLayout();
@@ -562,5 +566,5 @@ TimelineItem::addAvatar()
         messageLayout_->addWidget(checkmark_);
         messageLayout_->addWidget(timestamp_);
 
-        AvatarProvider::resolve(userid, [this](const QImage &img) { setUserAvatar(img); });
+        AvatarProvider::resolve(userid, this, [this](const QImage &img) { setUserAvatar(img); });
 }

src/timeline/widgets/AudioItem.cc

@@ -135,7 +135,7 @@ AudioItem::mousePressEvent(QMouseEvent *event)
                         return;
 
                 auto proxy = client_->downloadFile(url_);
-                connect(proxy,
+                connect(proxy.data(),
                         &DownloadMediaProxy::fileDownloaded,
                         this,
                         [proxy, this](const QByteArray &data) {

src/timeline/widgets/FileItem.cc

@@ -121,7 +121,7 @@ FileItem::mousePressEvent(QMouseEvent *event)
                         return;
 
                 auto proxy = client_->downloadFile(url_);
-                connect(proxy,
+                connect(proxy.data(),
                         &DownloadMediaProxy::fileDownloaded,
                         this,
                         [proxy, this](const QByteArray &data) {

src/timeline/widgets/ImageItem.cc

@@ -56,11 +56,13 @@ ImageItem::ImageItem(QSharedPointer<MatrixClient> client,
 
         auto proxy = client_.data()->downloadImage(url_);
 
-        connect(
-          proxy, &DownloadMediaProxy::imageDownloaded, this, [this, proxy](const QPixmap &img) {
-                  proxy->deleteLater();
-                  setImage(img);
-          });
+        connect(proxy.data(),
+                &DownloadMediaProxy::imageDownloaded,
+                this,
+                [this, proxy](const QPixmap &img) {
+                        proxy->deleteLater();
+                        setImage(img);
+                });
 }
 
 ImageItem::ImageItem(QSharedPointer<MatrixClient> client,
@@ -92,11 +94,13 @@ ImageItem::ImageItem(QSharedPointer<MatrixClient> client,
 
         auto proxy = client_.data()->downloadImage(url_);
 
-        connect(
-          proxy, &DownloadMediaProxy::imageDownloaded, this, [proxy, this](const QPixmap &img) {
-                  proxy->deleteLater();
-                  setImage(img);
-          });
+        connect(proxy.data(),
+                &DownloadMediaProxy::imageDownloaded,
+                this,
+                [proxy, this](const QPixmap &img) {
+                        proxy->deleteLater();
+                        setImage(img);
+                });
 }
 
 void
@@ -230,7 +234,7 @@ ImageItem::saveAs()
                 return;
 
         auto proxy = client_->downloadFile(url_);
-        connect(proxy,
+        connect(proxy.data(),
                 &DownloadMediaProxy::fileDownloaded,
                 this,
                 [proxy, filename](const QByteArray &data) {