Re: [PATCH weston 17/18] xwayland: Fix the race condition when mapping a surface

[Louis-Francis Ratté-Boulianne]

 Hi,

On Monday, June 10, 2013 14:11 EDT, "MoD" <mod-oss at hush.ai> wrote: 
 
> I've been poking at XWayland+Weston's XWM recently and wanted to find this
> behavior and check that this patch is effective so I could vouch for it, but
> I haven't seen something that looks like windows being mapped with the wrong
> coordinates or size. Louis-Francis, could you explain how to reproduce this
> behavior?

I reproduced it by running LibreOffice on a RaspberryPi. A race condition made the menus (sometimes) appears at the wrong spot. The race might be easier to reproduce on slowest hardware (rpi) though.

I also found it weird to map the surface without being sure we received MAP_NOTIFY first.

> Thanks.
> 
> On 22 de mayo de 2013 at 10:27 AM, ppaalanen at gmail.com wrote:
> >
> >From: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
> >
> >Make sure XCB_MAP_NOTIFY has been received and the window id has 
> >been
> >set before mapping the shell surface. It fixes race condition 
> >making the
> >surface appears at wrong coordinates or with wrong size.
> >---
> > src/xwayland/window-manager.c | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> >diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-
> >manager.c
> >index 366f2e0..c2e680b 100644
> >--- a/src/xwayland/window-manager.c
> >+++ b/src/xwayland/window-manager.c
> >@@ -113,6 +113,7 @@ struct weston_wm_window {
> > 	int decorate;
> > 	int override_redirect;
> > 	int fullscreen;
> >+	int mapped;
> > };
> > 
> > static struct weston_wm_window *
> >@@ -121,6 +122,9 @@ get_wm_window(struct weston_surface *surface);
> > static void
> > weston_wm_window_schedule_repaint(struct weston_wm_window 
> >*window);
> > 
> >+static void
> >+xserver_map_shell_surface(struct weston_wm *wm, struct 
> >weston_wm_window *window);
> >+
> > const char *
> > get_atom_name(xcb_connection_t *c, xcb_atom_t atom)
> > {
> >@@ -723,6 +727,14 @@ static void
> > weston_wm_handle_map_notify(struct weston_wm *wm, 
> >xcb_generic_event_t *event)
> > {
> > 	xcb_map_notify_event_t *map_notify = (xcb_map_notify_event_t *) 
> >event;
> >+	struct weston_wm_window *window;
> >+
> >+	window = hash_table_lookup(wm->window_hash, map_notify->window);
> >+
> >+	if (window->surface != NULL)
> >+		xserver_map_shell_surface(wm, window);
> >+	else
> >+		window->mapped = 1;
> > 
> > 	if (our_resource(wm, map_notify->window)) {
> > 			weston_log("XCB_MAP_NOTIFY (window %d, ours)\n",
> >@@ -915,6 +927,8 @@ weston_wm_window_create(struct weston_wm *wm,
> > 	window->override_redirect = override;
> > 	window->width = width;
> > 	window->height = height;
> >+	window->surface = NULL;
> >+	window->mapped = 0;
> > 
> > 	hash_table_insert(wm->window_hash, id, window);
> > }
> >@@ -1874,7 +1888,9 @@ xserver_set_window_id(struct wl_client 
> >*client, struct wl_resource *resource,
> > 		      &window->surface_destroy_listener);
> > 
> > 	weston_wm_window_schedule_repaint(window);
> >-	xserver_map_shell_surface(wm, window);
> >+
> >+	if (window->mapped)
> >+		xserver_map_shell_surface(wm, window);
> > }
> > 
> > const struct xserver_interface xserver_implementation = {
> >-- 
> >1.8.1.5
> >
> >_______________________________________________
> >wayland-devel mailing list
> >wayland-devel at lists.freedesktop.org
> >http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>