Ensure keyboard modifiers are synchronized properly

In certain circumstances, the keyboard modifiers get out-of-sync between the
guest and the client. This is easy to reproduce with the following steps:
 - launch virt-viewer with a guest that is not running
 - start the guest
 - while guest is booting, enable CAPS LOCK on the client
 - after guest finishes booting, it will set its modifiers to a default value
   (e.g. numlock on, capslock off)
 - now capslock is OFF in the guest, but ON in the client
 - toggle caps lock
 - now capslock is ON in the guest, but OFF in the client

This moves the responsibility for synchronizing client and guest modifiers into
SpiceGtkSession. It can't be handled easily within the SpiceDisplay widget since
there can be multiple display widgets for each inputs channel.

A new function (spice_gtk_session_sync_keyboard_modifiers()) was added so that
synchronization can be triggered manually if desired. But it also registers a
signal handler for the InputsChannel::inputs-modifiers signal to detect when the
guest has changed its modifiers. The signal handler simply overrides the guests
modifiers and sets them back to the value from the client.

3 files changed, 100 insertions, 89 deletions

diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
index a9ce025..f0f7edf 100644
--- a/gtk/spice-gtk-session.c
+++ b/gtk/spice-gtk-session.c
@@ -17,6 +17,22 @@
 */
 #include "config.h"
 
+#if HAVE_X11_XKBLIB_H
+#include <X11/XKBlib.h>
+#include <gdk/gdkx.h>
+#endif
+#ifdef GDK_WINDOWING_X11
+#include <X11/Xlib.h>
+#include <gdk/gdkx.h>
+#endif
+#ifdef WIN32
+#include <windows.h>
+#include <gdk/gdkwin32.h>
+#ifndef MAPVK_VK_TO_VSC /* may be undefined in older mingw-headers */
+#define MAPVK_VK_TO_VSC 0
+#endif
+#endif
+
 #include <gtk/gtk.h>
 #include <spice/vd_agent.h>
 #include "desktop-integration.h"
@@ -97,6 +113,70 @@ enum {
     PROP_AUTO_USBREDIR,
 };
 
+static guint32 get_keyboard_lock_modifiers(void)
+{
+    guint32 modifiers = 0;
+#if HAVE_X11_XKBLIB_H
+    Display *x_display = NULL;
+    XKeyboardState keyboard_state;
+
+    GdkScreen *screen = gdk_screen_get_default();
+    if (!GDK_IS_X11_DISPLAY(gdk_screen_get_display(screen))) {
+        SPICE_DEBUG("FIXME: gtk backend is not X11");
+        return 0;
+    }
+
+    x_display = GDK_SCREEN_XDISPLAY(screen);
+    XGetKeyboardControl(x_display, &keyboard_state);
+
+    if (keyboard_state.led_mask & 0x01) {
+        modifiers |= SPICE_INPUTS_CAPS_LOCK;
+    }
+    if (keyboard_state.led_mask & 0x02) {
+        modifiers |= SPICE_INPUTS_NUM_LOCK;
+    }
+    if (keyboard_state.led_mask & 0x04) {
+        modifiers |= SPICE_INPUTS_SCROLL_LOCK;
+    }
+#elif defined(win32)
+    if (GetKeyState(VK_CAPITAL) & 1) {
+        modifiers |= SPICE_INPUTS_CAPS_LOCK;
+    }
+    if (GetKeyState(VK_NUMLOCK) & 1) {
+        modifiers |= SPICE_INPUTS_NUM_LOCK;
+    }
+    if (GetKeyState(VK_SCROLL) & 1) {
+        modifiers |= SPICE_INPUTS_SCROLL_LOCK;
+    }
+#else
+    g_warning("get_keyboard_lock_modifiers not implemented");
+#endif // HAVE_X11_XKBLIB_H
+    return modifiers;
+}
+
+static void spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSession *self,
+                                                                  SpiceInputsChannel* inputs)
+{
+    gint guest_modifiers = 0, client_modifiers = 0;
+
+    g_return_if_fail(SPICE_IS_INPUTS_CHANNEL(inputs));
+
+    g_object_get(inputs, "key-modifiers", &guest_modifiers, NULL);
+
+    client_modifiers = get_keyboard_lock_modifiers();
+    g_debug("%s: input:%p client_modifiers:0x%x, guest_modifiers:0x%x",
+            G_STRFUNC, inputs, client_modifiers, guest_modifiers);
+
+    if (client_modifiers != guest_modifiers)
+        spice_inputs_set_key_locks(inputs, client_modifiers);
+}
+
+static void guest_modifiers_changed(SpiceInputsChannel *inputs, gpointer data)
+{
+    SpiceGtkSession *self = data;
+    spice_gtk_session_sync_keyboard_modifiers_for_channel(self, inputs);
+}
+
 static void spice_gtk_session_init(SpiceGtkSession *self)
 {
     SpiceGtkSessionPrivate *s;
@@ -872,6 +952,11 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
         g_signal_connect(channel, "main-clipboard-selection-release",
                          G_CALLBACK(clipboard_release), self);
     }
+    if (SPICE_IS_INPUTS_CHANNEL(channel)) {
+        spice_g_signal_connect_object(channel, "inputs-modifiers",
+                                      G_CALLBACK(guest_modifiers_changed), self, 0);
+        spice_gtk_session_sync_keyboard_modifiers_for_channel(self, SPICE_INPUTS_CHANNEL(channel));
+    }
 }
 
 static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
@@ -1029,3 +1114,15 @@ void spice_gtk_session_paste_from_guest(SpiceGtkSession *self)
     s->clipboard_by_guest[selection] = TRUE;
     s->clip_hasdata[selection] = FALSE;
 }
