[Libreoffice-commits] core.git: Branch 'distro/lhm/libreoffice-5-2+backports' - 2 commits - vcl/unx

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Mon Oct 8 13:00:09 UTC 2018


 vcl/unx/gtk3_kde5/filepicker_ipc_commands.hxx |   12 +
 vcl/unx/gtk3_kde5/kde5_filepicker_ipc.cxx     |  229 +++++++++++++++++++-------
 vcl/unx/gtk3_kde5/kde5_filepicker_ipc.hxx     |   18 +-
 3 files changed, 198 insertions(+), 61 deletions(-)

New commits:
commit 22c76a35b95f8113fceabacf96ec017bffc9a0ac
Author:     Michael Weghorn <m.weghorn at posteo.de>
AuthorDate: Tue Oct 2 16:08:35 2018 +0200
Commit:     Michael Weghorn <m.weghorn at posteo.de>
CommitDate: Mon Oct 8 15:00:00 2018 +0200

    tdf#120261 gtk3_kde5: Read IPC cmds in own thread on kde5 side
    
    Move reading IPC commands and arguments to those commands
    to a separate thread and use the signal/slot mechanism
    to notify 'FilePickerIpc' whenever a new command and its
    arguments have been read.
    
    This allows to handle the events of other commands that have been
    received while the dialog is being executed.
    This will be needed by a subsequent change that will modify
    how IPC is handled on the gtk3 side.
    
     Conflicts:
            vcl/unx/gtk3_kde5/kde5_filepicker_ipc.cxx
    
    Reviewed-on: https://gerrit.libreoffice.org/61252
    Tested-by: Jenkins
    Reviewed-by: Michael Weghorn <m.weghorn at posteo.de>
    (cherry picked from commit 3b36855e48f6ef3f4e52e998c52c531fe5191477)
    
    Change-Id: Ia77b21045b0196710cbe164fb640b36a128d5081
    Reviewed-on: https://gerrit.libreoffice.org/61533
    Reviewed-by: Michael Weghorn <m.weghorn at posteo.de>
    Tested-by: Michael Weghorn <m.weghorn at posteo.de>

diff --git a/vcl/unx/gtk3_kde5/kde5_filepicker_ipc.cxx b/vcl/unx/gtk3_kde5/kde5_filepicker_ipc.cxx
index d023e4461448..803e5bedd853 100644
--- a/vcl/unx/gtk3_kde5/kde5_filepicker_ipc.cxx
+++ b/vcl/unx/gtk3_kde5/kde5_filepicker_ipc.cxx
@@ -27,7 +27,6 @@
 
 #include <iostream>
 
-#include "filepicker_ipc_commands.hxx"
 #include "kde5_filepicker.hxx"
 
 #include <rtl/ustring.h>
@@ -53,49 +52,180 @@ void sendIpcArg(std::ostream& stream, const QStringList& list)
     }
 }
 
