Fix Wayland image upload crash if confirmation dialog is enabled (#5314)

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Minor: Added context menu action to toggle visibility of offline tabs. (#5318)
 - Minor: Report sub duration for more multi-month gift cases. (#5319)
+- Bugfix: Fixed a crash that could occur on Wayland when using the image uploader. (#5314)
 - Bugfix: Fixed split tooltip getting stuck in some cases. (#5309)
 - Bugfix: Fixed the version string not showing up as expected in Finder on macOS. (#5311)
 

src/singletons/ImageUploader.cpp

@@ -5,6 +5,7 @@
 #include "common/network/NetworkRequest.hpp"
 #include "common/network/NetworkResult.hpp"
 #include "common/QLogging.hpp"
+#include "debug/Benchmark.hpp"
 #include "messages/MessageBuilder.hpp"
 #include "providers/twitch/TwitchMessageBuilder.hpp"
 #include "singletons/Paths.hpp"
@@ -21,6 +22,8 @@
 #include <QPointer>
 #include <QSaveFile>
 
+#include <utility>
+
 #define UPLOAD_DELAY 2000
 // Delay between uploads in milliseconds
 
@@ -195,6 +198,11 @@ void ImageUploader::handleFailedUpload(const NetworkResult &result,
     }
 
     channel->addMessage(makeSystemMessage(errorMessage));
+    // NOTE: We abort any future uploads on failure. Should this be handled differently?
+    while (!this->uploadQueue_.empty())
+    {
+        this->uploadQueue_.pop();
+    }
     this->uploadMutex_.unlock();
 }
 
@@ -248,22 +256,20 @@ void ImageUploader::handleSuccessfulUpload(const NetworkResult &result,
     this->logToFile(originalFilePath, link, deletionLink, channel);
 }
 
-void ImageUploader::upload(const QMimeData *source, ChannelPtr channel,
+std::pair<std::queue<RawImageData>, QString> ImageUploader::getImages(
-                           QPointer<ResizingTextEdit> outputTextEdit)
+    const QMimeData *source) const
 {
-    if (!this->uploadMutex_.tryLock())
+    BenchmarkGuard benchmarkGuard("ImageUploader::getImages");
-    {
-        channel->addMessage(makeSystemMessage(
-            QString("Please wait until the upload finishes.")));
-        return;
-    }
 
-    channel->addMessage(makeSystemMessage(QString("Started upload...")));
+    auto tryUploadFromUrls =
-    auto tryUploadFromUrls = [&]() -> bool {
+        [&]() -> std::pair<std::queue<RawImageData>, QString> {
         if (!source->hasUrls())
         {
-            return false;
+            return {{}, {}};
         }
+
+        std::queue<RawImageData> images;
+
         auto mimeDb = QMimeDatabase();
         // This path gets chosen when files are copied from a file manager, like explorer.exe, caja.
         // Each entry in source->urls() is a QUrl pointing to a file that was copied.
@@ -273,101 +279,118 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel,
             QMimeType mime = mimeDb.mimeTypeForUrl(path);
             if (mime.name().startsWith("image") && !mime.inherits("image/gif"))
             {
-                channel->addMessage(makeSystemMessage(
-                    QString("Uploading image: %1").arg(localPath)));
                 QImage img = QImage(localPath);
                 if (img.isNull())
                 {
-                    channel->addMessage(
+                    return {{}, "Couldn't load image :("};
-                        makeSystemMessage(QString("Couldn't load image :(")));
-                    return false;
                 }
 
                 auto imageData = convertToPng(img);
-                if (imageData)
+                if (!imageData)
                 {
-                    RawImageData data = {*imageData, "png", localPath};
+                    return {
-                    this->uploadQueue_.push(data);
+                        {},
-                }
-                else
-                {
-                    channel->addMessage(makeSystemMessage(
                         QString("Cannot upload file: %1. Couldn't convert "
                                 "image to png.")
-                            .arg(localPath)));
+                            .arg(localPath),
-                    return false;
+                    };
                 }
+                images.push({*imageData, "png", localPath});
             }
             else if (mime.inherits("image/gif"))
             {
-                channel->addMessage(makeSystemMessage(
-                    QString("Uploading GIF: %1").arg(localPath)));
                 QFile file(localPath);
                 bool isOkay = file.open(QIODevice::ReadOnly);
                 if (!isOkay)
                 {
-                    channel->addMessage(
+                    return {{}, "Failed to open file :("};
-                        makeSystemMessage(QString("Failed to open file. :(")));
-                    return false;
                 }
                 // file.readAll() => might be a bit big but it /should/ work
-                RawImageData data = {file.readAll(), "gif", localPath};
+                images.push({file.readAll(), "gif", localPath});
-                this->uploadQueue_.push(data);
                 file.close();
             }
         }
-        if (!this->uploadQueue_.empty())
+
-        {
+        return {images, {}};
-            this->sendImageUploadRequest(this->uploadQueue_.front(), channel,
-                                         outputTextEdit);
-            this->uploadQueue_.pop();
-            return true;
-        }
-        return false;
     };
 
-    auto tryUploadDirectly = [&]() -> bool {
+    auto tryUploadDirectly =
+        [&]() -> std::pair<std::queue<RawImageData>, QString> {
+        std::queue<RawImageData> images;
+
         if (source->hasFormat("image/png"))
         {
             // the path to file is not present every time, thus the filePath is empty
-            this->sendImageUploadRequest({source->data("image/png"), "png", ""},
+            images.push({source->data("image/png"), "png", ""});
-                                         channel, outputTextEdit);
+            return {images, {}};
-            return true;
         }
+
         if (source->hasFormat("image/jpeg"))
         {
-            this->sendImageUploadRequest(
+            images.push({source->data("image/jpeg"), "jpeg", ""});
-                {source->data("image/jpeg"), "jpeg", ""}, channel,
+            return {images, {}};
-                outputTextEdit);
-            return true;
         }
+
         if (source->hasFormat("image/gif"))
         {
-            this->sendImageUploadRequest({source->data("image/gif"), "gif", ""},
+            images.push({source->data("image/gif"), "gif", ""});
-                                         channel, outputTextEdit);
+            return {images, {}};
-            return true;
         }
+
         // not PNG, try loading it into QImage and save it to a PNG.
         auto image = qvariant_cast<QImage>(source->imageData());
         auto imageData = convertToPng(image);
         if (imageData)
         {
-            sendImageUploadRequest({*imageData, "png", ""}, channel,
+            images.push({*imageData, "png", ""});
-                                   outputTextEdit);
+            return {images, {}};
-            return true;
         }
+
         // No direct upload happenned
-        channel->addMessage(makeSystemMessage(
+        return {{}, "Cannot upload file, failed to convert to png."};
-            QString("Cannot upload file, failed to convert to png.")));
-        return false;
     };
 
-    if (!tryUploadFromUrls() && !tryUploadDirectly())
+    const auto [urlImageData, urlError] = tryUploadFromUrls();
+
+    if (!urlImageData.empty())
     {
-        channel->addMessage(
+        return {urlImageData, {}};
-            makeSystemMessage(QString("Cannot upload file from clipboard.")));
-        this->uploadMutex_.unlock();
     }
+
+    const auto [directImageData, directError] = tryUploadDirectly();
+    if (!directImageData.empty())
+    {
+        return {directImageData, {}};
+    }
+
+    return {
+        {},
+        // TODO: verify that this looks ok xd
+        urlError + directError,
+    };
+}
+
+void ImageUploader::upload(std::queue<RawImageData> images, ChannelPtr channel,
+                           QPointer<ResizingTextEdit> outputTextEdit)
+{
+    BenchmarkGuard benchmarkGuard("upload");
+    if (!this->uploadMutex_.tryLock())
+    {
+        channel->addMessage(makeSystemMessage(
+            QString("Please wait until the upload finishes.")));
+        return;
+    }
+
+    assert(!images.empty());
+    assert(this->uploadQueue_.empty());
+
+    std::swap(this->uploadQueue_, images);
+
+    channel->addMessage(makeSystemMessage("Started upload..."));
+
+    this->sendImageUploadRequest(this->uploadQueue_.front(), std::move(channel),
+                                 std::move(outputTextEdit));
+    this->uploadQueue_.pop();
 }
 
 }  // namespace chatterino

src/singletons/ImageUploader.hpp

@@ -25,8 +25,16 @@ struct RawImageData {
 class ImageUploader final : public Singleton
 {
 public:
+    /**
+     * Tries to get the image(s) from the given QMimeData
+     *
+     * If no images were found, the second value in the pair will contain an error message
+     */
+    std::pair<std::queue<RawImageData>, QString> getImages(
+        const QMimeData *source) const;
+
     void save() override;
-    void upload(const QMimeData *source, ChannelPtr channel,
+    void upload(std::queue<RawImageData> images, ChannelPtr channel,
                 QPointer<ResizingTextEdit> outputTextEdit);
 
 private:

src/widgets/splits/Split.cpp

@@ -380,12 +380,26 @@ Split::Split(QWidget *parent)
 
     // this connection can be ignored since the SplitInput is owned by this Split
     std::ignore = this->input_->ui_.textEdit->imagePasted.connect(
-        [this](const QMimeData *source) {
+        [this](const QMimeData *original) {
             if (!getSettings()->imageUploaderEnabled)
             {
                 return;
             }
 
+            auto channel = this->getChannel();
+            auto *imageUploader = getIApp()->getImageUploader();
+
+            auto [images, imageProcessError] =
+                imageUploader->getImages(original);
+            if (images.empty())
+            {
+                channel->addMessage(makeSystemMessage(
+                    QString(
+                        "An error occurred trying to process your image: %1")
+                        .arg(imageProcessError)));
+                return;
+            }
+
             if (getSettings()->askOnImageUpload.getValue())
             {
                 QMessageBox msgBox(this->window());
@@ -427,9 +441,9 @@ Split::Split(QWidget *parent)
                     return;
                 }
             }
+
             QPointer<ResizingTextEdit> edit = this->input_->ui_.textEdit;
-            getIApp()->getImageUploader()->upload(source, this->getChannel(),
+            imageUploader->upload(std::move(images), channel, edit);
-                                                  edit);
         });
 
     getSettings()->imageUploaderEnabled.connect(