Unity/X11: Avoid infinite loops.

When fielding a moveresize request, the guest is expected to return the
window's resulting coordinates, even if the request failed.  The Unity/
X11 code was written such that after issuing the moveresize request to
the X server, it'd recursively & manually pump the event loop until
receiving a configurenotify event for the window in question.
Unfortunately, there is no guarantee that such an event will ever
arrive.

Since our implementation is synchronous with respect to the X server,
that logic isn't at all needed.  Instead, we'll simply flush the X
channel and make sure all requests have been handled (using XSync),
and query the X server again for the new coordinates.

Now, in theory, this could introduce some latency to the moveresize
operation, but correctness trumps performance right now.  If
performance is ever important, this code will need a rewrite, anyway.

Signed-off-by: Marcelo Vanzin <mvanzin@vmware.com>

diff --git a/open-vm-tools/lib/unity/unityPlatformX11Window.c b/open-vm-tools/lib/unity/unityPlatformX11Window.c
index 14b376d836e20887c49efa973d8f32e87ab62a1d..12978d71af8e8c07e8cde9f9857a60d37acdbd1d 100644 (file)
--- a/open-vm-tools/lib/unity/unityPlatformX11Window.c
+++ b/open-vm-tools/lib/unity/unityPlatformX11Window.c
@@ -1555,6 +1555,7 @@ UnityPlatformMoveResizeWindow(UnityPlatform *up,         // IN
    UnityPlatformWindow *upw;
    Bool retval = FALSE;
    XWindowAttributes winAttr;
+   XWindowAttributes newAttr;
    UnityRect desiredRect;
 
    ASSERT(moveResizeRect);
@@ -1566,11 +1567,6 @@ UnityPlatformMoveResizeWindow(UnityPlatform *up,         // IN
 
    desiredRect = *moveResizeRect;
 
-   if (upw->lastConfigureEvent) {
-      free(upw->lastConfigureEvent);
-      upw->lastConfigureEvent = NULL;
-   }
-
    UnityPlatformResetErrorCount(up);
    XGetWindowAttributes(up->display, upw->toplevelWindow, &winAttr);
    if (UnityPlatformGetErrorCount(up)) {
@@ -1634,45 +1630,14 @@ UnityPlatformMoveResizeWindow(UnityPlatform *up,         // IN
       }
    }
 
-   /*
-    * Protect against the window being destroyed while we're waiting for results of the
-    * resize.
-    */
-   UPWindow_Ref(up, upw);
+   XSync(up->display, False);
+   XGetWindowAttributes(up->display, upw->toplevelWindow, &newAttr);
+   moveResizeRect->x = newAttr.x;
+   moveResizeRect->y = newAttr.y;
+   moveResizeRect->width = newAttr.width;
+   moveResizeRect->height = newAttr.height;
 
-   /*
-    * Because the window manager may take a non-trivial amount of time to process the
-    * move/resize request, we have to spin here until a ConfigureNotify event is
-    * generated on the window.
-    */
-   while (!upw->lastConfigureEvent) {
-      Debug("Running main loop iteration\n");
-      UnityPlatformProcessMainLoop(); // Process events, do other Unity stuff, etc.
-   }
-
-   if (upw->lastConfigureEvent && upw->lastConfigureEvent->window == upw->toplevelWindow) {
-      moveResizeRect->x = upw->lastConfigureEvent->x;
-      moveResizeRect->y = upw->lastConfigureEvent->y;
-      moveResizeRect->width = upw->lastConfigureEvent->width;
-      moveResizeRect->height = upw->lastConfigureEvent->height;
-
-      retval = TRUE;
-   } else {
-      /*
-       * There are cases where we only get a ConfigureNotify on the clientWindow because
-       * no actual change happened, in which case we just verify that we have the right
-       * toplevelWindow position and size.
-       */
-      Debug("Didn't get lastConfigureEvent on the toplevel window - requerying\n");
-
-      XGetWindowAttributes(up->display, upw->toplevelWindow, &winAttr);
-      moveResizeRect->x = winAttr.x;
-      moveResizeRect->y = winAttr.y;
-      moveResizeRect->width = winAttr.width;
-      moveResizeRect->height = winAttr.height;
-
-      retval = TRUE;
-   }
+   retval = TRUE;
 
    Debug("MoveResizeWindow(%#lx/%#lx): original (%d,%d)+(%d,%d), desired (%d,%d)+(%d,%d), actual (%d,%d)+(%d,%d) = %d\n",
          upw->toplevelWindow, upw->clientWindow,
@@ -1682,8 +1647,6 @@ UnityPlatformMoveResizeWindow(UnityPlatform *up,         // IN
          moveResizeRect->width, moveResizeRect->height,
          retval);
 
-   UPWindow_Unref(up, upw);
-
    return retval;
 }
 
@@ -2523,14 +2486,6 @@ UPWindowProcessConfigureEvent(UnityPlatform *up,        // IN
       const int x = xevent->xconfigure.x;
       const int y = xevent->xconfigure.y;
 
-      /*
-       * Used for implementing the move_resize operation.
-       */
-      if (!upw->lastConfigureEvent) {
-         upw->lastConfigureEvent = Util_SafeMalloc(sizeof *upw->lastConfigureEvent);
-      }
-      *upw->lastConfigureEvent = xevent->xconfigure;
-
       Debug("Moving window %#lx/%#lx to (%d, %d) +(%d, %d)\n",
             upw->toplevelWindow, upw->clientWindow,
             x - border_width,
@@ -2553,10 +2508,6 @@ UPWindowProcessConfigureEvent(UnityPlatform *up,        // IN
          UPWindow_Restack(up, upw, xevent->xconfigure.above);
       }
    } else {
-      if (!upw->lastConfigureEvent) {
-         upw->lastConfigureEvent = Util_SafeMalloc(sizeof *upw->lastConfigureEvent);
-         *upw->lastConfigureEvent = xevent->xconfigure;
-      }
       Debug("ProcessConfigureEvent skipped event on window %#lx (upw was %#lx/%#lx)\n",
             xevent->xconfigure.window, upw->toplevelWindow, upw->clientWindow);
    }

diff --git a/open-vm-tools/lib/unity/unityX11.h b/open-vm-tools/lib/unity/unityX11.h
index 4704f98b588d46cfe6428797c489627760703f83..52e8ce0ba7c8cff87f10cdcd5039f067adff6714 100644 (file)
--- a/open-vm-tools/lib/unity/unityX11.h
+++ b/open-vm-tools/lib/unity/unityX11.h
@@ -345,7 +345,6 @@ struct UnityPlatformWindow {
       UnityIconType type;
    } iconPng;
 
-   XConfigureEvent *lastConfigureEvent; // Used for replying to MoveResizeWindow
    Bool windowProtocols[UNITY_X11_MAX_WIN_PROTOCOLS];
 
    Bool isRelevant; // Whether the window is relayed through the window tracker