refactor: Toast (#4899)

* Fixes a bug where avatars weren't loaded on fresh installations of Chatterino. * Avatars now update every two weeks. * Removes misleading `DownlaodManager` (now part of `Toasts.cpp`). * Refactors usage of WinToast to be easier to read. * Added version to AUMI. * Removes manual `QString` → `std::wstring` conversions. * Removes uses of implicit ASCII casts in `Toasts.cpp`, meaning it can be compiled with `QT_NO_CAST_FROM_ASCII`.
2024-11-13 19:49:51 +01:00 · 2023-10-17 03:50:18 +02:00 · 2023-10-17 03:50:18 +02:00 · b975900043
parent bddc08abd0
commit b975900043
9 changed files with 186 additions and 210 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -22,6 +22,7 @@
 - Bugfix: Fixed issue on Windows preventing the title bar from being dragged in the top left corner. (#4873)
 - Bugfix: Fixed an issue where reply context didn't render correctly if an emoji was touching text. (#4875)
 - Bugfix: Fixed the input completion popup from disappearing when clicking on it on Windows and macOS. (#4876)
+- Bugfix: Fixed an issue where notifications on Windows would contain no or an old avatar. (#4899)
 - Dev: Fixed UTF16 encoding of `modes` file for the installer. (#4791)
 - Dev: Temporarily disable High DPI scaling on Qt6 builds on Windows. (#4767)
 - Dev: Tests now run on Ubuntu 22.04 instead of 20.04 to loosen C++ restrictions in tests. (#4774)
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@ -26,8 +26,6 @@ set(SOURCE_FILES
        common/ChatterSet.hpp
        common/Credentials.cpp
        common/Credentials.hpp
-        common/DownloadManager.cpp
-        common/DownloadManager.hpp
        common/Env.cpp
        common/Env.hpp
        common/LinkParser.cpp
--- a/src/RunGui.cpp
+++ b/src/RunGui.cpp
@ -20,6 +20,7 @@
 #include <QtConcurrent>

 #include <csignal>
+#include <tuple>

 #ifdef USEWINSDK
 #    include "util/WindowsHelper.hpp"
@ -184,17 +185,20 @@ namespace {
    // improved in the future.
    void clearCache(const QDir &dir)
    {
-        int deletedCount = 0;
-        for (auto &&info : dir.entryInfoList(QDir::Files))
+        size_t deletedCount = 0;
+        for (const auto &info : dir.entryInfoList(QDir::Files))
        {
            if (info.lastModified().addDays(14) < QDateTime::currentDateTime())
            {
                bool res = QFile(info.absoluteFilePath()).remove();
                if (res)
+                {
                    ++deletedCount;
+                }
            }
        }
-        qCDebug(chatterinoCache) << "Deleted" << deletedCount << "files";
+        qCDebug(chatterinoCache)
+            << "Deleted" << deletedCount << "files in" << dir.path();
    }

    // We delete all but the five most recent crashdumps. This strategy may be
@ -259,12 +263,15 @@ void runGui(QApplication &a, Paths &paths, Settings &settings)

    // Clear the cache 1 minute after start.
    QTimer::singleShot(60 * 1000, [cachePath = paths.cacheDirectory(),
-                                   crashDirectory = paths.crashdumpDirectory] {
-        QtConcurrent::run([cachePath]() {
+                                   crashDirectory = paths.crashdumpDirectory,
+                                   avatarPath = paths.twitchProfileAvatars] {
+        std::ignore = QtConcurrent::run([cachePath] {
            clearCache(cachePath);
        });
-
-        QtConcurrent::run([crashDirectory]() {
+        std::ignore = QtConcurrent::run([avatarPath] {
+            clearCache(avatarPath);
+        });
+        std::ignore = QtConcurrent::run([crashDirectory] {
            clearCrashes(crashDirectory);
        });
    });
--- a/src/common/DownloadManager.cpp
+++ b/src/common/DownloadManager.cpp
@ -1,83 +0,0 @@
-#include "DownloadManager.hpp"
-
-#include "common/QLogging.hpp"
-#include "singletons/Paths.hpp"
-
-#include <QDesktopServices>
-
-namespace chatterino {
-
-DownloadManager::DownloadManager(QObject *parent)
-    : QObject(parent)
-    , manager_(new QNetworkAccessManager)
-{
-}
-
-DownloadManager::~DownloadManager()
-{
-    this->manager_->deleteLater();
-}
-
-void DownloadManager::setFile(QString fileURL, const QString &channelName)
-{
-    QString saveFilePath;
-    saveFilePath =
-        getPaths()->twitchProfileAvatars + "/twitch/" + channelName + ".png";
-    QNetworkRequest request;
-    request.setUrl(QUrl(fileURL));
-    this->reply_ = this->manager_->get(request);
-
-    this->file_ = new QFile;
-    this->file_->setFileName(saveFilePath);
-    this->file_->open(QIODevice::WriteOnly);
-
-    connect(this->reply_, SIGNAL(downloadProgress(qint64, qint64)), this,
-            SLOT(onDownloadProgress(qint64, qint64)));
-    connect(this->manager_, SIGNAL(finished(QNetworkReply *)), this,
-            SLOT(onFinished(QNetworkReply *)));
-    connect(this->reply_, SIGNAL(readyRead()), this, SLOT(onReadyRead()));
-    connect(this->reply_, SIGNAL(finished()), this, SLOT(onReplyFinished()));
-}
-
-void DownloadManager::onDownloadProgress(qint64 bytesRead, qint64 bytesTotal)
-{
-    qCDebug(chatterinoCommon)
-        << "Download progress: " << bytesRead << "/" << bytesTotal;
-}
-
-void DownloadManager::onFinished(QNetworkReply *reply)
-{
-    switch (reply->error())
-    {
-        case QNetworkReply::NoError: {
-            qCDebug(chatterinoCommon) << "file is downloaded successfully.";
-        }
-        break;
-        default: {
-            qCDebug(chatterinoCommon) << reply->errorString().toLatin1();
-        };
-    }
-
-    if (this->file_->isOpen())
-    {
-        this->file_->close();
-        this->file_->deleteLater();
-    }
-    emit downloadComplete();
-}
-
-void DownloadManager::onReadyRead()
-{
-    this->file_->write(this->reply_->readAll());
-}
-
-void DownloadManager::onReplyFinished()
-{
-    if (this->file_->isOpen())
-    {
-        this->file_->close();
-        this->file_->deleteLater();
-    }
-}
-
-}  // namespace chatterino
--- a/src/common/DownloadManager.hpp
+++ b/src/common/DownloadManager.hpp
@ -1,33 +0,0 @@
-#pragma once
-
-#include <QFile>
-#include <QNetworkAccessManager>
-#include <QNetworkReply>
-#include <QObject>
-
-namespace chatterino {
-
-class DownloadManager : public QObject
-{
-    Q_OBJECT
-public:
-    explicit DownloadManager(QObject *parent = nullptr);
-    virtual ~DownloadManager();
-    void setFile(QString fileURL, const QString &channelName);
-
-private:
-    QNetworkAccessManager *manager_;
-    QNetworkReply *reply_{};
-    QFile *file_{};
-
-private slots:
-    void onDownloadProgress(qint64, qint64);
-    void onFinished(QNetworkReply *);
-    void onReadyRead();
-    void onReplyFinished();
-
-signals:
-    void downloadComplete();
-};
-
-}  // namespace chatterino
--- a/src/singletons/Paths.cpp
+++ b/src/singletons/Paths.cpp
@ -122,9 +122,8 @@ void Paths::initSubDirectories()

    // create settings subdirectories and validate that they are created
    // properly
-    auto makePath = [&](const std::string &name) -> QString {
-        auto path = combinePath(this->rootAppDataDirectory,
-                                QString::fromStdString(name));
+    auto makePath = [&](const QString &name) -> QString {
+        auto path = combinePath(this->rootAppDataDirectory, name);

        if (!QDir().mkpath(path))
        {
@ -140,11 +139,11 @@ void Paths::initSubDirectories()
    this->cacheDirectory_ = makePath("Cache");
    this->messageLogDirectory = makePath("Logs");
    this->miscDirectory = makePath("Misc");
-    this->twitchProfileAvatars = makePath("ProfileAvatars");
+    this->twitchProfileAvatars =
+        makePath(combinePath("ProfileAvatars", "twitch"));
    this->pluginsDirectory = makePath("Plugins");
    this->themesDirectory = makePath("Themes");
    this->crashdumpDirectory = makePath("Crashes");
-    //QDir().mkdir(this->twitchProfileAvatars + "/twitch");
 }

 Paths *getPaths()
--- a/src/singletons/Paths.hpp
+++ b/src/singletons/Paths.hpp
@ -32,7 +32,7 @@ public:
    // Hash of QCoreApplication::applicationFilePath()
    QString applicationFilePathHash;

-    // Profile avatars for Twitch <appDataDirectory>/cache/twitch
+    // Profile avatars for Twitch <appDataDirectory>/ProfileAvatars/twitch
    QString twitchProfileAvatars;

    // Plugin files live here. <appDataDirectory>/Plugins
--- a/src/singletons/Toasts.cpp
+++ b/src/singletons/Toasts.cpp
@ -1,12 +1,11 @@
 #include "Toasts.hpp"

 #include "Application.hpp"
-#include "common/DownloadManager.hpp"
-#include "common/NetworkRequest.hpp"
+#include "common/Literals.hpp"
+#include "common/QLogging.hpp"
+#include "common/Version.hpp"
 #include "controllers/notifications/NotificationController.hpp"
 #include "providers/twitch/api/Helix.hpp"
-#include "providers/twitch/TwitchChannel.hpp"
-#include "providers/twitch/TwitchCommon.hpp"
 #include "providers/twitch/TwitchIrcServer.hpp"
 #include "singletons/Paths.hpp"
 #include "singletons/Settings.hpp"
@ -14,32 +13,64 @@
 #include "widgets/helper/CommonTexts.hpp"

 #ifdef Q_OS_WIN
-
 #    include <wintoastlib.h>
-
 #endif

 #include <QDesktopServices>
 #include <QFileInfo>
 #include <QNetworkAccessManager>
 #include <QNetworkReply>
+#include <QStringBuilder>
 #include <QUrl>

-#include <cstdlib>
+#include <utility>
+
+namespace {
+
+using namespace chatterino;
+using namespace literals;
+
+QString avatarFilePath(const QString &channelName)
+{
+    // TODO: cleanup channel (to be used as a file) and use combinePath
+    return getPaths()->twitchProfileAvatars % '/' % channelName % u".png";
+}
+
+bool hasAvatarForChannel(const QString &channelName)
+{
+    QFileInfo avatarFile(avatarFilePath(channelName));
+    return avatarFile.exists() && avatarFile.isFile();
+}
+
+/// A job that downlaods a twitch avatar and saves it to a file
+class AvatarDownloader : public QObject
+{
+    Q_OBJECT
+public:
+    AvatarDownloader(const QString &avatarURL, const QString &channelName);
+
+private:
+    QNetworkAccessManager manager_;
+    QFile file_;
+    QNetworkReply *reply_{};
+
+signals:
+    void downloadComplete();
+};
+
+}  // namespace

 namespace chatterino {

-std::map<ToastReaction, QString> Toasts::reactionToString = {
-    {ToastReaction::OpenInBrowser, OPEN_IN_BROWSER},
-    {ToastReaction::OpenInPlayer, OPEN_PLAYER_IN_BROWSER},
-    {ToastReaction::OpenInStreamlink, OPEN_IN_STREAMLINK},
-    {ToastReaction::DontOpen, DONT_OPEN}};
+#ifdef Q_OS_WIN
+using WinToastLib::WinToast;
+using WinToastLib::WinToastTemplate;
+#endif

 bool Toasts::isEnabled()
 {
 #ifdef Q_OS_WIN
-    return WinToastLib::WinToast::isCompatible() &&
-           getSettings()->notificationToast &&
+    return WinToast::isCompatible() && getSettings()->notificationToast &&
           !(isInStreamerMode() &&
             getSettings()->streamerModeSuppressLiveNotifications);
 #else
@ -49,24 +80,31 @@ bool Toasts::isEnabled()

 QString Toasts::findStringFromReaction(const ToastReaction &reaction)
 {
-    auto iterator = Toasts::reactionToString.find(reaction);
-    if (iterator != Toasts::reactionToString.end())
+    // The constants are macros right now, but we want to avoid ASCII casts,
+    // so we're concatenating them with a QString literal - effectively making them part of it.
+    switch (reaction)
    {
-        return iterator->second;
-    }
-    else
-    {
-        return DONT_OPEN;
+        case ToastReaction::OpenInBrowser:
+            return OPEN_IN_BROWSER u""_s;
+        case ToastReaction::OpenInPlayer:
+            return OPEN_PLAYER_IN_BROWSER u""_s;
+        case ToastReaction::OpenInStreamlink:
+            return OPEN_IN_STREAMLINK u""_s;
+        case ToastReaction::DontOpen:
+        default:
+            return DONT_OPEN u""_s;
    }
 }

 QString Toasts::findStringFromReaction(
-    const pajlada::Settings::Setting<int> &value)
+    const pajlada::Settings::Setting<int> &reaction)
 {
-    int i = static_cast<int>(value);
-    return Toasts::findStringFromReaction(static_cast<ToastReaction>(i));
+    static_assert(std::is_same_v<std::underlying_type_t<ToastReaction>, int>);
+    int value = reaction;
+    return Toasts::findStringFromReaction(static_cast<ToastReaction>(value));
 }

+// NOLINTNEXTLINE(readability-convert-member-functions-to-static)
 void Toasts::sendChannelNotification(const QString &channelName,
                                     const QString &channelTitle, Platform p)
 {
@ -75,6 +113,7 @@ void Toasts::sendChannelNotification(const QString &channelName,
        this->sendWindowsNotification(channelName, channelTitle, p);
    };
 #else
+    (void)channelTitle;
    auto sendChannelNotification = [] {
        // Unimplemented for macOS and Linux
    };
@ -82,9 +121,7 @@ void Toasts::sendChannelNotification(const QString &channelName,
    // Fetch user profile avatar
    if (p == Platform::Twitch)
    {
-        QFileInfo check_file(getPaths()->twitchProfileAvatars + "/twitch/" +
-                             channelName + ".png");
-        if (check_file.exists() && check_file.isFile())
+        if (hasAvatarForChannel(channelName))
        {
            sendChannelNotification();
        }
@ -93,10 +130,11 @@ void Toasts::sendChannelNotification(const QString &channelName,
            getHelix()->getUserByName(
                channelName,
                [channelName, sendChannelNotification](const auto &user) {
-                    DownloadManager *manager = new DownloadManager();
-                    manager->setFile(user.profileImageUrl, channelName);
-                    manager->connect(manager,
-                                     &DownloadManager::downloadComplete,
+                    // gets deleted when finished
+                    auto *downloader =
+                        new AvatarDownloader(user.profileImageUrl, channelName);
+                    QObject::connect(downloader,
+                                     &AvatarDownloader::downloadComplete,
                                     sendChannelNotification);
                },
                [] {
@ -116,13 +154,12 @@ private:

 public:
    CustomHandler(QString channelName, Platform p)
-        : channelName_(channelName)
+        : channelName_(std::move(channelName))
        , platform_(p)
    {
    }
-    void toastActivated() const
+    void toastActivated() const override
    {
-        QString link;
        auto toastReaction =
            static_cast<ToastReaction>(getSettings()->openFromToast.getValue());

@ -131,51 +168,74 @@ public:
            case ToastReaction::OpenInBrowser:
                if (platform_ == Platform::Twitch)
                {
-                    link = "https://www.twitch.tv/" + channelName_;
+                    QDesktopServices::openUrl(
+                        QUrl(u"https://www.twitch.tv/" % channelName_));
                }
-                QDesktopServices::openUrl(QUrl(link));
                break;
            case ToastReaction::OpenInPlayer:
                if (platform_ == Platform::Twitch)
                {
-                    link =
-                        "https://player.twitch.tv/?parent=twitch.tv&channel=" +
-                        channelName_;
+                    QDesktopServices::openUrl(QUrl(
+                        u"https://player.twitch.tv/?parent=twitch.tv&channel=" %
+                        channelName_));
                }
-                QDesktopServices::openUrl(QUrl(link));
                break;
            case ToastReaction::OpenInStreamlink: {
                openStreamlinkForChannel(channelName_);
                break;
            }
-                // the fourth and last option is "don't open"
-                // in this case obviously nothing should happen
+            case ToastReaction::DontOpen:
+                // nothing should happen
+                break;
        }
    }

-    void toastActivated(int actionIndex) const
+    void toastActivated(int actionIndex) const override
    {
    }

-    void toastFailed() const
+    void toastFailed() const override
    {
    }

-    void toastDismissed(WinToastDismissalReason state) const
+    void toastDismissed(WinToastDismissalReason state) const override
    {
    }
 };

+void Toasts::ensureInitialized()
+{
+    if (this->initialized_)
+    {
+        return;
+    }
+    this->initialized_ = true;
+
+    auto *instance = WinToast::instance();
+    instance->setAppName(L"Chatterino2");
+    instance->setAppUserModelId(
+        WinToast::configureAUMI(L"", L"Chatterino 2", L"",
+                                Version::instance().version().toStdWString()));
+    instance->setShortcutPolicy(WinToast::SHORTCUT_POLICY_IGNORE);
+    WinToast::WinToastError error{};
+    instance->initialize(&error);
+
+    if (error != WinToast::NoError)
+    {
+        qCDebug(chatterinoNotification)
+            << "Failed to initialize WinToast - error:" << error;
+    }
+}
+
 void Toasts::sendWindowsNotification(const QString &channelName,
                                     const QString &channelTitle, Platform p)
 {
-    WinToastLib::WinToastTemplate templ = WinToastLib::WinToastTemplate(
-        WinToastLib::WinToastTemplate::ImageAndText03);
-    QString str = channelName + " is live!";
-    std::string utf8_text = str.toUtf8().constData();
-    std::wstring widestr = std::wstring(utf8_text.begin(), utf8_text.end());
+    this->ensureInitialized();

-    templ.setTextField(widestr, WinToastLib::WinToastTemplate::FirstLine);
+    WinToastTemplate templ(WinToastTemplate::ImageAndText03);
+    QString str = channelName % u" is live!";
+
+    templ.setTextField(str.toStdWString(), WinToastTemplate::FirstLine);
    if (static_cast<ToastReaction>(getSettings()->openFromToast.getValue()) !=
        ToastReaction::DontOpen)
    {
@ -183,43 +243,68 @@ void Toasts::sendWindowsNotification(const QString &channelName,
            Toasts::findStringFromReaction(getSettings()->openFromToast);
        mode = mode.toLower();

-        templ.setTextField(QString("%1 \nClick to %2")
-                               .arg(channelTitle)
-                               .arg(mode)
-                               .toStdWString(),
-                           WinToastLib::WinToastTemplate::SecondLine);
+        templ.setTextField(
+            u"%1 \nClick to %2"_s.arg(channelTitle).arg(mode).toStdWString(),
+            WinToastTemplate::SecondLine);
    }

-    QString Path;
+    QString avatarPath;
    if (p == Platform::Twitch)
    {
-        Path = getPaths()->twitchProfileAvatars + "/twitch/" + channelName +
-               ".png";
+        avatarPath = avatarFilePath(channelName);
    }
-    std::string temp_Utf8 = Path.toUtf8().constData();
-    std::wstring imagePath = std::wstring(temp_Utf8.begin(), temp_Utf8.end());
-    templ.setImagePath(imagePath);
+    templ.setImagePath(avatarPath.toStdWString());
    if (getSettings()->notificationPlaySound)
    {
-        templ.setAudioOption(
-            WinToastLib::WinToastTemplate::AudioOption::Silent);
+        templ.setAudioOption(WinToastTemplate::AudioOption::Silent);
+    }
+
+    WinToast::WinToastError error = WinToast::NoError;
+    WinToast::instance()->showToast(templ, new CustomHandler(channelName, p),
+                                    &error);
+    if (error != WinToast::NoError)
+    {
+        qCWarning(chatterinoNotification) << "Failed to show toast:" << error;
    }
-    WinToastLib::WinToast::instance()->setAppName(L"Chatterino2");
-    int mbstowcs(wchar_t * aumi_version, const char *CHATTERINO_VERSION,
-                 size_t size);
-    std::string(CHATTERINO_VERSION);
-    std::wstring aumi_version =
-        std::wstring(CHATTERINO_VERSION.begin(), CHATTERINO_VERSION.end());
-    WinToastLib::WinToast::instance()->setAppUserModelId(
-        WinToastLib::WinToast::configureAUMI(L"", L"Chatterino 2", L"",
-                                             aumi_version));
-    WinToastLib::WinToast::instance()->setShortcutPolicy(
-        WinToastLib::WinToast::SHORTCUT_POLICY_IGNORE);
-    WinToastLib::WinToast::instance()->initialize();
-    WinToastLib::WinToast::instance()->showToast(
-        templ, new CustomHandler(channelName, p));
 }

 #endif

 }  // namespace chatterino
+
+namespace {
+
+AvatarDownloader::AvatarDownloader(const QString &avatarURL,
+                                   const QString &channelName)
+    : file_(avatarFilePath(channelName))
+{
+    if (!this->file_.open(QFile::WriteOnly | QFile::Truncate))
+    {
+        qCWarning(chatterinoNotification)
+            << "Failed to open avatar file" << this->file_.errorString();
+    }
+
+    this->reply_ = this->manager_.get(QNetworkRequest(avatarURL));
+
+    connect(this->reply_, &QNetworkReply::readyRead, this, [this] {
+        this->file_.write(this->reply_->readAll());
+    });
+    connect(this->reply_, &QNetworkReply::finished, this, [this] {
+        if (this->reply_->error() != QNetworkReply::NoError)
+        {
+            qCWarning(chatterinoNotification)
+                << "Failed to download avatar" << this->reply_->errorString();
+        }
+
+        if (this->file_.isOpen())
+        {
+            this->file_.close();
+        }
+        emit downloadComplete();
+        this->deleteLater();
+    });
+}
+
+#include "Toasts.moc"
+
+}  // namespace
--- a/src/singletons/Toasts.hpp
+++ b/src/singletons/Toasts.hpp
@ -24,14 +24,16 @@ public:
    static QString findStringFromReaction(const ToastReaction &reaction);
    static QString findStringFromReaction(
        const pajlada::Settings::Setting<int> &reaction);
-    static std::map<ToastReaction, QString> reactionToString;

    static bool isEnabled();

 private:
 #ifdef Q_OS_WIN
+    void ensureInitialized();
    void sendWindowsNotification(const QString &channelName,
                                 const QString &channelTitle, Platform p);
+
+    bool initialized_ = false;
 #endif
 };
 }  // namespace chatterino