[Spice-devel] [PATCH vdagent v3 2/3] retrieve _NET_WM_NAME using GDK
Jakub Janků
janku.jakub.jj at gmail.com
Fri Dec 1 15:29:32 UTC 2017
Hi,
On Fri, 2017-12-01 at 15:11 +0100, Victor Toso wrote:
> 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.
The value for GNOME is alright. I've tested XFCE and I'm getting
correct value "Xfwm4". (Haven't tested Mate yet.)
Gtk should use practically the same approach to get the _NET_WM_NAME as
we did, so it's weird that it doesn't work. Could you please try the
current vdagent to see what name does it return?
>
> > 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
Thanks,
Jakub
More information about the Spice-devel
mailing list