Refactor Native Messages (#4738)

* refactor: move ipc queue into its own class

* refactor: move windows.h related functions to AW

* refactor: make NM-Client methods static

* refactor: json access

* refactor: use struct initializer

* refactor: move `handleMessage` to anon-namespace

* refactor: clean-up includes

* refactor: move action handler to functions

* refactor: cleanup `handleSelect`

* fix: cleanup clang-tidy warnings

* chore: simplify json

* revert: keep handlers as methods

This is more readable and extensible.

* fix: typo

* fix: namespace

* fix: rename define

* refactor: `IpcQueue` to be simpler

* fix: rename cmake option

* fix: use variant when constructing

* fix: make it a ref

* fix: its a pair now

src/Application.cpp

@@ -270,7 +270,7 @@ void Application::initNm(Paths &paths)
     (void)paths;
 
 #ifdef Q_OS_WIN
-#    if defined QT_NO_DEBUG || defined C_DEBUG_NM
+#    if defined QT_NO_DEBUG || defined CHATTERINO_DEBUG_NM
     registerNmHost(paths);
     this->nmServer.start();
 #    endif

src/BrowserExtension.cpp

@@ -30,7 +30,7 @@ namespace {
 #endif
     }
 
-    void runLoop(NativeMessagingClient &client)
+    void runLoop()
     {
         auto received_message = std::make_shared<std::atomic_bool>(true);
 
@@ -73,7 +73,7 @@ namespace {
 
             received_message->store(true);
 
-            client.sendMessage(data);
+            nm::client::sendMessage(data);
         }
         _Exit(0);
     }
@@ -83,9 +83,7 @@ void runBrowserExtensionHost()
 {
     initFileMode();
 
-    NativeMessagingClient client;
+    runLoop();
-
-    runLoop(client);
 }
 
 }  // namespace chatterino

src/CMakeLists.txt

@@ -3,6 +3,9 @@ set(VERSION_PROJECT "${LIBRARY_PROJECT}-version")
 set(EXECUTABLE_PROJECT "${PROJECT_NAME}")
 add_compile_definitions(QT_DISABLE_DEPRECATED_BEFORE=0x050F00)
 