+
+void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self)
+{
+    GList *l = NULL, *channels = spice_session_get_channels(self->priv->session);
+
+    for (l = channels; l != NULL; l = l->next) {
+        if (SPICE_IS_INPUTS_CHANNEL(l->data)) {
+            SpiceInputsChannel *inputs = SPICE_INPUTS_CHANNEL(l->data);
+            spice_gtk_session_sync_keyboard_modifiers_for_channel(self, inputs);
+        }
+    }
+}
diff --git a/gtk/spice-gtk-session.h b/gtk/spice-gtk-session.h
index 3b4eac6..fbcc353 100644
--- a/gtk/spice-gtk-session.h
+++ b/gtk/spice-gtk-session.h
@@ -59,6 +59,7 @@ GType spice_gtk_session_get_type(void);
 SpiceGtkSession *spice_gtk_session_get(SpiceSession *session);
 void spice_gtk_session_copy_to_guest(SpiceGtkSession *self);
 void spice_gtk_session_paste_from_guest(SpiceGtkSession *self);
+void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self);
 
 G_END_DECLS
 
diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
index 0e4a0de..2044513 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -131,7 +131,6 @@ static void try_mouse_ungrab(SpiceDisplay *display);
 static void recalc_geometry(GtkWidget *widget);
 static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data);
 static void channel_destroy(SpiceSession *s, SpiceChannel *channel, gpointer data);
-static void sync_keyboard_lock_modifiers(SpiceDisplay *display);
 static void cursor_invalidate(SpiceDisplay *display);
 static void update_area(SpiceDisplay *display, gint x, gint y, gint width, gint height);
 static void release_keys(SpiceDisplay *display);
@@ -1457,7 +1456,8 @@ static gboolean focus_in_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_UN
         return true;
 
     release_keys(display);
-    sync_keyboard_lock_modifiers(display);
+    if (!d->disable_inputs)
+        spice_gtk_session_sync_keyboard_modifiers(d->gtk_session);
     update_keyboard_focus(display, true);
     try_keyboard_grab(display);
     update_display(display);
@@ -2411,7 +2411,6 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data)
     if (SPICE_IS_INPUTS_CHANNEL(channel)) {
         d->inputs = SPICE_INPUTS_CHANNEL(channel);
         spice_channel_connect(channel);
-        sync_keyboard_lock_modifiers(display);
         return;
     }
 
@@ -2600,89 +2599,3 @@ GdkPixbuf *spice_display_get_pixbuf(SpiceDisplay *display)
     return pixbuf;
 }
 
-#if HAVE_X11_XKBLIB_H
-static guint32 get_keyboard_lock_modifiers(Display *x_display)
-{
-    XKeyboardState keyboard_state;
-    guint32 modifiers = 0;
-
-    XGetKeyboardControl(x_display, &keyboard_state);
-
-    if (keyboard_state.led_mask & 0x01) {
-        modifiers |= SPICE_INPUTS_CAPS_LOCK;
-    }
-    if (keyboard_state.led_mask & 0x02) {
-        modifiers |= SPICE_INPUTS_NUM_LOCK;
-    }
-    if (keyboard_state.led_mask & 0x04) {
-        modifiers |= SPICE_INPUTS_SCROLL_LOCK;
-    }
-    return modifiers;
-}
-
-static void sync_keyboard_lock_modifiers(SpiceDisplay *display)
-{
-    Display *x_display;
-    SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
-    guint32 modifiers;
-    GdkWindow *w;
-
-    if (d->disable_inputs)
-        return;
-
-    w = gtk_widget_get_parent_window(GTK_WIDGET(display));
-    if (w == NULL) /* it can happen if the display is not yet shown */
-        return;
-
-    if (!GDK_IS_X11_DISPLAY(gdk_window_get_display(w))) {
-        SPICE_DEBUG("FIXME: gtk backend is not X11");
-        return;
-    }
-
-    x_display = GDK_WINDOW_XDISPLAY(w);
-    modifiers = get_keyboard_lock_modifiers(x_display);
-    if (d->inputs)
-        spice_inputs_set_key_locks(d->inputs, modifiers);
-}
-
-#elif defined (WIN32)
-static guint32 get_keyboard_lock_modifiers(void)
-{
-    guint32 modifiers = 0;
-
-    if (GetKeyState(VK_CAPITAL) & 1) {
-        modifiers |= SPICE_INPUTS_CAPS_LOCK;
-    }
-    if (GetKeyState(VK_NUMLOCK) & 1) {
-        modifiers |= SPICE_INPUTS_NUM_LOCK;
-    }
-    if (GetKeyState(VK_SCROLL) & 1) {
-        modifiers |= SPICE_INPUTS_SCROLL_LOCK;
-    }
-
-    return modifiers;
-}
-
-static void sync_keyboard_lock_modifiers(SpiceDisplay *display)
-{
-    SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
-    guint32 modifiers;
-    GdkWindow *w;
-
-    if (d->disable_inputs)
-        return;
-
-    w = gtk_widget_get_parent_window(GTK_WIDGET(display));
-    if (w == NULL) /* it can happen if the display is not yet shown */
-        return;
-
-    modifiers = get_keyboard_lock_modifiers();
-    if (d->inputs)
-        spice_inputs_set_key_locks(d->inputs, modifiers);
-}
-#else
-static void sync_keyboard_lock_modifiers(SpiceDisplay *display)
-{
-    g_warning("sync_keyboard_lock_modifiers not implemented");
-}
-#endif // HAVE_X11_XKBLIB_H