[Spice-devel] [PATCH vdagent v3 2/3] retrieve _NET_WM_NAME using GDK

[Victor Toso]

Hi,

On Mon, Nov 13, 2017 at 06:50:06PM +0100, Jakub Janků wrote:
> Get _NET_WM_NAME using gdk_x11_screen_get_window_manager_name().
> vdagent_x11_get_wm_name(): return the name instead of setting it.
> Return string specifics:
> - "unsupported", when not running on X11
> - "unknown", when the name cannot be retrieved

I'm actually getting:
* "GNOME Shell", either on Wayland/X11 GNOME
* "unknown", for XFCE
* "Metacity (Marco)", for MATE

Based on values in the array wms_with_icons_on_desktop, we might have
some regressions or we need to update the array too.

> Since the return value is never NULL, remove obsolete checks.

Yep, seems better this way.

> ---
>  src/vdagent/x11.c | 88 +++++++++++++------------------------------------------
>  1 file changed, 21 insertions(+), 67 deletions(-)
> 
> diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> index 6e47ea1..4ea86e9 100644
> --- a/src/vdagent/x11.c
> +++ b/src/vdagent/x11.c
> @@ -32,6 +32,10 @@
>     pending writes are flushed. */
>  
>  #include <glib.h>
> +#include <gdk/gdk.h>
> +#ifdef GDK_WINDOWING_X11
> +#include <gdk/gdkx.h>
> +#endif
>  #include <stdlib.h>
>  #include <limits.h>
>  #include <string.h>
> @@ -111,66 +115,16 @@ int vdagent_x11_restore_error_handler(struct vdagent_x11 *x11)
>      return error;
>  }
>  
> -static void vdagent_x11_get_wm_name(struct vdagent_x11 *x11)
> +static const gchar *vdagent_x11_get_wm_name()
>  {
> -    Atom type_ret;
> -    int format_ret;
> -    unsigned long len, remain;
> -    unsigned char *data = NULL;
> -    Window sup_window = None;
> -
> -    /* XGetWindowProperty can throw a BadWindow error. One way we can trigger
> -       this is when the display-manager (ie gdm) has set, and not cleared the
> -       _NET_SUPPORTING_WM_CHECK property, and the window manager running in
> -       the user session has not yet updated it to point to its window, so its
> -       pointing to a nonexistent window. */
> -    vdagent_x11_set_error_handler(x11, vdagent_x11_ignore_bad_window_handler);
> -
> -    /* Get the window manager SUPPORTING_WM_CHECK window */
> -    if (XGetWindowProperty(x11->display, x11->root_window[0],
> -            XInternAtom(x11->display, "_NET_SUPPORTING_WM_CHECK", False), 0,
> -            LONG_MAX, False, XA_WINDOW, &type_ret, &format_ret, &len,
> -            &remain, &data) == Success) {
> -        if (type_ret == XA_WINDOW)
> -            sup_window = *((Window *)data);
> -        XFree(data);
> -    }
> -    if (sup_window == None &&
> -        XGetWindowProperty(x11->display, x11->root_window[0],
> -            XInternAtom(x11->display, "_WIN_SUPPORTING_WM_CHECK", False), 0,
> -            LONG_MAX, False, XA_CARDINAL, &type_ret, &format_ret, &len,
> -            &remain, &data) == Success) {
> -        if (type_ret == XA_CARDINAL)
> -            sup_window = *((Window *)data);
> -        XFree(data);
> -    }
> -    /* So that we can get the net_wm_name */
> -    if (sup_window != None) {
> -        Atom utf8 = XInternAtom(x11->display, "UTF8_STRING", False);
> -        if (XGetWindowProperty(x11->display, sup_window,
> -                XInternAtom(x11->display, "_NET_WM_NAME", False), 0,
> -                LONG_MAX, False, utf8, &type_ret, &format_ret, &len,
> -                &remain, &data) == Success) {
> -            if (type_ret == utf8) {
> -                x11->net_wm_name =
> -                    g_strndup((char *)data, (format_ret / 8) * len);
> -            }
> -            XFree(data);
> -        }
> -        if (x11->net_wm_name == NULL &&
> -            XGetWindowProperty(x11->display, sup_window,
> -                XInternAtom(x11->display, "_NET_WM_NAME", False), 0,
> -                LONG_MAX, False, XA_STRING, &type_ret, &format_ret, &len,
> -                &remain, &data) == Success) {
> -            if (type_ret == XA_STRING) {
> -                x11->net_wm_name =
> -                    g_strndup((char *)data, (format_ret / 8) * len);
> -            }
> -            XFree(data);
> -        }
> -    }
> -
> -    vdagent_x11_restore_error_handler(x11);
> +#ifdef GDK_WINDOWING_X11
> +    GdkDisplay *display = gdk_display_get_default();
> +    if (GDK_IS_X11_DISPLAY(display))
> +        return gdk_x11_screen_get_window_manager_name(
> +            gdk_display_get_default_screen(display));
> +    else

No need for else

> +#endif
> +    return "unsupported";
>  }
>  
>  struct vdagent_x11 *vdagent_x11_create(struct udscs_connection *vdagentd,
> @@ -278,12 +232,13 @@ struct vdagent_x11 *vdagent_x11_create(struct udscs_connection *vdagentd,
>      /* Get net_wm_name, since we are started at the same time as the wm,
>         sometimes we need to wait a bit for it to show up. */
>      i = 10;
> -    vdagent_x11_get_wm_name(x11);
> -    while (x11->net_wm_name == NULL && --i > 0) {
> +    x11->net_wm_name = (gchar *)vdagent_x11_get_wm_name();
> +    while (!strcmp(x11->net_wm_name, "unknown") && --i > 0) {
>          usleep(100000);
> -        vdagent_x11_get_wm_name(x11);
> +        x11->net_wm_name = (gchar *)vdagent_x11_get_wm_name();
>      }
> -    if (x11->debug && x11->net_wm_name)
> +    x11->net_wm_name = g_strdup(x11->net_wm_name);
> +    if (x11->debug)
>          syslog(LOG_DEBUG, "net_wm_name: \"%s\", has icons: %d",
>                 x11->net_wm_name, vdagent_x11_has_icons_on_desktop(x11));
>  
> @@ -1347,10 +1302,9 @@ int vdagent_x11_has_icons_on_desktop(struct vdagent_x11 *x11)
>      };
>      int i;
>  
> -    if (x11->net_wm_name)
> -        for (i = 0; wms_with_icons_on_desktop[i]; i++)
> -            if (!strcmp(x11->net_wm_name, wms_with_icons_on_desktop[i]))
> -                return 1;
> +    for (i = 0; wms_with_icons_on_desktop[i]; i++)
> +        if (!strcmp(x11->net_wm_name, wms_with_icons_on_desktop[i]))
> +            return 1;

Sorry for taking time to review this patch ;)

Cheers,
        toso
-------------- 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/20171201/a4fc25ae/attachment.sig>