xwayland: Fix opaque override region on override-redirected windows

There is a race where the WM sends the opaque_override region for a window
but X has not sent yet the window surface. This patch fix it by introducing a
pointer that keeps the pending opaque override region, to be set later.

There is no way for the compositor tell beforehand whether a window received
its surface from X. The client also doesn't know about it, so the region
created on client side cannot be destroyed right after passed to
wm_xwin.set_opaque_override. Therefore this semantic of the request has
changed and described on the protocol.

This patch fix the problem where override-redirected X windows were not setting
opaque override region correctly.

Signed-off-by: Tiago Vignatti <tiago.vignatti@intel.com>

3 files changed, 37 insertions, 5 deletions

diff --git a/clients/xwm.c b/clients/xwm.c
index c457c58..edd7037 100644
--- a/clients/xwm.c
+++ b/clients/xwm.c
@@ -762,6 +762,7 @@ xwm_handle_map_notify(struct xwm_wm *wm, xcb_generic_event_t *event)
 	}
 
 	wm_xwin_map(window->xwin);
+	xwm_window_schedule_repaint(window);
 	fprintf(stderr, "XCB_MAP_NOTIFY (window %d)\n", map_notify->window);
 }
 
@@ -818,7 +819,6 @@ xwm_window_draw_decoration(struct xwm_window *window)
 		region = wl_compositor_create_region(wm->xwm->compositor);
 		wl_region_add(region, 0, 0, width, height);
 		wm_xwin_set_opaque_override(window->xwin, region);
-		wl_region_destroy(region);
 		return;
 	}
 
@@ -859,7 +859,6 @@ xwm_window_draw_decoration(struct xwm_window *window)
 	wl_region_add(region, x - 1, y - 1,
 		      window->width + 2, window->height + 2);
 	wm_xwin_set_opaque_override(window->xwin_frame, region);
-	wl_region_destroy(region);
 
 	region = wl_compositor_create_region(wm->xwm->compositor);
 	wl_region_add(region, t->margin, t->margin,
diff --git a/protocol/xserver.xml b/protocol/xserver.xml
index b76b01d..457daee 100644
--- a/protocol/xserver.xml
+++ b/protocol/xserver.xml
@@ -160,6 +160,12 @@
 	one is meant only as a hint for optimization while this is a necessity
 	for painting XWayland windows right. Therefore X must never use
 	wl_surface.set_opaque_region either.
+
+	There is no way for the compositor tell beforehand whether a window
+	received already its surface from X (xserver.set_window_surface). The
+	client also does not know about it, so the region created on client
+	side is destroyed implicitly inside the compositor after used. Thus
+	the client must not call wl_region_destroy for this request.
       </description>
 
       <arg name="region" type="object" interface="wl_region"/>
diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
index 1fbbbba..aa4e57e 100644
--- a/src/xwayland/window-manager.c
+++ b/src/xwayland/window-manager.c
@@ -36,6 +36,7 @@ struct xserver_window {
 
 	struct weston_xserver *wxs;
 	struct wl_surface *surface;
+	struct weston_region *pending_opaque;
 	struct shell_surface *shsurf;
 	struct wl_listener surface_destroy_listener;
 	struct wl_listener position_listener;
@@ -48,6 +49,10 @@ struct xserver_window {
 };
 
 static void
+set_opaque_override(struct weston_surface *surface,
+		    struct weston_region *region);
+
+static void
 surface_destroy(struct wl_listener *listener, void *data)
 {
 	struct xserver_window *window = container_of(listener,
@@ -55,6 +60,7 @@ surface_destroy(struct wl_listener *listener, void *data)
 
 	window->surface = NULL;
 	window->shsurf = NULL;
+	window->pending_opaque = NULL;
 
 	fprintf(stderr, "surface for xid %d destroyed\n", window->xid);
 }
@@ -132,6 +138,9 @@ xserver_handle_window_surface(struct wl_client *client,
 		shell_interface->set_transient_xwayland(window->shsurf,
 				window->x + 32, window->y + 32,
 				XWAYLAND_INACTIVE);
+
+	if (window->pending_opaque)
+		set_opaque_override(weston_surface, window->pending_opaque);
 }
 
 const struct xserver_interface xserver_implementation = {
@@ -161,6 +170,19 @@ wm_xwin_handle_set_window(struct wl_client *client,
 }
 
 static void
+set_opaque_override(struct weston_surface *surface,
+		    struct weston_region *region)
+{
+	pixman_region32_copy(&surface->pending.opaque, &region->region);
+	surface->geometry.dirty = 1;
+
+	/* clients must not explicitly destroy the region because it's
+	 * destroyed by the compositor after used. */
+	wl_resource_destroy(&region->resource);
+	region = NULL;
+}
+
+static void
 wm_xwin_handle_opaque_override(struct wl_client *client,
 			       struct wl_resource *resource,
 			       struct wl_resource *region_resource)
@@ -176,11 +198,15 @@ wm_xwin_handle_opaque_override(struct wl_client *client,
 	}
 	region = region_resource->data;
 
-	if (!surface)
+	/* X hasn't sent yet the surface for this particular window.
+	 * Therefore set opaque override region as pending to be consumed
+	 * inside xserver_handle_window_surface */
+	if (!surface) {
+		window->pending_opaque = region;
 		return;
+	}
 
-	pixman_region32_copy(&surface->pending.opaque, &region->region);
-	surface->geometry.dirty = 1;
+	set_opaque_override(surface, region);
 }
 
 static void
@@ -379,6 +405,7 @@ wm_handle_create_xwindow(struct wl_client *client,
 
 	window->surface = NULL;
 	window->shsurf = NULL;
+	window->pending_opaque = NULL;
 	window->wxs = wxs;
 	window->xid = xid;