[Spice-devel] [spice-gtk] gtk-session: do not sync modifiers when focused

[Victor Toso]

Hi,

On Tue, Apr 09, 2019 at 08:12:25AM +0000, Victor Toso wrote:
> From: Olivier Fourdan <ofourdan at redhat.com>
> 
> Spice gtk-session would try to synchronize the modifiers state whenever
> the keymap changes, but doing so is inherently racy.
> 
> While the there is a keyboard grab in effect, all key events are
> forwarded to the guest, hence all modifiers key press get processed by
> the kernel on the guest.
> 
> Trying to synchronize the modifiers will generate additional key press/
> release events which will result in the opposite effect and effectively
> desynchronize the modifier states in the guest.
> 
> Synchronizing modifiers from the host should therefore be limited to
> focus change, as actual press/release events might have occurred without
> the guest knowing. Otherwise, no need to synchronize the modifiers.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires at redhat.com>
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
> 
> Merge request spice-gtk!6 or
>     https://gitlab.freedesktop.org/spice/spice-gtk/merge_requests/6
> 
>  src/spice-gtk-session.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index b48f92a..65442b2 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -163,6 +163,13 @@ static void keymap_modifiers_changed(GdkKeymap *keymap, gpointer data)
>  {
>      SpiceGtkSession *self = data;
>  
> +    /* set_key_locks() is inherently racy, but no need to sync modifiers
> +     * if we have focus as the regular keypress/keyrelease will have set
> +     * the expected modifiers state in the guest.
> +     */
> +    if (self->priv->keyboard_has_focus)
> +      return;
> +

Doing a quick test, one can see that on keyboard_focus, before
this patch we had (NumLock on Wayland)

| spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x2, guest_modifiers:0x2
| spice-widget.c:1646 0:0 key_event press: keycode: 77  state: 16  group 0 modifier 1
| spice-util.c:270 spice_make_scancode:  scancode 69
| spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x0, guest_modifiers:0x2
| spice-widget.c:1646 0:0 key_event release: keycode: 77  state: 16  group 0 modifier 1
| spice-util.c:270 spice_make_scancode: release scancode 69

and

| spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x2, guest_modifiers:0x0
| spice-widget.c:1646 0:0 key_event press: keycode: 77  state: 0  group 0 modifier 1
| spice-util.c:270 spice_make_scancode:  scancode 69
| spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x2, guest_modifiers:0x2
| spice-widget.c:1646 0:0 key_event release: keycode: 77  state: 16  group 0 modifier 1
| spice-util.c:270 spice_make_scancode: release scancode 69
| spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x2, guest_modifiers:0x0

With this patch, becomes:

| spice-widget.c:1644 0:0 key_event press: keycode: 77  state: 16  group 0 modifier 1
| spice-util.c:270 spice_make_scancode:  scancode 69
| spice-widget.c:1644 0:0 key_event release: keycode: 77  state: 16  group 0 modifier 1
| spice-util.c:270 spice_make_scancode: release scancode 69

and

| spice-widget.c:1644 0:0 key_event press: keycode: 77  state: 0  group 0 modifier 1
| spice-util.c:270 spice_make_scancode:  scancode 69
| spice-widget.c:1644 0:0 key_event release: keycode: 77  state: 16  group 0 modifier 1
| spice-util.c:270 spice_make_scancode: release scancode 69

So, keypress alone is enough indeed when we have focus. Without focus,
this is still needed to keep the modifiers synced.

Many thanks for the investigation, explanation and fixing it!

Acked-by: Victor Toso <victortoso at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190409/0465eef6/attachment.sig>