-FilePickerIpc::FilePickerIpc(KDE5FilePicker* filePicker, QObject* _parent)
-    : QObject(_parent)
-    , m_filePicker(filePicker)
-    , m_stdinNotifier(new QSocketNotifier(fileno(stdin), QSocketNotifier::Read, this))
+void readCommandArgs(Commands command, QList<QVariant>& args)
 {
-    connect(m_stdinNotifier, &QSocketNotifier::activated, this, &FilePickerIpc::readCommands);
+    switch (command)
+    {
+        case Commands::SetTitle:
+        {
+            QString title;
+            readIpcArgs(std::cin, title);
+            args.append(title);
+            break;
+        }
+        case Commands::SetWinId:
+        {
+            sal_uIntPtr winId = 0;
+            readIpcArgs(std::cin, winId);
+            QVariant aWinIdVariant;
+            aWinIdVariant.setValue(winId);
+            args.append(aWinIdVariant);
+            break;
+        }
+        case Commands::SetMultiSelectionMode:
+        {
+            bool multiSelection = false;
+            readIpcArgs(std::cin, multiSelection);
+            args.append(multiSelection);
+            break;
+        }
+        case Commands::SetDefaultName:
+        {
+            QString name;
+            readIpcArgs(std::cin, name);
+            args.append(name);
+            break;
+        }
+        case Commands::SetDisplayDirectory:
+        {
+            QString dir;
+            readIpcArgs(std::cin, dir);
+            args.append(dir);
+            break;
+        }
+        case Commands::AppendFilter:
+        {
+            QString title, filter;
+            readIpcArgs(std::cin, title, filter);
+            args.append(title);
+            args.append(filter);
+            break;
+        }
+        case Commands::SetCurrentFilter:
+        {
+            QString title;
+            readIpcArgs(std::cin, title);
+            args.append(title);
+            break;
+        }
+        case Commands::SetValue:
+        {
+            sal_Int16 controlId = 0;
+            sal_Int16 nControlAction = 0;
+            bool value = false;
+            readIpcArgs(std::cin, controlId, nControlAction, value);
+            args.append(controlId);
+            args.append(nControlAction);
+            args.append(value);
+            break;
+        }
+        case Commands::GetValue:
+        {
+            sal_Int16 controlId = 0;
+            sal_Int16 nControlAction = 0;
+            readIpcArgs(std::cin, controlId, nControlAction);
+            args.append(controlId);
+            args.append(nControlAction);
+            break;
+        }
+        case Commands::EnableControl:
+        {
+            sal_Int16 controlId = 0;
+            bool enabled = false;
+            readIpcArgs(std::cin, controlId, enabled);
+            args.append(controlId);
+            args.append(enabled);
+            break;
+        }
+        case Commands::SetLabel:
+        {
+            sal_Int16 controlId = 0;
+            QString label;
+            readIpcArgs(std::cin, controlId, label);
+            args.append(controlId);
+            args.append(label);
+            break;
+        }
+        case Commands::GetLabel:
+        {
+            sal_Int16 controlId = 0;
+            readIpcArgs(std::cin, controlId);
+            args.append(controlId);
+            break;
+        }
+        case Commands::AddCheckBox:
+        {
+            sal_Int16 controlId = 0;
+            bool hidden = false;
+            QString label;
+            readIpcArgs(std::cin, controlId, hidden, label);
+            args.append(controlId);
+            args.append(hidden);
+            args.append(label);
+            break;
+        }
+        case Commands::Initialize:
+        {
+            bool saveDialog = false;
+            readIpcArgs(std::cin, saveDialog);
+            args.append(saveDialog);
+            break;
+        }
+        default:
+        {
+            // no extra parameters/arguments
+            break;
+        }
+    };
 }
 
-FilePickerIpc::~FilePickerIpc() = default;
-
-void FilePickerIpc::readCommands()
+void readCommands(FilePickerIpc* ipc)
 {
-    // don't trigger again, loop runs until all is done
-    disconnect(m_stdinNotifier, &QSocketNotifier::activated, this, &FilePickerIpc::readCommands);
-
-    while (readCommand())
+    while (!std::cin.eof())
     {
-        // read next command
+        uint64_t messageId = 0;
+        Commands command;
+        readIpcArgs(std::cin, messageId, command);
+
+        // retrieve additional command-specific arguments
+        QList<QVariant> args;
+        readCommandArgs(command, args);
+
+        emit ipc->commandReceived(messageId, command, args);
     }
 }
 
