change(linux): Implement ordered output sentinel in keyman-system-ser…

…vice

This change allows to press F24 as ordered output sentinel from
keyman-system-service.

This is part of implementing serialized output with keyman-system-service
instead of requiring a patched ibus.

The problem both approaches try to solve is that with non-compliant apps
it is not possible to directly delete characters from the context. Instead
we have to emit a backspace key before we can commit the new
characters. However, the backspace key press goes through a different
code path in ibus and so it can happen that the commit gets processed
before the backspace which then deletes from the characters we just
added instead of from the old content.

The previous implementation solved this by forwarding a F24 ordered output
sentinel key to ibus and relying on the patched ibus to send that back to
us. When we received the F24 key we committed the characters that we
queued when we forwarded the F24 key (implemented in #7079).

The new approach implemented in this change instead sends the F24 ordered
output sentinel key through keyman-system-service and so follows the
regular key processing without requiring a patched ibus to send the key
back to us. The rest of the algorithm stays the same: when we receive the
F24 key we commit the characters previously queued.

Fixes: #10799

linux/keyman-system-service/src/KeyboardDevice.cpp

@@ -10,19 +10,26 @@ using namespace std;
 
 KeyboardDevice::KeyboardDevice()
 {
-  dev = nullptr;
-  fd  = -1;
+  dev            = nullptr;
+  fd             = -1;
   hasCapsLockLed = -1;
   debug          = false;
 }
 
 KeyboardDevice::~KeyboardDevice()
+{
+  Close();
+}
+
+void KeyboardDevice::Close()
 {
   if (dev) {
     libevdev_free(dev);
+    dev = nullptr;
   }
   if (fd != -1) {
     close(fd);
+    fd = -1;
   }
 }
 
@@ -34,14 +41,14 @@ bool KeyboardDevice::Initialize(const char* name)
   fd = open(path.c_str(), O_RDWR);
   if (fd < 0) {
     std::cerr << "Failed to open device " << path << ": " << strerror(errno) << std::endl;
+    Close();
     return false;
   }
 
   int rc = libevdev_new_from_fd(fd, &dev);
   if (rc < 0) {
     std::cerr << "Failed to init libevdev for " << path << ": " << strerror(-rc) << std::endl;
-    close(fd);
-    fd = -1;
+    Close();
     return false;
   }
 

linux/keyman-system-service/src/KeyboardDevice.h

@@ -3,6 +3,7 @@
 
 #include <libevdev/libevdev.h>
 
+// Any keyboard device available on the system
 class KeyboardDevice
 {
   public:
@@ -20,6 +21,8 @@ class KeyboardDevice
     int fd;
     int hasCapsLockLed;
     bool debug;
+
+    void Close();
 };
 
 #endif // __KEYBOARDDEVICE_H__

linux/keyman-system-service/src/KeymanSystemService.cpp

@@ -1,6 +1,10 @@
-// This file implements a service which will run on the system dbus,
-// based on the sd-bus library, see
-// https://0pointer.net/blog/the-new-sd-bus-api-of-systemd.html
+/*
+ * Keyman is copyright (C) SIL International. MIT License.
+ *
+ * This file implements the dbus message handlers as well as a service
+ * which will run on the system dbus, based on the sd-bus library, see
+ * https://0pointer.net/blog/the-new-sd-bus-api-of-systemd.html
+ */
 
 #include <cstdint>
 #include <dirent.h>
@@ -21,6 +25,16 @@
 
 using namespace std;
 
+/**
+ * DBus message handler which sets the caps lock indicator
+ *
+ * @param msg        The dbus message containing a boolean with the new state for
+ *                   the caps lock indicator.
+ * @param user_data  The KeymanSystemService object
+ * @param ret_error  Out. Unused. Will be set to SD_BUS_ERROR_NULL.
+ *
+ * @return On success a non-negative integer. On failure a negative errno-style error code.
+ */
 static int32_t
 on_set_caps_lock_indicator(
   sd_bus_message *msg,
@@ -43,12 +57,18 @@ on_set_caps_lock_indicator(
   return sd_bus_reply_method_return(msg, "");
 }
 
+/**
+ * DBus message handler which gets the state of the caps lock indicator
+ *
+ * @param msg        The dbus message. On return this will be set to a boolean
+ *                   containing the state of the caps lock indicator.
+ * @param user_data  The KeymanSystemService object
+ * @param ret_error  Out. Unused. Will be set to SD_BUS_ERROR_NULL.
+ *
+ * @return On success a non-negative integer. On failure a negative errno-style error code.
+ */
 static int32_t
-on_get_caps_lock_indicator(
-  sd_bus_message *msg,
-  void *user_data,
-  sd_bus_error *ret_error
-) {
+on_get_caps_lock_indicator(sd_bus_message *msg, void *user_data, sd_bus_error *ret_error) {
   uint32_t state;
 
   *ret_error = SD_BUS_ERROR_NULL;
@@ -58,18 +78,71 @@ on_get_caps_lock_indicator(
   return sd_bus_reply_method_return(msg, "b", state);
 }
 
+/**
+ * DBus message handler to emit the sentinel (F24) key event to serialize
+ * the order of the output.
+ *
+ * @param msg        The dbus message containing a boolean whether a
+ *                   KeyDown (true) or a KeyUp (false) event should be emitted.
+ * @param user_data  The KeymanSystemService object
+ * @param ret_error  Out. Unused. Will be set to SD_BUS_ERROR_NULL.
+ *
+ * @return On success a non-negative integer. On failure a negative errno-style error code.
+ */
+static int32_t
+on_call_ordered_output_sentinel(sd_bus_message *msg, void *user_data, sd_bus_error *ret_error) {
+  int32_t ret;
+  uint32_t isKeyDown;
+
+  *ret_error = SD_BUS_ERROR_NULL;
+
+  ret = sd_bus_message_read_basic(msg, 'b', &isKeyDown);
+  if (ret < 0) {
+    syslog(LOG_USER | LOG_NOTICE, "Failed to parse parameter: %s", strerror(-ret));
+    return ret;
+  }
+
+  KeymanSystemService *service = static_cast<KeymanSystemService *>(user_data);
+  service->CallOrderedOutputSentinel(isKeyDown);
+  return sd_bus_reply_method_return(msg, "");
+}
+
+/**
+ * DBus message handler to emit the backspace key event.
+ *
+ * @param msg        The dbus message, not used
+ * @param user_data  The KeymanSystemService object
+ * @param ret_error  Out. Unused. Will be set to SD_BUS_ERROR_NULL.
+ *
+ * @return On success a non-negative integer. On failure a negative errno-style error code.
+ */
+static int32_t
+on_press_backspace(sd_bus_message *msg, void *user_data, sd_bus_error *ret_error) {
+  *ret_error = SD_BUS_ERROR_NULL;
+
+  KeymanSystemService *service = static_cast<KeymanSystemService *>(user_data);
+  service->PressBackspace();
+  return sd_bus_reply_method_return(msg, "");
+}
+
 static const sd_bus_vtable system_service_vtable[] = {
   SD_BUS_VTABLE_START(0),
     SD_BUS_METHOD("SetCapsLockIndicator", "b", "", on_set_caps_lock_indicator, SD_BUS_VTABLE_UNPRIVILEGED),
     SD_BUS_METHOD("GetCapsLockIndicator", "", "b", on_get_caps_lock_indicator, SD_BUS_VTABLE_UNPRIVILEGED),
-  SD_BUS_VTABLE_END
-};
+    SD_BUS_METHOD("CallOrderedOutputSentinel", "b", "", on_call_ordered_output_sentinel, SD_BUS_VTABLE_UNPRIVILEGED),
+    SD_BUS_METHOD("PressBackspace", "", "", on_press_backspace, SD_BUS_VTABLE_UNPRIVILEGED),
+  SD_BUS_VTABLE_END};
+
+/*----------------------------------------------------------------------
+KeymanSystemService class
+----------------------------------------------------------------------*/
 
 KeymanSystemService::KeymanSystemService()
 {
   int ret;
 
   GetKbdDevices();
+  CreateOrderOutputDevice();
 
 #ifdef KEYMAN_TESTING
   ret = sd_bus_open_user(&bus);
@@ -115,9 +188,18 @@ KeymanSystemService::~KeymanSystemService()
       delete device;
     }
     delete kbd_devices;
+    kbd_ordered_output = nullptr;
+  }
+
+  if (kbd_ordered_output) {
+    delete kbd_ordered_output;
+    kbd_ordered_output = nullptr;
   }
 }
 
+/**
+ * Message loop for dbus events
+ */
 int KeymanSystemService::Loop()
 {
   for (;;) {
@@ -144,6 +226,9 @@ int KeymanSystemService::Loop()
   return 0;
 }
 
+/**
+ * Gets a list of all keyboard devices that support the caps lock indicator
+ */
 void KeymanSystemService::GetKbdDevices() {
   kbd_devices = new std::list<KeyboardDevice *>();
 
@@ -166,7 +251,24 @@ void KeymanSystemService::GetKbdDevices() {
   }
 }
 
-// Set the CapsLock indicator on all keyboard devices.
+/**
+ * Create the fake keyboard device used to serialize the output
+ */
+void KeymanSystemService::CreateOrderOutputDevice() {
+  if (!kbd_ordered_output) {
+    kbd_ordered_output = new OrderOutputDevice();
+    if (!kbd_ordered_output->Initialize()) {
+      delete kbd_ordered_output;
+      kbd_ordered_output = nullptr;
+    }
+  }
+}
+
+/**
+ * Set the CapsLock indicator on all keyboard devices.
+ *
+ * @param state  1 to turn on caps lock indicator, 0 to turn off caps lock indicator
+ */
 void
 KeymanSystemService::SetCapsLockIndicatorOnDevices(uint32_t state) {
   for (KeyboardDevice *kbdDevice : *kbd_devices) {
@@ -179,8 +281,11 @@ KeymanSystemService::SetCapsLockIndicatorOnDevices(uint32_t state) {
   }
 }
 
-// Get the CapsLock indicator state from the list of keyboard devices.
-// This will return true if any keyboard has the CapsLock indicator lit.
+/**
+ * Get the CapsLock indicator state from the list of keyboard devices.
+ *
+ * @return true if any keyboard has the CapsLock indicator lit
+ */
 uint32_t
 KeymanSystemService::GetCapsLockIndicatorOnDevices() {
   bool state = false;
@@ -193,3 +298,29 @@ KeymanSystemService::GetCapsLockIndicatorOnDevices() {
   }
   return state;
 }
+
+/**
+ * Emit a ordered output sentinel key event
+ *
+ * @param isKeyDown  true to emit a KeyDown event, false for KeyUp
+ */
+void
+KeymanSystemService::CallOrderedOutputSentinel(uint32_t isKeyDown) {
+  if (!kbd_ordered_output) {
+    syslog(LOG_USER | LOG_ERR, "%s: No keyboard initialized", __FUNCTION__);
+    return;
+  }
+  kbd_ordered_output->PressSentinelKey(isKeyDown);
+}
+
+/**
+ * Emit the backspace key event
+ */
+void
+KeymanSystemService::PressBackspace() {
+  if (!kbd_ordered_output) {
+    syslog(LOG_USER | LOG_ERR, "%s: No keyboard initialized", __FUNCTION__);
+    return;
+  }
+  kbd_ordered_output->PressBackspace();
+}

linux/keyman-system-service/src/KeymanSystemService.h

@@ -1,20 +1,29 @@
+/*
+ * Keyman is copyright (C) SIL International. MIT License.
+ *
+ * Defines the Keyman system service
+ */
+
 #ifndef __KEYMANSYSTEMSERVICE_H__
 #define __KEYMANSYSTEMSERVICE_H__
 
 #include <list>
 #include <systemd/sd-bus.h>
+#include "OrderOutputDevice.h"
 #include "KeyboardDevice.h"
 
 using namespace std;
 
 class KeymanSystemService {
 private:
-  std::list<KeyboardDevice *>* kbd_devices = NULL;
-  sd_bus_slot *slot                        = NULL;
-  sd_bus *bus                              = NULL;
-  bool failed                              = false;
+  std::list<KeyboardDevice *>* kbd_devices  = nullptr;
+  OrderOutputDevice *kbd_ordered_output     = nullptr;
+  sd_bus_slot *slot                         = nullptr;
+  sd_bus *bus                               = nullptr;
+  bool failed                               = false;
 
   void GetKbdDevices();
+  void CreateOrderOutputDevice();
 
 public:
   KeymanSystemService();
@@ -28,6 +37,8 @@ class KeymanSystemService {
   int Loop();
   void SetCapsLockIndicatorOnDevices(uint32_t state);
   uint32_t GetCapsLockIndicatorOnDevices();
+  void CallOrderedOutputSentinel(uint32_t isKeyDown);
+  void PressBackspace();
 };
 
 #endif // __KEYMANSYSTEMSERVICE_H__