+# registers the native messageing host
+option(CHATTERINO_DEBUG_NATIVE_MESSAGES "Debug native messages" OFF)
+
 set(SOURCE_FILES
         Application.cpp
         Application.hpp
@@ -408,6 +411,8 @@ set(SOURCE_FILES
         util/IncognitoBrowser.hpp
         util/InitUpdateButton.cpp
         util/InitUpdateButton.hpp
+        util/IpcQueue.cpp
+        util/IpcQueue.hpp
         util/LayoutHelper.cpp
         util/LayoutHelper.hpp
         util/NuulsUploader.cpp
@@ -847,6 +852,9 @@ if (WIN32)
         set_target_properties(${EXECUTABLE_PROJECT} PROPERTIES WIN32_EXECUTABLE TRUE)
     endif ()
 endif ()
+if (CHATTERINO_DEBUG_NATIVE_MESSAGES)
+    target_compile_definitions(${LIBRARY_PROJECT} PRIVATE CHATTERINO_DEBUG_NM)
+endif ()
 
 if (MSVC)
     target_compile_options(${LIBRARY_PROJECT} PUBLIC /EHsc /bigobj)

src/singletons/NativeMessaging.cpp

@@ -1,38 +1,38 @@
 #include "singletons/NativeMessaging.hpp"
 
 #include "Application.hpp"
+#include "common/Literals.hpp"
 #include "common/QLogging.hpp"
 #include "providers/twitch/TwitchIrcServer.hpp"
 #include "singletons/Paths.hpp"
+#include "util/IpcQueue.hpp"
 #include "util/PostToThread.hpp"
 
-#include <boost/date_time/posix_time/posix_time.hpp>
-#include <boost/interprocess/ipc/message_queue.hpp>
 #include <QCoreApplication>
 #include <QFile>
 #include <QJsonArray>
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QJsonValue>
-
+#include <QSettings>
-namespace ipc = boost::interprocess;
 
 #ifdef Q_OS_WIN
-// clang-format off
-#    include <QSettings>
-#    include <Windows.h>
-// clang-format on
-#    include "singletons/WindowManager.hpp"
 #    include "widgets/AttachedWindow.hpp"
 #endif
 
-#include <iostream>
+namespace {
 
-#define EXTENSION_ID "glknmaideaikkmemifbfkhnomoknepka"
+using namespace chatterino::literals;
-#define MESSAGE_SIZE 1024
+
+const QString EXTENSION_ID = u"glknmaideaikkmemifbfkhnomoknepka"_s;
+constexpr const size_t MESSAGE_SIZE = 1024;
+
+}  // namespace
 
 namespace chatterino {
 
+using namespace literals;
+
 void registerNmManifest(Paths &paths, const QString &manifestFilename,
                         const QString &registryKeyName,
                         const QJsonDocument &document);
@@ -40,47 +40,42 @@ void registerNmManifest(Paths &paths, const QString &manifestFilename,
 void registerNmHost(Paths &paths)
 {
     if (paths.isPortable())
+    {
         return;
+    }
 
-    auto getBaseDocument = [&] {
+    auto getBaseDocument = [] {
-        QJsonObject obj;
+        return QJsonObject{
-        obj.insert("name", "com.chatterino.chatterino");
+            {u"name"_s, "com.chatterino.chatterino"_L1},
-        obj.insert("description", "Browser interaction with chatterino.");
+            {u"description"_s, "Browser interaction with chatterino."_L1},
-        obj.insert("path", QCoreApplication::applicationFilePath());
+            {u"path"_s, QCoreApplication::applicationFilePath()},
-        obj.insert("type", "stdio");
+            {u"type"_s, "stdio"_L1},
-
+        };
-        return obj;
     };
 
     // chrome
     {
-        QJsonDocument document;
-
         auto obj = getBaseDocument();
-        QJsonArray allowed_origins_arr = {"chrome-extension://" EXTENSION_ID
+        QJsonArray allowedOriginsArr = {
-                                          "/"};
+            u"chrome-extension://%1/"_s.arg(EXTENSION_ID)};
-        obj.insert("allowed_origins", allowed_origins_arr);
+        obj.insert("allowed_origins", allowedOriginsArr);
-        document.setObject(obj);
 
         registerNmManifest(paths, "/native-messaging-manifest-chrome.json",
                            "HKCU\\Software\\Google\\Chrome\\NativeMessagingHost"
                            "s\\com.chatterino.chatterino",
-                           document);
+                           QJsonDocument(obj));
     }
 
     // firefox
     {
-        QJsonDocument document;
-
         auto obj = getBaseDocument();
-        QJsonArray allowed_extensions = {"chatterino_native@chatterino.com"};
+        QJsonArray allowedExtensions = {"chatterino_native@chatterino.com"};
-        obj.insert("allowed_extensions", allowed_extensions);
+        obj.insert("allowed_extensions", allowedExtensions);
-        document.setObject(obj);
 
         registerNmManifest(paths, "/native-messaging-manifest-firefox.json",
                            "HKCU\\Software\\Mozilla\\NativeMessagingHosts\\com."
                            "chatterino.chatterino",
-                           document);
+                           QJsonDocument(obj));
     }
 }
 
@@ -112,33 +107,27 @@ std::string &getNmQueueName(Paths &paths)
 
 // CLIENT
 
-void NativeMessagingClient::sendMessage(const QByteArray &array)
+namespace nm::client {
-{
-    try
-    {
-        ipc::message_queue messageQueue(ipc::open_only, "chatterino_gui");
 
-        messageQueue.try_send(array.data(), size_t(array.size()), 1);
+    void sendMessage(const QByteArray &array)
-        //        messageQueue.timed_send(array.data(), size_t(array.size()), 1,
-        //                                boost::posix_time::second_clock::local_time() +
-        //                                    boost::posix_time::seconds(10));
-    }
-    catch (ipc::interprocess_exception &ex)
     {
-        qCDebug(chatterinoNativeMessage) << "send to gui process:" << ex.what();
+        ipc::sendMessage("chatterino_gui", array);
-    }
     }
 
-void NativeMessagingClient::writeToCout(const QByteArray &array)
+    void writeToCout(const QByteArray &array)
     {
-    auto *data = array.data();
+        const auto *data = array.data();
         auto size = uint32_t(array.size());
 
+        // We're writing the raw bytes to cout.
+        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
         std::cout.write(reinterpret_cast<char *>(&size), 4);
         std::cout.write(data, size);
         std::cout.flush();
     }
 
+}  // namespace nm::client
+
 // SERVER
 
 void NativeMessagingServer::start()
@@ -148,93 +137,87 @@ void NativeMessagingServer::start()
 
 void NativeMessagingServer::ReceiverThread::run()
 {
-    try
+    auto [messageQueue, error] =
+        ipc::IpcQueue::tryReplaceOrCreate("chatterino_gui", 100, MESSAGE_SIZE);
+
+    if (!error.isEmpty())
     {
-        ipc::message_queue::remove("chatterino_gui");
+        qCDebug(chatterinoNativeMessage)
-        ipc::message_queue messageQueue(ipc::open_or_create, "chatterino_gui",
+            << "Failed to create message queue:" << error;
-                                        100, MESSAGE_SIZE);
+
+        nmIpcError().set(error);
+        return;
+    }
 
     while (true)
     {
-            try
+        auto buf = messageQueue->receive();
+        if (buf.isEmpty())
         {
-                auto buf = std::make_unique<char[]>(MESSAGE_SIZE);
+            continue;
-                auto retSize = ipc::message_queue::size_type();
+        }
-                auto priority = static_cast<unsigned int>(0);
+        auto document = QJsonDocument::fromJson(buf);
-
-                messageQueue.receive(buf.get(), MESSAGE_SIZE, retSize,
-                                     priority);
-
-                auto document = QJsonDocument::fromJson(
-                    QByteArray::fromRawData(buf.get(), retSize));
 
         this->handleMessage(document.object());
     }
-            catch (ipc::interprocess_exception &ex)
-            {
-                qCDebug(chatterinoNativeMessage)
-                    << "received from gui process:" << ex.what();
-            }
-        }
-    }
-    catch (ipc::interprocess_exception &ex)
-    {
-        qCDebug(chatterinoNativeMessage)
-            << "run ipc message queue:" << ex.what();
-
-        nmIpcError().set(QString::fromLatin1(ex.what()));
-    }
 }
 
 void NativeMessagingServer::ReceiverThread::handleMessage(
     const QJsonObject &root)
 {
-    QString action = root.value("action").toString();
+    QString action = root["action"_L1].toString();
-
-    if (action.isNull())
-    {
-        qCDebug(chatterinoNativeMessage) << "NM action was null";
-        return;
-    }
 
     if (action == "select")
     {
-        QString _type = root.value("type").toString();
+        this->handleSelect(root);
-        bool attach = root.value("attach").toBool();
+        return;
-        bool attachFullscreen = root.value("attach_fullscreen").toBool();
+    }
-        QString name = root.value("name").toString();
+    if (action == "detach")
-
-#ifdef USEWINSDK
-        AttachedWindow::GetArgs args;
-        args.winId = root.value("winId").toString();
-        args.yOffset = root.value("yOffset").toInt(-1);
-
     {
-            const auto sizeObject = root.value("size").toObject();
+        this->handleDetach(root);
-            args.x = sizeObject.value("x").toDouble(-1.0);
+        return;
-            args.pixelRatio = sizeObject.value("pixelRatio").toDouble(-1.0);
-            args.width = sizeObject.value("width").toInt(-1);
-            args.height = sizeObject.value("height").toInt(-1);
     }
 
-        args.fullscreen = attachFullscreen;
+    qCDebug(chatterinoNativeMessage) << "NM unknown action" << action;
+}
 
-        qCDebug(chatterinoNativeMessage)
+// NOLINTBEGIN(readability-convert-member-functions-to-static)
-            << args.x << args.pixelRatio << args.width << args.height
+void NativeMessagingServer::ReceiverThread::handleSelect(
-            << args.winId;
+    const QJsonObject &root)
-
-        if (_type.isNull() || args.winId.isNull())
 {
+    QString type = root["type"_L1].toString();
+    bool attach = root["attach"_L1].toBool();
+    bool attachFullscreen = root["attach_fullscreen"_L1].toBool();
+    QString name = root["name"_L1].toString();
+
+#ifdef USEWINSDK
+    const auto sizeObject = root["size"_L1].toObject();
+    AttachedWindow::GetArgs args = {
+        .winId = root["winId"_L1].toString(),
+        .yOffset = root["yOffset"_L1].toInt(-1),
+        .x = sizeObject["x"_L1].toDouble(-1.0),
+        .pixelRatio = sizeObject["pixelRatio"_L1].toDouble(-1.0),
+        .width = sizeObject["width"_L1].toInt(-1),
+        .height = sizeObject["height"_L1].toInt(-1),
+        .fullscreen = attachFullscreen,
+    };
+
     qCDebug(chatterinoNativeMessage)
-                << "NM type, name or winId missing";
+        << args.x << args.pixelRatio << args.width << args.height << args.winId;
-            attach = false;
+
-            attachFullscreen = false;
+    if (args.winId.isNull())
+    {
+        qCDebug(chatterinoNativeMessage) << "winId in select is missing";
         return;
     }
 #endif
 
-        if (_type == "twitch")
+    if (type != u"twitch"_s)
     {
+        qCDebug(chatterinoNativeMessage) << "NM unknown channel type";
+        return;
+    }
+
     postToThread([=] {
         auto *app = getApp();
 
@@ -250,8 +233,7 @@ void NativeMessagingServer::ReceiverThread::handleMessage(
         if (attach || attachFullscreen)
         {
 #ifdef USEWINSDK
-                    auto *window =
+            auto *window = AttachedWindow::getForeground(args);
-                        AttachedWindow::get(::GetForegroundWindow(), args);
             if (!name.isEmpty())
             {
                 window->setChannel(app->twitch->getOrAddChannel(name));
@@ -260,14 +242,11 @@ void NativeMessagingServer::ReceiverThread::handleMessage(
         }
     });
 }
-        else
+
+void NativeMessagingServer::ReceiverThread::handleDetach(
+    const QJsonObject &root)
 {
-            qCDebug(chatterinoNativeMessage) << "NM unknown channel type";
+    QString winId = root["winId"_L1].toString();
-        }
-    }
-    else if (action == "detach")
-    {
-        QString winId = root.value("winId").toString();
 
     if (winId.isNull())
     {
@@ -282,11 +261,7 @@ void NativeMessagingServer::ReceiverThread::handleMessage(
     });
 #endif
 }
-    else
+// NOLINTEND(readability-convert-member-functions-to-static)
-    {
-        qCDebug(chatterinoNativeMessage) << "NM unknown action " + action;
-    }
-}
 
 Atomic<boost::optional<QString>> &nmIpcError()
 {

src/singletons/NativeMessaging.hpp

@@ -16,12 +16,12 @@ std::string &getNmQueueName(Paths &paths);
 
 Atomic<boost::optional<QString>> &nmIpcError();
 
-class NativeMessagingClient final
+namespace nm::client {
-{
+
-public:
     void sendMessage(const QByteArray &array);
     void writeToCout(const QByteArray &array);
-};
+
+}  // namespace nm::client
 
 class NativeMessagingServer final
 {
@@ -36,6 +36,8 @@ private:
 
     private:
         void handleMessage(const QJsonObject &root);
+        void handleSelect(const QJsonObject &root);
+        void handleDetach(const QJsonObject &root);
     };
 
     ReceiverThread thread;

src/util/IpcQueue.cpp

@@ -0,0 +1,87 @@
+#include "util/IpcQueue.hpp"
+
+#include "common/QLogging.hpp"
+
+#include <boost/interprocess/ipc/message_queue.hpp>
+#include <QByteArray>
+#include <QString>
+#include <QtGlobal>
+
+namespace boost_ipc = boost::interprocess;
+
+namespace chatterino::ipc {
+
+void sendMessage(const char *name, const QByteArray &data)
+{
+    try
+    {
+        boost_ipc::message_queue messageQueue(boost_ipc::open_only, name);
+
+        messageQueue.try_send(data.data(), size_t(data.size()), 1);
+    }
+    catch (boost_ipc::interprocess_exception &ex)
+    {
+        qCDebug(chatterinoNativeMessage)
+            << "Failed to send message:" << ex.what();
+    }
+}
+
+class IpcQueuePrivate
+{
+public:
+    IpcQueuePrivate(const char *name, size_t maxMessages, size_t maxMessageSize)
+        : queue(boost_ipc::open_or_create, name, maxMessages, maxMessageSize)
+    {
+    }
+
+    boost_ipc::message_queue queue;
+};
+
+IpcQueue::IpcQueue(IpcQueuePrivate *priv)
+    : private_(priv){};
+IpcQueue::~IpcQueue() = default;
+
+std::pair<std::unique_ptr<IpcQueue>, QString> IpcQueue::tryReplaceOrCreate(
+    const char *name, size_t maxMessages, size_t maxMessageSize)
+{
+    try
+    {
+        boost_ipc::message_queue::remove(name);
+        return std::make_pair(
+            std::unique_ptr<IpcQueue>(new IpcQueue(
+                new IpcQueuePrivate(name, maxMessages, maxMessageSize))),
+            QString());
+    }
+    catch (boost_ipc::interprocess_exception &ex)
+    {
+        return {nullptr, QString::fromLatin1(ex.what())};
+    }
+}
+
+QByteArray IpcQueue::receive()
+{
+    try
+    {
+        auto *d = this->private_.get();
+
+        QByteArray buf;
+        // The new storage is uninitialized
+        buf.resize(static_cast<qsizetype>(d->queue.get_max_msg_size()));
+
+        size_t messageSize = 0;
+        unsigned int priority = 0;
+        d->queue.receive(buf.data(), buf.size(), messageSize, priority);
+
+        // truncate to the initialized storage
+        buf.truncate(static_cast<qsizetype>(messageSize));
+        return buf;
+    }
+    catch (boost_ipc::interprocess_exception &ex)
+    {
+        qCDebug(chatterinoNativeMessage)
+            << "Failed to receive message:" << ex.what();
+    }
+    return {};
+}
+
+}  // namespace chatterino::ipc

src/util/IpcQueue.hpp

@@ -0,0 +1,35 @@
+#pragma once
+
+#include <memory>
+#include <utility>
+
+class QByteArray;
+class QString;
+
+namespace chatterino::ipc {
+
+void sendMessage(const char *name, const QByteArray &data);
+
+class IpcQueuePrivate;
+class IpcQueue
+{
+public:
+    ~IpcQueue();
+
+    static std::pair<std::unique_ptr<IpcQueue>, QString> tryReplaceOrCreate(
+        const char *name, size_t maxMessages, size_t maxMessageSize);
+
+    // TODO: use std::expected
+    /// Try to receive a message.
+    /// In the case of an error, the buffer is empty.
+    QByteArray receive();
+
+private:
+    IpcQueue(IpcQueuePrivate *priv);
+
+    std::unique_ptr<IpcQueuePrivate> private_;
+
+    friend class IpcQueuePrivate;
+};
+
+}  // namespace chatterino::ipc

src/widgets/AttachedWindow.cpp

@@ -136,6 +136,13 @@ AttachedWindow *AttachedWindow::get(void *target, const GetArgs &args)
     return window;
 }
 
+#ifdef USEWINSDK
+AttachedWindow *AttachedWindow::getForeground(const GetArgs &args)
+{
+    return AttachedWindow::get(::GetForegroundWindow(), args);
+}
+#endif
+
 void AttachedWindow::detach(const QString &winId)
 {
     for (Item &item : items)

src/widgets/AttachedWindow.hpp

@@ -31,6 +31,9 @@ public:
     virtual ~AttachedWindow() override;
 
     static AttachedWindow *get(void *target_, const GetArgs &args);
+#ifdef USEWINSDK
+    static AttachedWindow *getForeground(const GetArgs &args);
+#endif
     static void detach(const QString &winId);
 
     void setChannel(ChannelPtr channel);