-bool FilePickerIpc::readCommand()
+FilePickerIpc::FilePickerIpc(KDE5FilePicker* filePicker, QObject* parent)
+    : QObject(parent)
+    , m_filePicker(filePicker)
 {
-    if (std::cin.eof())
-        return false;
+    // required to be able to pass those via signal/slot
+    qRegisterMetaType<uint64_t>("uint64_t");
+    qRegisterMetaType<Commands>("Commands");
 
-    uint64_t messageId = 0;
-    Commands command;
-    readIpcArgs(std::cin, messageId, command);
+    connect(this, &FilePickerIpc::commandReceived, this, &FilePickerIpc::handleCommand);
 
+    // read IPC commands and their args in a separate thread, so this does not block everything else;
+    // 'commandReceived' signal is emitted every time a command and its args have been read;
+    // thread will run until the filepicker process is terminated
+    m_ipcReaderThread = std::unique_ptr<std::thread>{ new std::thread(readCommands, this) };
+}
+
+FilePickerIpc::~FilePickerIpc() = default;
+
+bool FilePickerIpc::handleCommand(uint64_t messageId, Commands command, QList<QVariant> args)
+{
     switch (command)
     {
         case Commands::SetTitle:
         {
-            QString title;
-            readIpcArgs(std::cin, title);
+            QString title = args.takeFirst().toString();
             m_filePicker->setTitle(title);
             return true;
         }
         case Commands::SetWinId:
         {
-            sal_uIntPtr winId = 0;
-            readIpcArgs(std::cin, winId);
+            sal_uIntPtr winId = args.takeFirst().value<sal_uIntPtr>();
             m_filePicker->setWinId(winId);
             return true;
         }
@@ -106,22 +236,19 @@ bool FilePickerIpc::readCommand()
         }
         case Commands::SetMultiSelectionMode:
         {
-            bool multiSelection = false;
-            readIpcArgs(std::cin, multiSelection);
+            bool multiSelection = args.takeFirst().toBool();
             m_filePicker->setMultiSelectionMode(multiSelection);
             return true;
         }
         case Commands::SetDefaultName:
         {
-            QString name;
-            readIpcArgs(std::cin, name);
+            QString name = args.takeFirst().toString();
             m_filePicker->setDefaultName(name);
             return true;
         }
         case Commands::SetDisplayDirectory:
         {
-            QString dir;
-            readIpcArgs(std::cin, dir);
+            QString dir = args.takeFirst().toString();
             m_filePicker->setDisplayDirectory(dir);
             return true;
         }
@@ -155,15 +282,14 @@ bool FilePickerIpc::readCommand()
         }
         case Commands::AppendFilter:
         {
-            QString title, filter;
-            readIpcArgs(std::cin, title, filter);
+            QString title = args.takeFirst().toString();
+            QString filter = args.takeFirst().toString();
             m_filePicker->appendFilter(title, filter);
             return true;
         }
         case Commands::SetCurrentFilter:
         {
-            QString title;
-            readIpcArgs(std::cin, title);
+            QString title = args.takeFirst().toString();
             m_filePicker->setCurrentFilter(title);
             return true;
         }
@@ -174,57 +300,50 @@ bool FilePickerIpc::readCommand()
         }
         case Commands::SetValue:
         {
-            sal_Int16 controlId = 0;
-            sal_Int16 nControlAction = 0;
-            bool value = false;
-            readIpcArgs(std::cin, controlId, nControlAction, value);
+            sal_Int16 controlId = args.takeFirst().value<sal_Int16>();
+            sal_Int16 nControlAction = args.takeFirst().value<sal_Int16>();
+            bool value = args.takeFirst().toBool();
             m_filePicker->setValue(controlId, nControlAction, value);
             return true;
         }
         case Commands::GetValue:
         {
-            sal_Int16 controlId = 0;
-            sal_Int16 nControlAction = 0;
-            readIpcArgs(std::cin, controlId, nControlAction);
+            sal_Int16 controlId = args.takeFirst().value<sal_Int16>();
+            sal_Int16 nControlAction = args.takeFirst().value<sal_Int16>();
             sendIpcArgs(std::cout, messageId, m_filePicker->getValue(controlId, nControlAction));
             return true;
         }
         case Commands::EnableControl:
         {
-            sal_Int16 controlId = 0;
-            bool enabled = false;
-            readIpcArgs(std::cin, controlId, enabled);
+            sal_Int16 controlId = args.takeFirst().value<sal_Int16>();
+            bool enabled = args.takeFirst().toBool();
             m_filePicker->enableControl(controlId, enabled);
             return true;
         }
         case Commands::SetLabel:
         {
-            sal_Int16 controlId = 0;
-            QString label;
-            readIpcArgs(std::cin, controlId, label);
+            sal_Int16 controlId = args.takeFirst().value<sal_Int16>();
+            QString label = args.takeFirst().toString();
             m_filePicker->setLabel(controlId, label);
             return true;
         }
         case Commands::GetLabel:
         {
-            sal_Int16 controlId = 0;
-            readIpcArgs(std::cin, controlId);
+            sal_Int16 controlId = args.takeFirst().value<sal_Int16>();
             sendIpcArgs(std::cout, messageId, m_filePicker->getLabel(controlId));
             return true;
         }
         case Commands::AddCheckBox:
         {
-            sal_Int16 controlId = 0;
-            bool hidden = false;
-            QString label;
-            readIpcArgs(std::cin, controlId, hidden, label);
+            sal_Int16 controlId = args.takeFirst().value<sal_Int16>();
+            bool hidden = args.takeFirst().toBool();
+            QString label = args.takeFirst().toString();
             m_filePicker->addCheckBox(controlId, label, hidden);
             return true;
         }
         case Commands::Initialize:
         {
-            bool saveDialog = false;
-            readIpcArgs(std::cin, saveDialog);
+            bool saveDialog = args.takeFirst().toBool();
             m_filePicker->initialize(saveDialog);
             return true;
         }
