From 1addcf631a0ad475d42415fcf5c0c0ef7c7b5bc7 Mon Sep 17 00:00:00 2001 From: Roland Winklmeier Date: Tue, 16 Oct 2018 17:17:43 +0200 Subject: [PATCH] Refactor the remote hotkeys to avoid round trips The previous implementation was hard to follow and maintain. Instead of doing intentional rounds trips, we use now a two way approach. GUI is automatically forwarding remote actions by calling "callHotkeyActionRemotely" through DBus. Core on the other hand, emits a signal "remoteHotkeyAction" that is processed in a different function in GUI. On both sides, actions from the same local machine are filtered. ref T402 --- src/blackcore/context/contextapplication.cpp | 14 ++------------ src/blackcore/context/contextapplication.h | 2 +- .../context/contextapplicationempty.h | 2 +- .../context/contextapplicationimpl.cpp | 18 +++++++++++++++--- src/blackcore/context/contextapplicationimpl.h | 2 +- .../context/contextapplicationproxy.cpp | 14 ++++++++++++-- .../context/contextapplicationproxy.h | 4 +++- src/blackcore/inputmanager.cpp | 4 ++-- src/blackcore/inputmanager.h | 2 +- 9 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/blackcore/context/contextapplication.cpp b/src/blackcore/context/contextapplication.cpp index ec77da2e1..36add582c 100644 --- a/src/blackcore/context/contextapplication.cpp +++ b/src/blackcore/context/contextapplication.cpp @@ -112,23 +112,13 @@ namespace BlackCore s = connect(sApp->getInputManager(), &CInputManager::remoteActionFromLocal, this, [ = ](const QString & action, bool argument) { if (!myself) { return; } - this->callHotkeyAction(action, argument, {}); + this->callHotkeyActionRemotely(action, argument, {}); }, Qt::QueuedConnection); Q_ASSERT_X(s, Q_FUNC_INFO, "Connect remote action failed"); Q_UNUSED(s); - s = connect(this, &IContextApplication::remoteHotkeyAction, [ = ](const QString & action, bool argument, const CIdentifier & origin) - { - if (!myself) { return; } - if (origin.isFromLocalMachine()) { return; } - sApp->getInputManager()->callFunctionsBy(action, argument); - CLogMessage(this, CLogCategory::contextSlot()).debug() << "Calling function" << action << "from origin" << origin.getMachineName(); - }); - Q_ASSERT_X(s, Q_FUNC_INFO, "Connect remote hotkey action failed"); - Q_UNUSED(s); - // Enable event forwarding from GUI process to core sApp->getInputManager()->setForwarding(true); } @@ -158,7 +148,7 @@ namespace BlackCore qFatal("Not implemented"); // avoid losing a change during context interface construction } - void IContextApplication::callHotkeyAction(const QString &action, bool argument, const CIdentifier &origin) + void IContextApplication::callHotkeyActionRemotely(const QString &action, bool argument, const CIdentifier &origin) { Q_UNUSED(action); Q_UNUSED(argument); diff --git a/src/blackcore/context/contextapplication.h b/src/blackcore/context/contextapplication.h index e8b9584c5..78bda1743 100644 --- a/src/blackcore/context/contextapplication.h +++ b/src/blackcore/context/contextapplication.h @@ -182,7 +182,7 @@ namespace BlackCore //! Call a hotkey action on a remote process //! \note Not pure because it can be called from the base class constructor. //! \note This is the function which relays action calls via DBus - virtual void callHotkeyAction(const QString &action, bool argument, const BlackMisc::CIdentifier &origin); + virtual void callHotkeyActionRemotely(const QString &action, bool argument, const BlackMisc::CIdentifier &origin); //! Register application, can also be used for ping virtual BlackMisc::CIdentifier registerApplication(const BlackMisc::CIdentifier &application) = 0; diff --git a/src/blackcore/context/contextapplicationempty.h b/src/blackcore/context/contextapplicationempty.h index 9864a4b02..f3c91030a 100644 --- a/src/blackcore/context/contextapplicationempty.h +++ b/src/blackcore/context/contextapplicationempty.h @@ -135,7 +135,7 @@ namespace BlackCore } //! \copydoc IContextApplication::callHotkeyAction - virtual void callHotkeyAction(const QString &action, bool argument, const BlackMisc::CIdentifier &origin) override + virtual void callHotkeyActionRemotely(const QString &action, bool argument, const BlackMisc::CIdentifier &origin) override { Q_UNUSED(action); Q_UNUSED(argument); diff --git a/src/blackcore/context/contextapplicationimpl.cpp b/src/blackcore/context/contextapplicationimpl.cpp index 7044f7c13..3052c14ba 100644 --- a/src/blackcore/context/contextapplicationimpl.cpp +++ b/src/blackcore/context/contextapplicationimpl.cpp @@ -131,10 +131,22 @@ namespace BlackCore emit this->hotkeyActionsRegistered(actions, origin); } - void CContextApplication::callHotkeyAction(const QString &action, bool argument, const CIdentifier &origin) + void CContextApplication::callHotkeyActionRemotely(const QString &action, bool argument, const CIdentifier &origin) { - // Intentionally don't check for round trip here - emit this->remoteHotkeyAction(action, argument, origin); + if (origin.hasApplicationProcessId()) + { + // If it originated from this process, then we are going to emit a signal + emit this->remoteHotkeyAction(action, argument, origin); + } + else + { + // action came from a different process but on the same machine. Ignore + if (origin.isFromLocalMachine()) { return; } + + // Different process and different machine. Process it. + // However, it should not emit a remote action itself. + sApp->getInputManager()->callFunctionsBy(action, argument, false); + } } bool CContextApplication::writeToFile(const QString &fileName, const QString &content) diff --git a/src/blackcore/context/contextapplicationimpl.h b/src/blackcore/context/contextapplicationimpl.h index c45b819f9..ed9aac0ce 100644 --- a/src/blackcore/context/contextapplicationimpl.h +++ b/src/blackcore/context/contextapplicationimpl.h @@ -60,7 +60,7 @@ namespace BlackCore virtual BlackMisc::CStatusMessage saveSettingsByKey(const QStringList &keys) override; virtual BlackMisc::CStatusMessage loadSettings() override; virtual void registerHotkeyActions(const QStringList &actions, const BlackMisc::CIdentifier &origin) override; - virtual void callHotkeyAction(const QString &action, bool argument, const BlackMisc::CIdentifier &origin) override; + virtual void callHotkeyActionRemotely(const QString &action, bool argument, const BlackMisc::CIdentifier &origin) override; virtual bool writeToFile(const QString &fileName, const QString &content) override; virtual BlackMisc::CIdentifier registerApplication(const BlackMisc::CIdentifier &application) override; virtual void unregisterApplication(const BlackMisc::CIdentifier &application) override; diff --git a/src/blackcore/context/contextapplicationproxy.cpp b/src/blackcore/context/contextapplicationproxy.cpp index bad4b1bb7..8eea6a67d 100644 --- a/src/blackcore/context/contextapplicationproxy.cpp +++ b/src/blackcore/context/contextapplicationproxy.cpp @@ -7,6 +7,7 @@ * contained in the LICENSE file. */ +#include "blackcore/application.h" #include "blackcore/context/contextapplicationproxy.h" #include "blackmisc/dbus.h" #include "blackmisc/dbusserver.h" @@ -38,6 +39,8 @@ namespace BlackCore } }); + connect(this, &CContextApplicationProxy::remoteHotkeyAction, this, &CContextApplicationProxy::processRemoteHotkeyActionCall); + m_pingTimer.setObjectName(serviceName + "::m_pingTimer"); connect(&m_pingTimer, &QTimer::timeout, this, &CContextApplicationProxy::reRegisterApplications); m_pingTimer.start(PingIdentifiersMs); @@ -158,9 +161,9 @@ namespace BlackCore m_dBusInterface->callDBus(QLatin1String("registerHotkeyActions"), actions, origin); } - void CContextApplicationProxy::callHotkeyAction(const QString &action, bool argument, const CIdentifier &origin) + void CContextApplicationProxy::callHotkeyActionRemotely(const QString &action, bool argument, const CIdentifier &origin) { - m_dBusInterface->callDBus(QLatin1String("callHotkeyAction"), action, argument, origin); + m_dBusInterface->callDBus(QLatin1String("callHotkeyActionRemotely"), action, argument, origin); } CIdentifier CContextApplicationProxy::registerApplication(const CIdentifier &application) @@ -243,5 +246,12 @@ namespace BlackCore CDBusServer::disconnectFromDBus(connection, dBusAddress); return ok; } + + void CContextApplicationProxy::processRemoteHotkeyActionCall(const QString &action, bool argument, const BlackMisc::CIdentifier &origin) + { + if (origin.isFromLocalMachine()) { return; } + sApp->getInputManager()->callFunctionsBy(action, argument); + CLogMessage(this, CLogCategory::contextSlot()).debug() << "Calling function" << action << "from origin" << origin.getMachineName(); + } } // namespace } // namespace diff --git a/src/blackcore/context/contextapplicationproxy.h b/src/blackcore/context/contextapplicationproxy.h index caa1d6dcc..257c67d04 100644 --- a/src/blackcore/context/contextapplicationproxy.h +++ b/src/blackcore/context/contextapplicationproxy.h @@ -65,7 +65,7 @@ namespace BlackCore virtual BlackMisc::CStatusMessage saveSettingsByKey(const QStringList &keys) override; virtual BlackMisc::CStatusMessage loadSettings() override; virtual void registerHotkeyActions(const QStringList &actions, const BlackMisc::CIdentifier &origin) override; - virtual void callHotkeyAction(const QString &action, bool argument, const BlackMisc::CIdentifier &origin) override; + virtual void callHotkeyActionRemotely(const QString &action, bool argument, const BlackMisc::CIdentifier &origin) override; virtual BlackMisc::CIdentifier registerApplication(const BlackMisc::CIdentifier &application) override; virtual void unregisterApplication(const BlackMisc::CIdentifier &application) override; virtual BlackMisc::CIdentifierList getRegisteredApplications() const override; @@ -98,6 +98,8 @@ namespace BlackCore //! Ping/heartbeat identifiers void reRegisterApplications(); + + void processRemoteHotkeyActionCall(const QString &action, bool argument, const BlackMisc::CIdentifier &origin); }; } // ns } // ns diff --git a/src/blackcore/inputmanager.cpp b/src/blackcore/inputmanager.cpp index 871dfbb9e..cbb4a1a34 100644 --- a/src/blackcore/inputmanager.cpp +++ b/src/blackcore/inputmanager.cpp @@ -95,10 +95,10 @@ namespace BlackCore m_capturedCombination = {}; } - void CInputManager::callFunctionsBy(const QString &action, bool isKeyDown) + void CInputManager::callFunctionsBy(const QString &action, bool isKeyDown, bool shouldEmit) { if (action.isEmpty()) { return; } - if (m_actionRelayingEnabled) emit remoteActionFromLocal(action, isKeyDown); + if (m_actionRelayingEnabled && shouldEmit) { emit remoteActionFromLocal(action, isKeyDown); } for (const auto &boundAction : as_const(m_boundActions)) { diff --git a/src/blackcore/inputmanager.h b/src/blackcore/inputmanager.h index f2c550b95..118a890ea 100644 --- a/src/blackcore/inputmanager.h +++ b/src/blackcore/inputmanager.h @@ -85,7 +85,7 @@ namespace BlackCore void setForwarding(bool enabled) { m_actionRelayingEnabled = enabled; } //! Call functions by hotkeyfunction - void callFunctionsBy(const QString &action, bool isKeyDown); + void callFunctionsBy(const QString &action, bool isKeyDown, bool shouldEmit = true); //! Triggers a key event manually and calls the registered functions. void triggerKey(const BlackMisc::Input::CHotkeyCombination &combination, bool isPressed);