made caller/concurrent rules tighter

src/common/NetworkPrivate.cpp

@@ -125,6 +125,11 @@ void loadUncached(const std::shared_ptr<NetworkData> &data)
         }
 
         auto handleReply = [data, reply]() mutable {
+            if (data->hasCaller_ && !data->caller_.get())
+            {
+                return;
+            }
+
             // TODO(pajlada): A reply was received, kill the timeout timer
             if (reply->error() != QNetworkReply::NetworkError::NoError)
             {
@@ -162,7 +167,6 @@ void loadUncached(const std::shared_ptr<NetworkData> &data)
                 if (data->executeConcurrently || isGuiThread())
                 {
                     handleReply();
-
                     delete worker;
                 }
                 else
@@ -206,11 +210,22 @@ void loadCached(const std::shared_ptr<NetworkData> &data)
                 // XXX: If outcome is Failure, we should invalidate the cache file
                 // somehow/somewhere
                 /*auto outcome =*/
+                if (data->hasCaller_ && !data->caller_.get())
+                {
+                    return;
+                }
                 data->onSuccess_(result);
             }
             else
             {
-                postToThread([data, result]() { data->onSuccess_(result); });
+                postToThread([data, result]() {
+                    if (data->hasCaller_ && !data->caller_.get())
+                    {
+                        return;
+                    }
+
+                    data->onSuccess_(result);
+                });
             }
         }
     }  // namespace chatterino

src/common/NetworkPrivate.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "common/NetworkCommon.hpp"
+#include "util/QObjectRef.hpp"
 
 #include <QNetworkRequest>
 #include <functional>
@@ -32,7 +33,8 @@ struct NetworkData {
     ~NetworkData();
 
     QNetworkRequest request_;
-    const QObject *caller_ = nullptr;
+    bool hasCaller_{};
+    QObjectRef<QObject> caller_;
     bool useQuickLoadCache_{};
     bool executeConcurrently{};
 

src/common/NetworkRequest.cpp

@@ -50,7 +50,11 @@ NetworkRequest NetworkRequest::type(NetworkRequestType newRequestType) &&
 
 NetworkRequest NetworkRequest::caller(const QObject *caller) &&
 {
-    this->data->caller_ = caller;
+    // Caller must be in gui thread
+    assert(caller->thread() == qApp->thread());
+
+    this->data->caller_ = const_cast<QObject *>(caller);
+    this->data->hasCaller_ = true;
     return std::move(*this);
 }
 
@@ -144,6 +148,9 @@ void NetworkRequest::execute()
         this->data->useQuickLoadCache_ = false;
     }
 
+    // Can not have a caller and be concurrent at the same time.
+    assert(!(this->data->caller_ && this->data->executeConcurrently));
+
     load(std::move(this->data));
 }
 

src/common/NetworkRequest.hpp

@@ -44,6 +44,10 @@ public:
 
     NetworkRequest payload(const QByteArray &payload) &&;
     NetworkRequest cache() &&;
+    /// NetworkRequest makes sure that the `caller` object still exists when the
+    /// callbacks are executed. Cannot be used with concurrent() since we can't
+    /// make sure that the object doesn't get deleted while the callback is
+    /// running.
     NetworkRequest caller(const QObject *caller) &&;
     NetworkRequest header(const char *headerName, const char *value) &&;
     NetworkRequest header(const char *headerName, const QByteArray &value) &&;

src/messages/Image.cpp

@@ -327,7 +327,7 @@ int Image::height() const
     assertInGuiThread();
 
     if (auto pixmap = this->frames_->first())
-        return pixmap->height() * this->scale_;
+        return int(pixmap->height() * this->scale_);
     else
         return 16;
 }
@@ -336,7 +336,6 @@ void Image::load()
 {
     NetworkRequest(this->url().string)
         .concurrent()
-        .caller(&this->object_)
         .cache()
         .onSuccess([that = this, weak = weakOf(this)](auto result) -> Outcome {
             auto shared = weak.lock();

src/providers/twitch/PartialTwitchUser.cpp

@@ -31,11 +31,6 @@ void PartialTwitchUser::getId(std::function<void(QString)> successCallback,
 {
     assert(!this->username_.isEmpty());
 
-    if (caller == nullptr)
-    {
-        caller = QThread::currentThread();
-    }
-
     NetworkRequest("https://api.twitch.tv/kraken/users?login=" +
                    this->username_)
         .caller(caller)

src/providers/twitch/TwitchChannel.cpp

@@ -607,7 +607,6 @@ void TwitchChannel::loadRecentMessages()
     }
 
     NetworkRequest(Env::get().recentMessagesApiUrl.arg(this->getName()))
-        .caller(QThread::currentThread())
         .concurrent()
         .onSuccess([weak = weakOf<Channel>(this)](auto result) -> Outcome {
             auto shared = weak.lock();

src/util/QObjectRef.hpp

@@ -69,7 +69,7 @@ private:
         this->t_ = other;
     }
 
-    T *t_{};
+    std::atomic<T *> t_{};
     QMetaObject::Connection conn_;
 };
 }  // namespace chatterino

src/widgets/dialogs/LogsPopup.cpp

@@ -93,7 +93,7 @@ void LogsPopup::getLogviewerLogs(const QString &roomID)
 
     NetworkRequest(url)
         .caller(this)
-        .onError([this](int errorCode) {
+        .onError([this](int /*errorCode*/) {
             this->getOverrustleLogs();
             return true;
         })
@@ -138,7 +138,7 @@ void LogsPopup::getOverrustleLogs()
 
     NetworkRequest(url)
         .caller(this)
-        .onError([this](int errorCode) {
+        .onError([this](int /*errorCode*/) {
             auto box = new QMessageBox(
                 QMessageBox::Information, "Error getting logs",
                 "No logs could be found for channel " + this->channelName_);