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.

Also: if there is no owned selection, there is nothing to paste, so don't bother
trying to convert it.

4 files changed, 194 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 <X11/Xwindows.h>
 
 #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 809e57846..2b9a6f34f 100644
--- a/hw/xwin/winclipboard/thread.c
+++ b/hw/xwin/winclipboard/thread.c
@@ -250,7 +250,9 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay)
      *       because there may be events in local data structures
      *       already.
      */
-    winClipboardFlushXEvents(hwnd, iWindow, pDisplay, fUseUnicode, &atoms);
+    ClipboardConversionData data;
+    data.fUseUnicode = fUseUnicode;
+    winClipboardFlushXEvents(hwnd, iWindow, pDisplay, &data, &atoms);
 
     /* Pre-flush Windows messages */
     if (!winClipboardFlushWindowsMessageQueue(hwnd)) {
@@ -316,7 +318,8 @@ winClipboardProc(Bool fUseUnicode, char *szDisplay)
                 ("winClipboardProc - X connection ready, pumping X event queue\n");
 
             /* 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 a171ca595..83f11417d 100644
--- a/hw/xwin/winclipboard/wndproc.c
+++ b/hw/xwin/winclipboard/wndproc.c
@@ -45,6 +45,7 @@
 
 #include <sys/types.h>
 #include <sys/time.h>
+#include <limits.h>
 
 #include <X11/Xatom.h>
 
@@ -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;
             }
         }
@@ -445,6 +446,7 @@ winClipboardWindowProc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
     {
         int iReturn;
         Bool fConvertToUnicode;
+        Bool pasted = FALSE;
 
         if (message == WM_RENDERALLFORMATS)
             winDebug("winClipboardWindowProc - WM_RENDERALLFORMATS - Hello.\n");
@@ -458,18 +460,89 @@ 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;
+        Atom 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 */
+        ClipboardConversionData data;
+        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 best_target = 0;
+
+        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 */
@@ -483,40 +556,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) {
@@ -526,7 +606,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 0c7a404ce..eba236802 100644
--- a/hw/xwin/winclipboard/xevents.c
+++ b/hw/xwin/winclipboard/xevents.c
@@ -135,13 +135,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];
+        data->targetList[i] = atom;
+        char *pszAtomName = XGetAtomName(pDisplay, 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;
@@ -277,7 +323,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? */
 
@@ -294,7 +340,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");
 
@@ -316,7 +362,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);
             }
@@ -335,7 +381,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,
@@ -366,7 +412,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,
@@ -512,49 +558,22 @@ winClipboardFlushXEvents(HWND hwnd,
             }
 
             /*
-             * Request conversion of UTF8 and CompoundText targets.
-             */
-            if (event.xselection.property == None) {
-                if (event.xselection.target == XA_STRING) {
-                    winDebug("winClipboardFlushXEvents - SelectionNotify - "
-                             "XA_STRING\n");
+              SelectionNotify with property of None indicates either:
 
-                    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 {
+              (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) {
                     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 */
@@ -572,7 +591,7 @@ winClipboardFlushXEvents(HWND hwnd,
             if (iReturn != Success) {
                 ErrorF("winClipboardFlushXEvents - SelectionNotify - "
                        "XGetWindowProperty () failed, aborting: %d\n", iReturn);
-                break;
+                goto winClipboardFlushXEvents_SelectionNotify_Done;
             }
 
             {
@@ -587,7 +606,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,
@@ -654,7 +673,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,
@@ -714,7 +733,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);
@@ -731,7 +750,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);
@@ -761,7 +780,7 @@ winClipboardFlushXEvents(HWND hwnd,
                 SetClipboardData(CF_UNICODETEXT, NULL);
                 SetClipboardData(CF_TEXT, NULL);
             }
-            return WIN_XEVENTS_NOTIFY;
+            return WIN_XEVENTS_NOTIFY_DATA;
 
         case SelectionClear:
             winDebug("winClipboardFlushXEvents - SelectionClear - doing nothing\n");