From 851b50419907276fdc781b6be2d42b38849bbd77 Mon Sep 17 00:00:00 2001 From: Jon TURNEY Date: Tue, 24 Sep 2013 16:02:37 +0100 Subject: [PATCH] hw/xwin: Retrieve TARGETS to avoid unnecessary failing conversion attempts See http://cygwin.com/ml/cygwin-xfree/2013-07/msg00016.html It looks like the change in a9aca218f557c723e637287272819a7c17174e1e had some unforseen consequences. If the X11 selection contents are not convertable to COMPOUND_TEXT, UTF8_STRING or STRING format (for example, if it is an image), after those conversion attempts have failed, we sit in winProcessXEventsTimeout() until the timeout expires. It also seems that maybe gnuplot doesn't respond correctly to this sequence of conversion requests and doesn't reply to some of them, which also causes us to sit in winProcessXEventsTimeout() until the timeout expires. The Windows application which has requested the clipboard contents via GetClipboardContents() is blocked until we return from WM_RENDERFORMAT, so sitting waiting for this timeout to expire should be avoided. So instead, explicitly request conversion to the TARGETS target, choose the most preferred format, and request conversion to that. Also: if there is no owned selection, there is nothing to paste, so don't bother trying to convert it. v2: Fix compilation with -Werror=declaration-after-statement Signed-off-by: Jon TURNEY Reviewed-by: Colin Harrison --- hw/xwin/winclipboard/internal.h | 13 ++- hw/xwin/winclipboard/thread.c | 6 +- hw/xwin/winclipboard/wndproc.c | 139 +++++++++++++++++++++++++------- hw/xwin/winclipboard/xevents.c | 123 ++++++++++++++++------------ 4 files changed, 195 insertions(+), 86 deletions(-) diff --git a/hw/xwin/winclipboard/internal.h b/hw/xwin/winclipboard/internal.h index 94956f80d..bcf45ca4d 100644 --- a/hw/xwin/winclipboard/internal.h +++ b/hw/xwin/winclipboard/internal.h @@ -39,8 +39,9 @@ #include #define WIN_XEVENTS_SUCCESS 0 -#define WIN_XEVENTS_CONVERT 2 -#define WIN_XEVENTS_NOTIFY 3 +#define WIN_XEVENTS_FAILED 1 +#define WIN_XEVENTS_NOTIFY_DATA 3 +#define WIN_XEVENTS_NOTIFY_TARGETS 4 #define WM_WM_REINIT (WM_USER + 1) @@ -95,9 +96,15 @@ typedef struct * winclipboardxevents.c */ +typedef struct +{ + Bool fUseUnicode; + Atom *targetList; +} ClipboardConversionData; + int winClipboardFlushXEvents(HWND hwnd, - Window iWindow, Display * pDisplay, Bool fUnicodeSupport, ClipboardAtoms *atom); + Window iWindow, Display * pDisplay, ClipboardConversionData *data, ClipboardAtoms *atom); Atom diff --git a/hw/xwin/winclipboard/thread.c b/hw/xwin/winclipboard/thread.c index c179e3f83..be053574d 100644 --- a/hw/xwin/winclipboard/thread.c +++ b/hw/xwin/winclipboard/thread.c @@ -123,6 +123,7 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) int iSelectError; Bool fShutdown = FALSE; static Bool fErrorHandlerSet = FALSE; + ClipboardConversionData data; winDebug("winClipboardProc - Hello\n"); @@ -260,7 +261,8 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) * because there may be events in local data structures * already. */ - winClipboardFlushXEvents(hwnd, iWindow, pDisplay, fUseUnicode, &atoms); + data.fUseUnicode = fUseUnicode; + winClipboardFlushXEvents(hwnd, iWindow, pDisplay, &data, &atoms); /* Pre-flush Windows messages */ if (!winClipboardFlushWindowsMessageQueue(hwnd)) { @@ -318,7 +320,7 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay) /* Branch on which descriptor became active */ if (FD_ISSET(iConnectionNumber, &fdsRead)) { /* Process X events */ - winClipboardFlushXEvents(hwnd, iWindow, pDisplay, fUseUnicode, &atoms); + winClipboardFlushXEvents(hwnd, iWindow, pDisplay, &data, &atoms); } #ifdef HAS_DEVWINDOWS diff --git a/hw/xwin/winclipboard/wndproc.c b/hw/xwin/winclipboard/wndproc.c index 165ff558a..23a0ac9bf 100644 --- a/hw/xwin/winclipboard/wndproc.c +++ b/hw/xwin/winclipboard/wndproc.c @@ -45,6 +45,7 @@ #include #include +#include #include @@ -64,7 +65,7 @@ static int winProcessXEventsTimeout(HWND hwnd, Window iWindow, Display * pDisplay, - Bool fUseUnicode, ClipboardAtoms *atoms, int iTimeoutSec) + ClipboardConversionData *data, ClipboardAtoms *atoms, int iTimeoutSec) { int iConnNumber; struct timeval tv; @@ -117,14 +118,14 @@ winProcessXEventsTimeout(HWND hwnd, Window iWindow, Display * pDisplay, /* Process X events */ /* Exit when we see that server is shutting down */ iReturn = winClipboardFlushXEvents(hwnd, - iWindow, pDisplay, fUseUnicode, atoms); + iWindow, pDisplay, data, atoms); winDebug ("winProcessXEventsTimeout () - winClipboardFlushXEvents returned %d\n", iReturn); - if (WIN_XEVENTS_NOTIFY == iReturn) { - /* Bail out if notify processed */ + if ((WIN_XEVENTS_NOTIFY_DATA == iReturn) || (WIN_XEVENTS_NOTIFY_TARGETS == iReturn) || (WIN_XEVENTS_FAILED == iReturn)) { + /* Bail out */ return iReturn; } } @@ -415,6 +416,10 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) { int iReturn; Bool fConvertToUnicode; + Bool pasted = FALSE; + Atom selection; + ClipboardConversionData data; + int best_target = 0; winDebug("winClipboardWindowProc - WM_RENDER*FORMAT - Hello.\n"); @@ -424,18 +429,88 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) else fConvertToUnicode = (CF_UNICODETEXT == wParam); - /* Request the selection contents */ - iReturn = XConvertSelection(pDisplay, - winClipboardGetLastOwnedSelectionAtom(atoms), - atoms->atomCompoundText, - atoms->atomLocalProperty, - iWindow, CurrentTime); - if (iReturn == BadAtom || iReturn == BadWindow) { - ErrorF("winClipboardWindowProc - WM_RENDER*FORMAT - " - "XConvertSelection () failed\n"); - break; + selection = winClipboardGetLastOwnedSelectionAtom(atoms); + if (selection == None) { + ErrorF("winClipboardWindowProc - no monitored selection is owned\n"); + goto fake_paste; } + winDebug("winClipboardWindowProc - requesting targets for selection from owner\n"); + + /* Request the selection's supported conversion targets */ + XConvertSelection(pDisplay, + selection, + atoms->atomTargets, + atoms->atomLocalProperty, + iWindow, CurrentTime); + + /* Process X events */ + data.fUseUnicode = fConvertToUnicode; + iReturn = winProcessXEventsTimeout(hwnd, + iWindow, + pDisplay, + &data, + atoms, + WIN_POLL_TIMEOUT); + + if (WIN_XEVENTS_NOTIFY_TARGETS != iReturn) { + ErrorF + ("winClipboardWindowProc - timed out waiting for WIN_XEVENTS_NOTIFY_TARGETS\n"); + goto fake_paste; + } + + /* Choose the most preferred target */ + { + struct target_priority + { + Atom target; + unsigned int priority; + }; + + struct target_priority target_priority_table[] = + { + { atoms->atomCompoundText, 0 }, +#ifdef X_HAVE_UTF8_STRING + { atoms->atomUTF8String, 1 }, +#endif + { XA_STRING, 2 }, + }; + + int best_priority = INT_MAX; + + int i,j; + for (i = 0 ; data.targetList[i] != 0; i++) + { + for (j = 0; j < sizeof(target_priority_table)/sizeof(struct target_priority); j ++) + { + if ((data.targetList[i] == target_priority_table[j].target) && + (target_priority_table[j].priority < best_priority)) + { + best_target = target_priority_table[j].target; + best_priority = target_priority_table[j].priority; + } + } + } + } + + free(data.targetList); + data.targetList = 0; + + winDebug("winClipboardWindowProc - best target is %d\n", best_target); + + /* No useful targets found */ + if (best_target == 0) + goto fake_paste; + + winDebug("winClipboardWindowProc - requesting selection from owner\n"); + + /* Request the selection contents */ + XConvertSelection(pDisplay, + selection, + best_target, + atoms->atomLocalProperty, + iWindow, CurrentTime); + /* Special handling for WM_RENDERALLFORMATS */ if (message == WM_RENDERALLFORMATS) { /* We must open and empty the clipboard */ @@ -449,40 +524,47 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) ErrorF("winClipboardWindowProc - WM_RENDER*FORMATS - " "OpenClipboard () failed: %08x\n", GetLastError()); - break; } if (!EmptyClipboard()) { ErrorF("winClipboardWindowProc - WM_RENDER*FORMATS - " "EmptyClipboard () failed: %08x\n", GetLastError()); - break; } } - /* Process the SelectionNotify event */ + /* Process X events */ iReturn = winProcessXEventsTimeout(hwnd, iWindow, pDisplay, - fConvertToUnicode, + &data, atoms, WIN_POLL_TIMEOUT); /* - * The last call to winProcessXEventsTimeout - * from above had better have seen a notify event, or else we - * are dealing with a buggy or old X11 app. In these cases we - * have to paste some fake data to the Win32 clipboard to - * satisfy the requirement that we write something to it. + * winProcessXEventsTimeout had better have seen a notify event, + * or else we are dealing with a buggy or old X11 app. */ - if (WIN_XEVENTS_NOTIFY != iReturn) { + if (WIN_XEVENTS_NOTIFY_DATA != iReturn) { + ErrorF + ("winClipboardWindowProc - timed out waiting for WIN_XEVENTS_NOTIFY_DATA\n"); + } + else { + pasted = TRUE; + } + + /* + * If we couldn't get the data from the X clipboard, we + * have to paste some fake data to the Win32 clipboard to + * satisfy the requirement that we write something to it. + */ + fake_paste: + if (!pasted) + { /* Paste no data, to satisfy required call to SetClipboardData */ SetClipboardData(CF_UNICODETEXT, NULL); SetClipboardData(CF_TEXT, NULL); - - ErrorF - ("winClipboardWindowProc - timed out waiting for WIN_XEVENTS_NOTIFY\n"); - } + } /* Special handling for WM_RENDERALLFORMATS */ if (message == WM_RENDERALLFORMATS) { @@ -492,7 +574,6 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) ErrorF("winClipboardWindowProc - WM_RENDERALLFORMATS - " "CloseClipboard () failed: %08x\n", GetLastError()); - break; } } diff --git a/hw/xwin/winclipboard/xevents.c b/hw/xwin/winclipboard/xevents.c index 42e9147b5..3dca35496 100644 --- a/hw/xwin/winclipboard/xevents.c +++ b/hw/xwin/winclipboard/xevents.c @@ -134,13 +134,59 @@ winClipboardInitMonitoredSelections(void) lastOwnedSelectionIndex = CLIP_OWN_NONE; } +static int +winClipboardSelectionNotifyTargets(HWND hwnd, Window iWindow, Display *pDisplay, ClipboardConversionData *data, ClipboardAtoms *atoms) +{ + Atom type; + int format; + unsigned long nitems; + unsigned long after; + Atom *prop; + + /* Retrieve the selection data and delete the property */ + int iReturn = XGetWindowProperty(pDisplay, + iWindow, + atoms->atomLocalProperty, + 0, + INT_MAX, + True, + AnyPropertyType, + &type, + &format, + &nitems, + &after, + (unsigned char **)&prop); + if (iReturn != Success) { + ErrorF("winClipboardFlushXEvents - SelectionNotify - " + "XGetWindowProperty () failed, aborting: %d\n", iReturn); + } else { + int i; + data->targetList = malloc((nitems+1)*sizeof(Atom)); + + for (i = 0; i < nitems; i++) + { + Atom atom = prop[i]; + char *pszAtomName = XGetAtomName(pDisplay, atom); + data->targetList[i] = atom; + winDebug("winClipboardFlushXEvents - SelectionNotify - target[%d] %d = %s\n", i, atom, pszAtomName); + XFree(pszAtomName); + } + + data->targetList[nitems] = 0; + + XFree(prop); + } + + return WIN_XEVENTS_NOTIFY_TARGETS; +} + /* * Process any pending X events */ int winClipboardFlushXEvents(HWND hwnd, - Window iWindow, Display * pDisplay, Bool fUseUnicode, ClipboardAtoms *atoms) + Window iWindow, Display * pDisplay, ClipboardConversionData *data, ClipboardAtoms *atoms) { Atom atomClipboard = atoms->atomClipboard; Atom atomLocalProperty = atoms->atomLocalProperty; @@ -273,7 +319,7 @@ winClipboardFlushXEvents(HWND hwnd, fCloseClipboard = TRUE; /* Check that clipboard format is available */ - if (fUseUnicode && !IsClipboardFormatAvailable(CF_UNICODETEXT)) { + if (data->fUseUnicode && !IsClipboardFormatAvailable(CF_UNICODETEXT)) { static int count; /* Hack to stop acroread spamming the log */ static HWND lasthwnd; /* I've not seen any other client get here repeatedly? */ @@ -290,7 +336,7 @@ winClipboardFlushXEvents(HWND hwnd, fAbort = TRUE; goto winClipboardFlushXEvents_SelectionRequest_Done; } - else if (!fUseUnicode && !IsClipboardFormatAvailable(CF_TEXT)) { + else if (!data->fUseUnicode && !IsClipboardFormatAvailable(CF_TEXT)) { ErrorF("winClipboardFlushXEvents - CF_TEXT is not " "available from Win32 clipboard. Aborting.\n"); @@ -312,7 +358,7 @@ winClipboardFlushXEvents(HWND hwnd, xiccesStyle = XStringStyle; /* Get a pointer to the clipboard text, in desired format */ - if (fUseUnicode) { + if (data->fUseUnicode) { /* Retrieve clipboard data */ hGlobal = GetClipboardData(CF_UNICODETEXT); } @@ -331,7 +377,7 @@ winClipboardFlushXEvents(HWND hwnd, pszGlobalData = (char *) GlobalLock(hGlobal); /* Convert the Unicode string to UTF8 (MBCS) */ - if (fUseUnicode) { + if (data->fUseUnicode) { iConvertDataLen = WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR) pszGlobalData, @@ -362,7 +408,7 @@ winClipboardFlushXEvents(HWND hwnd, xtpText.nitems = 0; /* Create the text property from the text list */ - if (fUseUnicode) { + if (data->fUseUnicode) { #ifdef X_HAVE_UTF8_STRING iReturn = Xutf8TextListToTextProperty(pDisplay, pszTextList, @@ -507,49 +553,22 @@ winClipboardFlushXEvents(HWND hwnd, } /* - * Request conversion of UTF8 and CompoundText targets. - */ + SelectionNotify with property of None indicates either: + + (i) Generated by the X server if no owner for the specified selection exists + (perhaps it's disappeared on us mid-transaction), or + (ii) Sent by the selection owner when the requested selection conversion could + not be performed or server errors prevented the conversion data being returned + */ if (event.xselection.property == None) { - if (event.xselection.target == XA_STRING) { - winDebug("winClipboardFlushXEvents - SelectionNotify - " - "XA_STRING\n"); - - return WIN_XEVENTS_CONVERT; - } - else if (event.xselection.target == atomUTF8String) { - winDebug("winClipboardFlushXEvents - SelectionNotify - " - "Requesting conversion of UTF8 target.\n"); - - XConvertSelection(pDisplay, - event.xselection.selection, - XA_STRING, - atomLocalProperty, iWindow, CurrentTime); - - /* Process the ConvertSelection event */ - XFlush(pDisplay); - return WIN_XEVENTS_CONVERT; - } -#ifdef X_HAVE_UTF8_STRING - else if (event.xselection.target == atomCompoundText) { - winDebug("winClipboardFlushXEvents - SelectionNotify - " - "Requesting conversion of CompoundText target.\n"); - - XConvertSelection(pDisplay, - event.xselection.selection, - atomUTF8String, - atomLocalProperty, iWindow, CurrentTime); - - /* Process the ConvertSelection event */ - XFlush(pDisplay); - return WIN_XEVENTS_CONVERT; - } -#endif - else { ErrorF("winClipboardFlushXEvents - SelectionNotify - " - "Unknown format. Cannot request conversion, " - "aborting.\n"); - break; + "Conversion to format %d refused.\n", + event.xselection.target); + return WIN_XEVENTS_FAILED; } + + if (event.xselection.target == atomTargets) { + return winClipboardSelectionNotifyTargets(hwnd, iWindow, pDisplay, data, atoms); } /* Retrieve the selection data and delete the property */ @@ -567,7 +586,7 @@ winClipboardFlushXEvents(HWND hwnd, if (iReturn != Success) { ErrorF("winClipboardFlushXEvents - SelectionNotify - " "XGetWindowProperty () failed, aborting: %d\n", iReturn); - break; + goto winClipboardFlushXEvents_SelectionNotify_Done; } { @@ -581,7 +600,7 @@ winClipboardFlushXEvents(HWND hwnd, pszAtomName = NULL; } - if (fUseUnicode) { + if (data->fUseUnicode) { #ifdef X_HAVE_UTF8_STRING /* Convert the text property to a text list */ iReturn = Xutf8TextPropertyToTextList(pDisplay, @@ -648,7 +667,7 @@ winClipboardFlushXEvents(HWND hwnd, /* Convert the X clipboard string to DOS format */ winClipboardUNIXtoDOS(&pszReturnData, strlen(pszReturnData)); - if (fUseUnicode) { + if (data->fUseUnicode) { /* Find out how much space needed to convert MBCS to Unicode */ iUnicodeLen = MultiByteToWideChar(CP_UTF8, 0, @@ -707,7 +726,7 @@ winClipboardFlushXEvents(HWND hwnd, } /* Copy the returned string into the global memory */ - if (fUseUnicode) { + if (data->fUseUnicode) { memcpy(pszGlobalData, pwszUnicodeStr, sizeof(wchar_t) * (iUnicodeLen + 1)); free(pwszUnicodeStr); @@ -724,7 +743,7 @@ winClipboardFlushXEvents(HWND hwnd, pszGlobalData = NULL; /* Push the selection data to the Windows clipboard */ - if (fUseUnicode) + if (data->fUseUnicode) SetClipboardData(CF_UNICODETEXT, hGlobal); else SetClipboardData(CF_TEXT, hGlobal); @@ -754,7 +773,7 @@ winClipboardFlushXEvents(HWND hwnd, SetClipboardData(CF_UNICODETEXT, NULL); SetClipboardData(CF_TEXT, NULL); } - return WIN_XEVENTS_NOTIFY; + return WIN_XEVENTS_NOTIFY_DATA; case SelectionClear: winDebug("SelectionClear - doing nothing\n");