[Spice-devel] [PATCH spice-gtk] widget: fix request_auto_usbredir() critical

[Victor Toso]

Hi,

On Wed, Jun 06, 2018 at 06:19:27PM +0200, Marc-André Lureau wrote:
> On Wed, Jun 6, 2018 at 6:03 PM, Victor Toso <victortoso at redhat.com> wrote:
> > On Wed, Jun 06, 2018 at 05:23:24PM +0200, marcandre.lureau at redhat.com wrote:
> >> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> >>
> >> On f28, when focusing out of the display, we get the following
> >> critical:
> >>
> >> (spicy:15388): GSpice-CRITICAL **: 17:20:07.710:
> >> spice_gtk_session_request_auto_usbredir: assertion
> >> 's->auto_usbredir_reqs > 0' failed
> >>
> >> This is due to unbalanced gtk+ focus-in and focus-out events (one more
> >> focus-out). This may be fixable on the gtk+ side, but it's also easy
> >> to prevent on our side when the last focus state is unchanged.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> >
> > Have you seen https://gitlab.gnome.org/GNOME/gtk/issues/792 ?

> Nope, thanks for the link.
>
> >
> > IMHO it can be prevented in spice-gtk but some warning should be
> > kept as this is unexpected.
>
> The function looks generic enough to prevent doing unnecessary
> work if the state is not changed.
>
> I am not sure it's spice-gtk job to report gtk+ bugs :)

Sadly we rely on gtk and gtk bugs are easier to track if we have
some info on it. The critical that you are fixing is bad, could
be avoided by this patch but this patch workaround a bug in gtk+
so we should know about it somehow with some debug/warning
message, imho.

> How long before the gtk+ bug is fixed (if it's ever fixed)?

The fix was pushed [0] but need to double check if it was pushed
to the stable branches, etc.

[0] https://gitlab.gnome.org/GNOME/gtk/commit/d3885e92a7db130d96865

> I'd just go with this simple patch, and let the gtk+ team
> decide how and when to fix the gtk+ behaviour.

I'm not saying no for your patch, I prefer warning/debug message
due gtk+ event bug instead of critical message later on.

        toso

> >
> >> ---
> >>  src/spice-widget.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/src/spice-widget.c b/src/spice-widget.c
> >> index 72f5334..2adcf30 100644
> >> --- a/src/spice-widget.c
> >> +++ b/src/spice-widget.c
> >> @@ -207,6 +207,9 @@ static void update_keyboard_focus(SpiceDisplay *display, gboolean state)
> >>  {
> >>      SpiceDisplayPrivate *d = display->priv;
> >>
> >> +    if (d->keyboard_have_focus == state)
> >> +        return;
> >> +
> >>      d->keyboard_have_focus = state;
> >>      spice_gtk_session_set_keyboard_has_focus(d->gtk_session, state);
> >>
> >>
> >> base-commit: 7b67ed4eb6ab530bf58afac8ff4ed190cc951dfb
> >> --
> >> 2.18.0.rc1.1.gae296d1cf5
> >>
> >> _______________________________________________
> >> Spice-devel mailing list
> >> Spice-devel at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> 
> 
> 
> -- 
> Marc-André Lureau
-------------- 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/20180606/8d365aa7/attachment.sig>