diff --git a/vcl/unx/gtk3_kde5/kde5_filepicker_ipc.hxx b/vcl/unx/gtk3_kde5/kde5_filepicker_ipc.hxx
index ff5abd250a2a..fa9be696c565 100644
--- a/vcl/unx/gtk3_kde5/kde5_filepicker_ipc.hxx
+++ b/vcl/unx/gtk3_kde5/kde5_filepicker_ipc.hxx
@@ -19,8 +19,13 @@
 
 #pragma once
 
+#include <memory>
+#include <thread>
+
 #include <QObject>
 
+#include "filepicker_ipc_commands.hxx"
+
 class KDE5FilePicker;
 class WinIdEmbedder;
 class QSocketNotifier;
@@ -32,14 +37,15 @@ public:
     explicit FilePickerIpc(KDE5FilePicker* filePicker, QObject* parent = nullptr);
     ~FilePickerIpc() override;
 
-private Q_SLOTS:
-    void readCommands();
-
 private:
-    bool readCommand();
-
     KDE5FilePicker* m_filePicker = nullptr;
-    QSocketNotifier* m_stdinNotifier = nullptr;
+    std::unique_ptr<std::thread> m_ipcReaderThread;
+
+private Q_SLOTS:
+    bool handleCommand(uint64_t messageId, Commands command, QList<QVariant> args);
+
+Q_SIGNALS:
+    bool commandReceived(uint64_t messageId, Commands command, QList<QVariant> args);
 };
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
commit 27c2adfdbcda6e4bc1026ae7be93e4499c27e1db
Author:     Katarina Behrens <Katarina.Behrens at cib.de>
AuthorDate: Fri Jun 8 15:04:13 2018 +0200
Commit:     Michael Weghorn <m.weghorn at posteo.de>
CommitDate: Mon Oct 8 14:59:48 2018 +0200

    Add overloaded read/send funcs for uint64_t (conditionally)
    
    otherwise build fails on 32bit platforms as multiple matching
    overloaded func are found. Thanks _rene_ and mst_ for helping
    me to figure this out
    
    Change-Id: Ief4571ad735ad4efea9ddc70daea45885e5954c7
    Reviewed-on: https://gerrit.libreoffice.org/55474
    Reviewed-by: Rene Engelhard <rene at debian.org>
    Tested-by: Rene Engelhard <rene at debian.org>
    Reviewed-by: Katarina Behrens <Katarina.Behrens at cib.de>
    Tested-by: Katarina Behrens <Katarina.Behrens at cib.de>
    (cherry picked from commit ca4dede186183ca760013ad1885947d68bae02f4)
    Reviewed-on: https://gerrit.libreoffice.org/61532
    Reviewed-by: Michael Weghorn <m.weghorn at posteo.de>
    Tested-by: Michael Weghorn <m.weghorn at posteo.de>

diff --git a/vcl/unx/gtk3_kde5/filepicker_ipc_commands.hxx b/vcl/unx/gtk3_kde5/filepicker_ipc_commands.hxx
index 8ce8cfa9e78d..1acef8caf48e 100644
--- a/vcl/unx/gtk3_kde5/filepicker_ipc_commands.hxx
+++ b/vcl/unx/gtk3_kde5/filepicker_ipc_commands.hxx
@@ -100,6 +100,14 @@ inline void readIpcArg(std::istream& stream, sal_uIntPtr& value)
     stream.ignore(); // skip space
 }
 
+#if SAL_TYPES_SIZEOFPOINTER == 4
+inline void readIpcArg(std::istream& stream, uint64_t& value)
+{
+    stream >> value;
+    stream.ignore(); // skip space
+}
+#endif
+
 inline void readIpcArgs(std::istream& /*stream*/)
 {
     // end of arguments, nothing to do
@@ -135,6 +143,10 @@ inline void sendIpcArg(std::ostream& stream, sal_Int16 value) { stream << value
 
 inline void sendIpcArg(std::ostream& stream, sal_uIntPtr value) { stream << value << ' '; }
 
+#if SAL_TYPES_SIZEOFPOINTER == 4
+inline void sendIpcArg(std::ostream& stream, uint64_t value) { stream << value << ' '; }
+#endif
+
 inline void sendIpcArgsImpl(std::ostream& stream)
 {
     // end of arguments, flush stream


More information about the Libreoffice-commits mailing list