Keep ownership of second selection when first is lost

This fixes regression introduced by the extended clipboard extension.
Previously it was possible for the server to hold on to the CLIPBOARD
selection even if another application took ownership of PRIMARY. This is
important to handle the common use case of selecting something in order
to paste over it.

The new request based model doesn't readily support this as we assume
the client has lost its data once we push the new PRIMARY selection to
it. So to handle this we have the maintain a cache of the client's data,
and make sure to fill that cache before we do anything that might cause
the client to lose the data.

2 files changed, 64 insertions, 13 deletions

diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx
index 39cdde1f..fcc93a4e 100644
--- a/common/rfb/VNCServerST.cxx
+++ b/common/rfb/VNCServerST.cxx
@@ -338,8 +338,10 @@ void VNCServerST::setScreenLayout(const ScreenSet& layout)
 
 void VNCServerST::requestClipboard()
 {
-  if (clipboardClient == NULL)
+  if (clipboardClient == NULL) {
+    slog.debug("Got request for client clipboard but no client currently owns the clipboard");
     return;
+  }
 
   clipboardClient->requestClipboardOrClose();
 }
@@ -348,9 +350,6 @@ void VNCServerST::announceClipboard(bool available)
 {
   std::list<VNCSConnectionST*>::iterator ci, ci_next;
 
-  if (available)
-    clipboardClient = NULL;
-
   clipboardRequestors.clear();
 
   for (ci = clients.begin(); ci != clients.end(); ci = ci_next) {
diff --git a/unix/xserver/hw/vnc/vncSelection.c b/unix/xserver/hw/vnc/vncSelection.c
index a1af467a..8cde6de5 100644
--- a/unix/xserver/hw/vnc/vncSelection.c
+++ b/unix/xserver/hw/vnc/vncSelection.c
@@ -50,6 +50,8 @@ static Window wid;
 static Bool probing;
 static Atom activeSelection = None;
 
+static char* cachedData = NULL;
+
 struct VncDataTarget {
   ClientPtr client;
   Atom selection;
@@ -118,6 +120,11 @@ void vncHandleClipboardRequest(void)
 
 void vncHandleClipboardAnnounce(int available)
 {
+  /* The data has changed in some way, so whatever is in our cache is
+   * now stale */
+  free(cachedData);
+  cachedData = NULL;
+
   if (available) {
     int rc;
 
@@ -167,6 +174,9 @@ void vncHandleClipboardData(const char* data)
 
   LOG_DEBUG("Got remote clipboard data, sending to X11 clients");
 
+  free(cachedData);
+  cachedData = strdup(data);
+
   while (vncDataTargetHead != NULL) {
     int rc;
     xEvent event;
@@ -177,7 +187,7 @@ void vncHandleClipboardData(const char* data)
                              vncDataTargetHead->property,
                              vncDataTargetHead->requestor,
                              vncDataTargetHead->time,
-                             data);
+                             cachedData);
     if (rc != Success) {
       event.u.u.type = SelectionNotify;
       event.u.selectionNotify.time = vncDataTargetHead->time;
@@ -277,6 +287,49 @@ static int vncOwnSelection(Atom selection)
   return Success;
 }
 
+static Bool vncWeAreOwner(Atom selection)
+{
+  Selection *pSel;
+  int rc;
+
+  rc = dixLookupSelection(&pSel, selection, serverClient, DixReadAccess);
+  if (rc != Success)
+    return FALSE;
+
+  if (pSel->client != serverClient)
+    return FALSE;
+
+  if (pSel->window != wid)
+    return FALSE;
+
+  return TRUE;
+}
+
+static void vncMaybeRequestCache(void)
+{
+  /* Telling a client that we have clipboard data will likely mean that
+   * we can no longer request its clipboard data. This is a problem as
+   * we might initially own multiple selections and we now just lost
+   * one, and we still want to be able to service the other one. Solve
+   * this by requesting the data from the client when we can't affort to
+   * lose it and cache it. */
+
+  /* Already cached? */
+  if (cachedData != NULL)
+    return;
+
+  if (!vncWeAreOwner(xaCLIPBOARD)) {
+    if (!vncGetSetPrimary())
+      return;
+    if (!vncWeAreOwner(xaPRIMARY))
+      return;
+  }
+
+  LOG_DEBUG("Requesting clipboard data from client for caching");
+
+  vncRequestClipboard();
+}
+
 static int vncConvertSelection(ClientPtr client, Atom selection,
                                Atom target, Atom property,
                                Window requestor, CARD32 time,
@@ -419,12 +472,16 @@ static int vncProcConvertSelection(ClientPtr client)
     return BadAtom;
   }
 
+  /* Do we own this selection? */
   rc = dixLookupSelection(&pSel, stuff->selection, client, DixReadAccess);
   if (rc == Success && pSel->client == serverClient &&
       pSel->window == wid) {
+    /* cachedData will be NULL for the first request, but can then be
+     * reused once we've gotten the data once from the client */
     rc = vncConvertSelection(client, stuff->selection,
                              stuff->target, stuff->property,
-                             stuff->requestor, stuff->time, NULL);
+                             stuff->requestor, stuff->time,
+                             cachedData);
     if (rc != Success) {
       xEvent event;
 
@@ -511,6 +568,7 @@ static void vncHandleSelection(Atom selection, Atom target,
     if (probing) {
       if (vncHasAtom(xaSTRING, (const Atom*)prop->data, prop->size) ||
           vncHasAtom(xaUTF8_STRING, (const Atom*)prop->data, prop->size)) {
+        vncMaybeRequestCache();
         LOG_DEBUG("Compatible format found, notifying clients");
         activeSelection = selection;
         vncAnnounceClipboard(TRUE);
@@ -595,6 +653,7 @@ static void vncSelectionCallback(CallbackListPtr *callbacks,
   SelectionInfoRec *info = (SelectionInfoRec *) args;
 
   if (info->selection->selection == activeSelection) {
+    vncMaybeRequestCache();
     LOG_DEBUG("Local clipboard lost, notifying clients");
     activeSelection = None;
     vncAnnounceClipboard(FALSE);
@@ -608,13 +667,6 @@ static void vncSelectionCallback(CallbackListPtr *callbacks,
   LOG_DEBUG("Selection owner change for %s",
             NameForAtom(info->selection->selection));
 
-  /*
-   * If we're the previous owner of this selection, then we're also the
-   * owner of _the other_ selection. Make sure we drop all ownerships so
-   * we either own both selections or nonw.
-   */
-  DeleteWindowFromAnySelections(pWindow);
-
   if ((info->selection->selection != xaPRIMARY) &&
       (info->selection->selection != xaCLIPBOARD))
     return;