From 196330111b49741c7018624de7c8c8e989fdf208 Mon Sep 17 00:00:00 2001 From: Norbert Federa Date: Mon, 7 Jul 2014 20:26:41 +0200 Subject: [PATCH] xfreerdp: xfixes selection ownership notification The X11 core protocol does not have support for selection ownership notifications. Until now xfreerdp worked around this issue by always sending a format list pdu to the server after sending the format data response pdu which makes the server side think that the clients clipboard data has changed. This workaround has some severe drawbacks: * it causes unnecessary data transfers because even without local clipboard data changes the same data is always re-transferred over the channel * with some clipboard managers (in the server sessions) you will get massive endless data transfer loops because these managers immediately request the data on clipboard changes. The correct (core X11) way would be polling for selection ownership changes which must include the ability to detect changes to the TIMESTAMP target if the selection owner did not change. The alternative to the poll based approach is using the X Fixes extension in order to get selection ownership notifications. This commit adds support for the XFIXES solution and also moves the complete clipboard related event handling from xf_event.c to xf_cliprdr.c --- client/X11/CMakeLists.txt | 11 +++++ client/X11/xf_cliprdr.c | 100 +++++++++++++++++++++++++++++++++++--- client/X11/xf_cliprdr.h | 7 +-- client/X11/xf_event.c | 58 ++-------------------- 4 files changed, 110 insertions(+), 66 deletions(-) diff --git a/client/X11/CMakeLists.txt b/client/X11/CMakeLists.txt index 6c004e678..e57f3a18d 100644 --- a/client/X11/CMakeLists.txt +++ b/client/X11/CMakeLists.txt @@ -148,6 +148,10 @@ set(XRENDER_FEATURE_TYPE "RECOMMENDED") set(XRENDER_FEATURE_PURPOSE "rendering") set(XRENDER_FEATURE_DESCRIPTION "X11 render extension") +set(XFIXES_FEATURE_TYPE "RECOMMENDED") +set(XFIXES_FEATURE_PURPOSE "X11 xfixes extension") +set(XFIXES_FEATURE_DESCRIPTION "Useful additions to the X11 core protocol") + find_feature(XShm ${XSHM_FEATURE_TYPE} ${XSHM_FEATURE_PURPOSE} ${XSHM_FEATURE_DESCRIPTION}) find_feature(Xinerama ${XINERAMA_FEATURE_TYPE} ${XINERAMA_FEATURE_PURPOSE} ${XINERAMA_FEATURE_DESCRIPTION}) find_feature(Xext ${XEXT_FEATURE_TYPE} ${XEXT_FEATURE_PURPOSE} ${XEXT_FEATURE_DESCRIPTION}) @@ -155,6 +159,7 @@ find_feature(Xcursor ${XCURSOR_FEATURE_TYPE} ${XCURSOR_FEATURE_PURPOSE} ${XCURSO find_feature(Xv ${XV_FEATURE_TYPE} ${XV_FEATURE_PURPOSE} ${XV_FEATURE_DESCRIPTION}) find_feature(Xi ${XI_FEATURE_TYPE} ${XI_FEATURE_PURPOSE} ${XI_FEATURE_DESCRIPTION}) find_feature(Xrender ${XRENDER_FEATURE_TYPE} ${XRENDER_FEATURE_PURPOSE} ${XRENDER_FEATURE_DESCRIPTION}) +find_feature(Xfixes ${XFIXES_FEATURE_TYPE} ${XFIXES_FEATURE_PURPOSE} ${XFIXES_FEATURE_DESCRIPTION}) if(WITH_XINERAMA) add_definitions(-DWITH_XINERAMA) @@ -192,6 +197,12 @@ if(WITH_XRENDER) set(${MODULE_PREFIX}_LIBS ${${MODULE_PREFIX}_LIBS} ${XRENDER_LIBRARIES}) endif() +if(WITH_XFIXES) + add_definitions(-DWITH_XFIXES) + include_directories(${XFIXES_INCLUDE_DIRS}) + set(${MODULE_PREFIX}_LIBS ${${MODULE_PREFIX}_LIBS} ${XFIXES_LIBRARIES}) +endif() + include_directories(${CMAKE_SOURCE_DIR}/resources) set(${MODULE_PREFIX}_LIBS ${${MODULE_PREFIX}_LIBS} freerdp-client) diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c index 8fb49f98e..eabdac19e 100644 --- a/client/X11/xf_cliprdr.c +++ b/client/X11/xf_cliprdr.c @@ -25,6 +25,10 @@ #include #include +#ifdef WITH_XFIXES +#include +#endif + #include #include @@ -88,6 +92,11 @@ struct clipboard_context BOOL incr_starts; BYTE* incr_data; int incr_data_length; + + /* X Fixes extension */ + int xfixes_event_base; + int xfixes_error_base; + BOOL xfixes_supported; }; void xf_cliprdr_init(xfContext* xfc, rdpChannels* channels) @@ -121,6 +130,30 @@ void xf_cliprdr_init(xfContext* xfc, rdpChannels* channels) XSelectInput(xfc->display, cb->root_window, PropertyChangeMask); +#ifdef WITH_XFIXES + if (XFixesQueryExtension(xfc->display, &cb->xfixes_event_base, &cb->xfixes_error_base)) + { + int xfmajor, xfminor; + if (XFixesQueryVersion(xfc->display, &xfmajor, &xfminor)) + { + DEBUG_X11_CLIPRDR("Found X Fixes extension version %d.%d", xfmajor, xfminor); + XFixesSelectSelectionInput(xfc->display, cb->root_window, + cb->clipboard_atom, XFixesSetSelectionOwnerNotifyMask); + cb->xfixes_supported = TRUE; + } + else + { + fprintf(stderr, "%s: Error querying X Fixes extension version\n", __FUNCTION__); + } + } + else + { + fprintf(stderr, "%s: Error loading X Fixes extension\n", __FUNCTION__); + } +#else + fprintf(stderr, "Warning: Using clipboard redirection without XFIXES extension is strongly discouraged!\n"); +#endif + n = 0; cb->format_mappings[n].target_format = XInternAtom(xfc->display, "_FREERDP_RAW", FALSE); cb->format_mappings[n].format_id = CB_FORMAT_RAW; @@ -718,8 +751,11 @@ static void xf_cliprdr_process_requested_data(xfContext* xfc, BOOL has_data, BYT else xf_cliprdr_send_null_data_response(xfc); - /* Resend the format list, otherwise the server won't request again for the next paste */ - xf_cliprdr_send_format_list(xfc); + if (!cb->xfixes_supported) + { + /* Resend the format list, otherwise the server won't request again for the next paste */ + xf_cliprdr_send_format_list(xfc); + } } static BOOL xf_cliprdr_get_requested_data(xfContext* xfc, Atom target) @@ -1077,7 +1113,7 @@ void xf_process_cliprdr_event(xfContext* xfc, wMessage* event) } } -BOOL xf_cliprdr_process_selection_notify(xfContext* xfc, XEvent* xevent) +static BOOL xf_cliprdr_process_selection_notify(xfContext* xfc, XEvent* xevent) { clipboardContext* cb = (clipboardContext*) xfc->clipboard_context; @@ -1101,7 +1137,7 @@ BOOL xf_cliprdr_process_selection_notify(xfContext* xfc, XEvent* xevent) } } -BOOL xf_cliprdr_process_selection_request(xfContext* xfc, XEvent* xevent) +static BOOL xf_cliprdr_process_selection_request(xfContext* xfc, XEvent* xevent) { int i; int fmt; @@ -1218,7 +1254,7 @@ BOOL xf_cliprdr_process_selection_request(xfContext* xfc, XEvent* xevent) return TRUE; } -BOOL xf_cliprdr_process_selection_clear(xfContext* xfc, XEvent* xevent) +static BOOL xf_cliprdr_process_selection_clear(xfContext* xfc, XEvent* xevent) { clipboardContext* cb = (clipboardContext*) xfc->clipboard_context; @@ -1230,7 +1266,7 @@ BOOL xf_cliprdr_process_selection_clear(xfContext* xfc, XEvent* xevent) return TRUE; } -BOOL xf_cliprdr_process_property_notify(xfContext* xfc, XEvent* xevent) +static BOOL xf_cliprdr_process_property_notify(xfContext* xfc, XEvent* xevent) { clipboardContext* cb = (clipboardContext*) xfc->clipboard_context; @@ -1257,7 +1293,7 @@ BOOL xf_cliprdr_process_property_notify(xfContext* xfc, XEvent* xevent) return TRUE; } -void xf_cliprdr_check_owner(xfContext* xfc) +static void xf_cliprdr_check_owner(xfContext* xfc) { Window owner; clipboardContext* cb = (clipboardContext*) xfc->clipboard_context; @@ -1274,3 +1310,53 @@ void xf_cliprdr_check_owner(xfContext* xfc) } } +void xf_cliprdr_handle_xevent(xfContext* xfc, XEvent* event) +{ + clipboardContext* cb; + + if (!xfc || !event) + return; + + if (!(cb = (clipboardContext*) xfc->clipboard_context)) + return; + +#ifdef WITH_XFIXES + if (cb->xfixes_supported && event->type == XFixesSelectionNotify + cb->xfixes_event_base) + { + XFixesSelectionNotifyEvent* se = (XFixesSelectionNotifyEvent*) event; + if (se->subtype == XFixesSetSelectionOwnerNotify) + { + if (se->selection != cb->clipboard_atom) + return; + + if (XGetSelectionOwner(xfc->display, se->selection) == xfc->drawable) + return; + + cb->owner = None; + xf_cliprdr_check_owner(xfc); + } + return; + } +#endif + switch (event->type) + { + case SelectionNotify: + xf_cliprdr_process_selection_notify(xfc, event); + break; + case SelectionRequest: + xf_cliprdr_process_selection_request(xfc, event); + break; + case SelectionClear: + xf_cliprdr_process_selection_clear(xfc, event); + break; + case PropertyNotify: + xf_cliprdr_process_property_notify(xfc, event); + break; + case FocusIn: + if (!cb->xfixes_supported) + { + xf_cliprdr_check_owner(xfc); + } + break; + } +} diff --git a/client/X11/xf_cliprdr.h b/client/X11/xf_cliprdr.h index 3e3d680ee..06713c76f 100644 --- a/client/X11/xf_cliprdr.h +++ b/client/X11/xf_cliprdr.h @@ -26,10 +26,5 @@ void xf_cliprdr_init(xfContext* xfc, rdpChannels* channels); void xf_cliprdr_uninit(xfContext* xfc); void xf_process_cliprdr_event(xfContext* xfc, wMessage* event); -BOOL xf_cliprdr_process_selection_notify(xfContext* xfc, XEvent* xevent); -BOOL xf_cliprdr_process_selection_request(xfContext* xfc, XEvent* xevent); -BOOL xf_cliprdr_process_selection_clear(xfContext* xfc, XEvent* xevent); -BOOL xf_cliprdr_process_property_notify(xfContext* xfc, XEvent* xevent); -void xf_cliprdr_check_owner(xfContext* xfc); - +void xf_cliprdr_handle_xevent(xfContext* xfc, XEvent* event); #endif /* __XF_CLIPRDR_H */ diff --git a/client/X11/xf_event.c b/client/X11/xf_event.c index a4692fe3a..e2c8bdfaf 100644 --- a/client/X11/xf_event.c +++ b/client/X11/xf_event.c @@ -545,9 +545,6 @@ static BOOL xf_event_FocusIn(xfContext* xfc, XEvent* event, BOOL app) xf_keyboard_focus_in(xfc); - if (!app) - xf_cliprdr_check_owner(xfc); - return TRUE; } @@ -793,39 +790,6 @@ static BOOL xf_event_UnmapNotify(xfContext* xfc, XEvent* event, BOOL app) return TRUE; } -static BOOL xf_event_SelectionNotify(xfContext* xfc, XEvent* event, BOOL app) -{ - if (!app) - { - if (xf_cliprdr_process_selection_notify(xfc, event)) - return TRUE; - } - - return TRUE; -} - -static BOOL xf_event_SelectionRequest(xfContext* xfc, XEvent* event, BOOL app) -{ - if (!app) - { - if (xf_cliprdr_process_selection_request(xfc, event)) - return TRUE; - } - - return TRUE; -} - -static BOOL xf_event_SelectionClear(xfContext* xfc, XEvent* event, BOOL app) -{ - if (!app) - { - if (xf_cliprdr_process_selection_clear(xfc, event)) - return TRUE; - } - - return TRUE; -} - static BOOL xf_event_PropertyNotify(xfContext* xfc, XEvent* event, BOOL app) { /* @@ -922,11 +886,6 @@ static BOOL xf_event_PropertyNotify(xfContext* xfc, XEvent* event, BOOL app) } } } - else - { - if (xf_cliprdr_process_property_notify(xfc, event)) - return TRUE; - } return TRUE; } @@ -1112,24 +1071,17 @@ BOOL xf_event_process(freerdp* instance, XEvent* event) status = xf_event_ClientMessage(xfc, event, xfc->remote_app); break; - case SelectionNotify: - status = xf_event_SelectionNotify(xfc, event, xfc->remote_app); - break; - - case SelectionRequest: - status = xf_event_SelectionRequest(xfc, event, xfc->remote_app); - break; - - case SelectionClear: - status = xf_event_SelectionClear(xfc, event, xfc->remote_app); - break; - case PropertyNotify: status = xf_event_PropertyNotify(xfc, event, xfc->remote_app); break; } + if (!xfc->remote_app) + { + xf_cliprdr_handle_xevent(xfc, event); + } + xf_input_handle_event(xfc, event); XSync(xfc->display, FALSE);