Show avatars in the completion popup

9 files changed, 82 insertions, 76 deletions

diff --git a/src/AvatarProvider.cc b/src/AvatarProvider.cc
index 1c509d0a..e6e98886 100644
--- a/src/AvatarProvider.cc
+++ b/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();
-}
diff --git a/src/ChatPage.cc b/src/ChatPage.cc
index b49fb6a2..0d69c51f 100644
--- a/src/ChatPage.cc
+++ b/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
diff --git a/src/MatrixClient.cc b/src/MatrixClient.cc
index dff7b825..60feefde 100644
--- a/src/MatrixClient.cc
+++ b/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();
 
diff --git a/src/SuggestionsPopup.cpp b/src/SuggestionsPopup.cpp
index 3a7b3852..8a900790 100644
--- a/src/SuggestionsPopup.cpp
+++ b/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
diff --git a/src/dialogs/ReadReceipts.cc b/src/dialogs/ReadReceipts.cc
index 839871be..63ce68e6 100644
--- a/src/dialogs/ReadReceipts.cc
+++ b/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
diff --git a/src/timeline/TimelineItem.cc b/src/timeline/TimelineItem.cc
index 326b2c14..0296c6cd 100644
--- a/src/timeline/TimelineItem.cc
+++ b/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); });
 }
diff --git a/src/timeline/widgets/AudioItem.cc b/src/timeline/widgets/AudioItem.cc
index 9f8b5dd1..79f944ff 100644
--- a/src/timeline/widgets/AudioItem.cc
+++ b/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) {
diff --git a/src/timeline/widgets/FileItem.cc b/src/timeline/widgets/FileItem.cc
index d11ebe91..7445eb0f 100644
--- a/src/timeline/widgets/FileItem.cc
+++ b/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) {
diff --git a/src/timeline/widgets/ImageItem.cc b/src/timeline/widgets/ImageItem.cc
index fc1e46f5..3aae63c8 100644
--- a/src/timeline/widgets/ImageItem.cc
+++ b/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) {