[Spice-devel] [PATCH vdagent 2/2] vdagent-x11: replace WM_NAME related code with GDK
Jakub Janků
janku.jakub.jj at gmail.com
Mon Oct 30 10:52:34 UTC 2017
On Oct 30, 2017 10:19 AM, "Frediano Ziglio" <fziglio at redhat.com> wrote:
>
> remove from x11:
> - vdagent_x11_get_wm_name()
> - vdagent_x11_has_icons_on_desktop()
> - char *new_wm_name
>
> vdagent_x11_has_icons_on_desktop() handling has been
> moved from x11.c to vdagent.c and now uses GDK.
> ---
> src/vdagent/vdagent.c | 46 +++++++++++++++++++++--
> src/vdagent/x11-priv.h | 1 -
> src/vdagent/x11.c | 99
> --------------------------------------------------
> src/vdagent/x11.h | 2 -
> 4 files changed, 42 insertions(+), 106 deletions(-)
>
> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> index 4710a44..a19a77e 100644
> --- a/src/vdagent/vdagent.c
> +++ b/src/vdagent/vdagent.c
> @@ -37,6 +37,10 @@
> #include <poll.h>
> #include <glib-unix.h>
> #include <gtk/gtk.h>
> +#include <gdk/gdk.h>
> +#ifdef GDK_WINDOWING_X11
> +#include <gdk/gdkx.h>
> +#endif
>
> #include "udscs.h"
> #include "vdagentd-proto.h"
> @@ -51,6 +55,8 @@ typedef struct VDAgent {
> struct udscs_connection *conn;
> GIOChannel *x11_channel;
>
> + gboolean has_icons_on_desktop;
> +
> GMainLoop *loop;
> } VDAgent;
>
> @@ -92,6 +98,36 @@ static GOptionEntry entries[] = {
> { NULL }
> };
>
> +static gboolean vdagent_has_icons_on_desktop()
> +{
> +#ifdef GDK_WINDOWING_X11
> + const char * const wms_with_icons[] = {
> + "Metacity", /* GNOME-2 or GNOME-3 fallback */
> + "Xfwm4", /* Xfce */
> + "Marco", /* Mate */
> + };
> +
> + GdkDisplay *display = gdk_display_get_default();
> + if (!GDK_IS_X11_DISPLAY(display))
> + return FALSE;
> +
> + for (int i = 0; i < 10; i++) {
> + const char *wm_name = gdk_x11_screen_get_window_manager_name(
> + gdk_display_get_default_screen(display));
> +
> + if (strcmp(wm_name, "unknown")) {
> + for (i = 0; i < G_N_ELEMENTS(wms_with_icons); i++)
> + if (!strcmp(wm_name, wms_with_icons[i]))
> + return TRUE;
> + return FALSE;
> + }
> +
> + usleep(100000);
> + }
> +#endif
> + return FALSE;
> +}
> +
> /**
> * xfer_get_download_directory
> *
> @@ -109,7 +145,7 @@ static const gchar *xfer_get_download_directory(V
DAgent
> *agent)
> return fx_dir;
> }
>
> - return
> g_get_user_special_dir(vdagent_x11_has_icons_on_desktop(agent->x11) ?
> + return g_get_user_special_dir(agent->has_icons_on_desktop ?
> G_USER_DIRECTORY_DESKTOP :
> G_USER_DIRECTORY_DOWNLOAD);
> }
> @@ -123,6 +159,7 @@ static const gchar *xfer_get_download_directory(V
DAgent
> *agent)
> static gboolean vdagent_init_file_xfer(VDAgent *agent)
> {
> const gchar *xfer_dir;
> + gboolean open_dir;
>
> if (agent->xfers != NULL) {
> syslog(LOG_DEBUG, "File-xfer already initialized");
> @@ -137,11 +174,10 @@ static gboolean vdagent_init_file_xfer(VDAgent
*agent)
> return FALSE;
> }
>
> - if (fx_open_dir == -1)
> - fx_open_dir = !vdagent_x11_has_icons_on_desktop(agent->x11);
> + open_dir = fx_open_dir == -1 ? !agent->has_icons_on_desktop :
> fx_open_dir;
>
> agent->xfers = vdagent_file_xfers_create(agent->conn, xfer_dir,
> - fx_open_dir, debug);
> + open_dir, debug);
> return (agent->xfers != NULL);
> }
>
> @@ -372,6 +408,8 @@ static gboolean vdagent_init_async_cb(gpointer
user_data)
> x11_io_channel_cb,
> agent);
>
> + agent->has_icons_on_desktop = vdagent_has_icons_on_desktop();
> +
> if (!vdagent_init_file_xfer(agent))
> syslog(LOG_WARNING, "File transfer is disabled");
>
> diff --git a/src/vdagent/x11-priv.h b/src/vdagent/x11-priv.h
> index 677a44d..3776098 100644
> --- a/src/vdagent/x11-priv.h
> +++ b/src/vdagent/x11-priv.h
> @@ -87,7 +87,6 @@ struct vdagent_x11 {
> Window root_window[MAX_SCREENS];
> Window selection_window;
> struct udscs_connection *vdagentd;
> - char *net_wm_name;
> int debug;
> int fd;
> int screen_count;
> diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> index 6e47ea1..7387a00 100644
> --- a/src/vdagent/x11.c
> +++ b/src/vdagent/x11.c
> @@ -37,7 +37,6 @@
> #include <string.h>
> #include <syslog.h>
> #include <assert.h>
> -#include <unistd.h>
> #include <X11/Xatom.h>
> #include <X11/Xlib.h>
> #include <X11/extensions/Xfixes.h>
> @@ -111,68 +110,6 @@ int vdagent_x11_restore_error_handler(struct
vdagent_x11
> *x11)
> return error;
> }
>
> -static void vdagent_x11_get_wm_name(struct vdagent_x11 *x11)
> -{
> - 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);
> -}
> -
> struct vdagent_x11 *vdagent_x11_create(struct udscs_connection *vdagentd,
> int debug, int sync)
> {
> @@ -275,18 +212,6 @@ struct vdagent_x11 *vdagent_x11_create(struct
> udscs_connection *vdagentd,
> }
> vdagent_x11_send_daemon_guest_xorg_res(x11, 1);
>
> - /* 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) {
> - usleep(100000);
> - vdagent_x11_get_wm_name(x11);
> - }
> - if (x11->debug && x11->net_wm_name)
> - syslog(LOG_DEBUG, "net_wm_name: \"%s\", has icons: %d",
> - x11->net_wm_name, vdagent_x11_has_icons_on_desktop(x11));
> -
> /* Flush output buffers and consume any pending events */
> vdagent_x11_do_read(x11);
>
> @@ -308,7 +233,6 @@ void vdagent_x11_destroy(struct vdagent_x11 *x11, int
> vdagentd_disconnected)
> }
>
> XCloseDisplay(x11->display);
> - g_free(x11->net_wm_name);
> free(x11->randr.failed_conf);
> free(x11);
> }
> @@ -1331,26 +1255,3 @@ void vdagent_x11_client_disconnected(struct
> vdagent_x11 *x11)
> vdagent_x11_clipboard_release(x11, sel);
> }
> }
> -
> -/* Function used to determine the default location to save file-xfers,
> - xdg desktop dir or xdg download dir. We err on the safe side and use a
> - whitelist approach, so any unknown desktop will end up with saving
> - file-xfers to the xdg download dir, and opening the xdg download dir
with
> - xdg-open when the file-xfer completes. */
> -int vdagent_x11_has_icons_on_desktop(struct vdagent_x11 *x11)
> -{
> - const char * const wms_with_icons_on_desktop[] = {
> - "Metacity", /* GNOME-2 or GNOME-3 fallback */
> - "Xfwm4", /* Xfce */
> - "Marco", /* Mate */
> - NULL
> - };
> - 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;
> -
> - return 0;
> -}
> diff --git a/src/vdagent/x11.h b/src/vdagent/x11.h
> index 4fd0380..2060655 100644
> --- a/src/vdagent/x11.h
> +++ b/src/vdagent/x11.h
> @@ -48,6 +48,4 @@ void vdagent_x11_clipboard_release(struct vdagent_x11
*x11,
> uint8_t selection);
>
> void vdagent_x11_client_disconnected(struct vdagent_x11 *x11);
>
> -int vdagent_x11_has_icons_on_desktop(struct vdagent_x11 *x11);
> -
> #endif
Looks like this patch is doing too much thing:
- move code from x11.c to vdagent.c, why?
- change cache from name to boolean, seems fine;
- use GDK instead of direct X11 calls.
I personally like code dealing with X11 being in x11.c instead of vdagent.c.
Hi,
In the future, I'd like to replace all X11 code with GTK (I'm not sure this
will be possible everywhere though). So moving this code from x11.c into
vdagent.c might be the first step towards removing x11.c entirely. Moving
code handling clipboard into clipboard.c and using GTK will follow
hopefully soon.
Frediano
Jakub
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20171030/3386ce95/attachment-0001.html>
More information about the Spice-devel
mailing list