diff --git a/CHANGELOG.md b/CHANGELOG.md index cafe3e3e..3ed8fc68 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/singletons/ImageUploader.cpp b/src/singletons/ImageUploader.cpp index f8ad53d2..3926df8f 100644 --- a/src/singletons/ImageUploader.cpp +++ b/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 #include +#include + #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, - QPointer outputTextEdit) +std::pair, QString> ImageUploader::getImages( + const QMimeData *source) const { - if (!this->uploadMutex_.tryLock()) - { - channel->addMessage(makeSystemMessage( - QString("Please wait until the upload finishes."))); - return; - } + BenchmarkGuard benchmarkGuard("ImageUploader::getImages"); - channel->addMessage(makeSystemMessage(QString("Started upload..."))); - auto tryUploadFromUrls = [&]() -> bool { + auto tryUploadFromUrls = + [&]() -> std::pair, QString> { if (!source->hasUrls()) { - return false; + return {{}, {}}; } + + std::queue 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( - makeSystemMessage(QString("Couldn't load image :("))); - return false; + return {{}, "Couldn't load image :("}; } auto imageData = convertToPng(img); - if (imageData) + if (!imageData) { - RawImageData data = {*imageData, "png", localPath}; - this->uploadQueue_.push(data); - } - else - { - channel->addMessage(makeSystemMessage( + return { + {}, QString("Cannot upload file: %1. Couldn't convert " "image to png.") - .arg(localPath))); - return false; + .arg(localPath), + }; } + 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( - makeSystemMessage(QString("Failed to open file. :("))); - return false; + return {{}, "Failed to open file :("}; } // file.readAll() => might be a bit big but it /should/ work - RawImageData data = {file.readAll(), "gif", localPath}; - this->uploadQueue_.push(data); + images.push({file.readAll(), "gif", localPath}); file.close(); } } - if (!this->uploadQueue_.empty()) - { - this->sendImageUploadRequest(this->uploadQueue_.front(), channel, - outputTextEdit); - this->uploadQueue_.pop(); - return true; - } - return false; + + return {images, {}}; }; - auto tryUploadDirectly = [&]() -> bool { + auto tryUploadDirectly = + [&]() -> std::pair, QString> { + std::queue 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", ""}, - channel, outputTextEdit); - return true; + images.push({source->data("image/png"), "png", ""}); + return {images, {}}; } + if (source->hasFormat("image/jpeg")) { - this->sendImageUploadRequest( - {source->data("image/jpeg"), "jpeg", ""}, channel, - outputTextEdit); - return true; + images.push({source->data("image/jpeg"), "jpeg", ""}); + return {images, {}}; } + if (source->hasFormat("image/gif")) { - this->sendImageUploadRequest({source->data("image/gif"), "gif", ""}, - channel, outputTextEdit); - return true; + images.push({source->data("image/gif"), "gif", ""}); + return {images, {}}; } + // not PNG, try loading it into QImage and save it to a PNG. auto image = qvariant_cast(source->imageData()); auto imageData = convertToPng(image); if (imageData) { - sendImageUploadRequest({*imageData, "png", ""}, channel, - outputTextEdit); - return true; + images.push({*imageData, "png", ""}); + return {images, {}}; } + // No direct upload happenned - channel->addMessage(makeSystemMessage( - QString("Cannot upload file, failed to convert to png."))); - return false; + return {{}, "Cannot upload file, failed to convert to png."}; }; - if (!tryUploadFromUrls() && !tryUploadDirectly()) + const auto [urlImageData, urlError] = tryUploadFromUrls(); + + if (!urlImageData.empty()) { - channel->addMessage( - makeSystemMessage(QString("Cannot upload file from clipboard."))); - this->uploadMutex_.unlock(); + return {urlImageData, {}}; } + + 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 images, ChannelPtr channel, + QPointer 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 diff --git a/src/singletons/ImageUploader.hpp b/src/singletons/ImageUploader.hpp index 26018058..41f4c8b6 100644 --- a/src/singletons/ImageUploader.hpp +++ b/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, QString> getImages( + const QMimeData *source) const; + void save() override; - void upload(const QMimeData *source, ChannelPtr channel, + void upload(std::queue images, ChannelPtr channel, QPointer outputTextEdit); private: diff --git a/src/widgets/splits/Split.cpp b/src/widgets/splits/Split.cpp index 9e312631..1e4c228e 100644 --- a/src/widgets/splits/Split.cpp +++ b/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 edit = this->input_->ui_.textEdit; - getIApp()->getImageUploader()->upload(source, this->getChannel(), - edit); + imageUploader->upload(std::move(images), channel, edit); }); getSettings()->imageUploaderEnabled.connect(