On Mon, Jul 08, 2013 at 01:51:34PM -0400, Kristian Høgsberg wrote: > On Mon, Jul 08, 2013 at 12:10:54PM +0100, Rob Bradford wrote: > > *bump* > > > > This is a fix for: > > https://bugs.freedesktop.org/show_bug.cgi?id=65726&list_id=320167 > > > > Although it might be much better if we could emit the event > > (especially the leave) before we destroy the surface - that being said > > the destruction of the object will also result in the proxy being > > removed from the map and so when the event is received the callback > > will still have a NULL pointer. > > The problem with the patch is that if we follow this logic, the > otherwise useful allow-null attribute breaks. All object references > can potentially be destroyed between the time the server sends the > event and the time the client receives the event. So I'm more in > favor of changing the order of events so we send out the event before > destroying the wl_resource.
Ooh, and it turns out to be just this commit, which I just pushed: commit 9dadfb53526bc97d62dc01c165e8b6f722f7ea5a Author: Kristian Høgsberg <[email protected]> Date: Mon Jul 8 13:49:36 2013 -0400 compositor: Eliminate marshalling warning for leave events Don't NULL the resource pointer before calling weston_surface_destroy(). We use to have more of a distinction between compositor created surfaces and client surfaces, and weston_surface_destroy couldn't be used for client surfaces. Now it all goes through weston_surface_destroy() and we can remove the assert and the NULL-ing of resource, which caused the marshalling warning. diff --git a/src/compositor.c b/src/compositor.c index a02da8b..92d89a7 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -1002,9 +1002,6 @@ struct weston_frame_callback { WL_EXPORT void weston_surface_destroy(struct weston_surface *surface) { - /* Not a valid way to destroy a client surface */ - assert(surface->resource == NULL); - wl_signal_emit(&surface->destroy_signal, &surface->resource); struct weston_compositor *compositor = surface->compositor; @@ -1053,7 +1050,6 @@ destroy_surface(struct wl_resource *resource) { struct weston_surface *surface = wl_resource_get_user_data(resource); - surface->resource = NULL; weston_surface_destroy(surface); } > Kristian > > > Rob > > > > On 17 June 2013 23:56, Jason Ekstrand <[email protected]> wrote: > > > The current specified behavior does not allow a null surface in either of > > > these events. However, if the client calls wl_surface.destroy while the > > > surface has focus then the leave handler will get a null surface anyway > > > because the proxy corresponding to the wl_surface no longer exists. This > > > change makes this edge-case explicit and allows the server to avoid > > > sending > > > an event with an argument it knows the client has destroyed. > > > > > > Signed-off-by: Jason Ekstrand <[email protected]> > > > --- > > > protocol/wayland.xml | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > > > index 3d4ec9b..faa284e 100644 > > > --- a/protocol/wayland.xml > > > +++ b/protocol/wayland.xml > > > @@ -1313,13 +1313,14 @@ > > > <event name="leave"> > > > <description summary="leave event"> > > > Notification that this seat's pointer is no longer focused on > > > - a certain surface. > > > + a certain surface. The surface parameter may be null if the > > > + surface has been destroyed. > > > > > > The leave notification is sent before the enter notification > > > for the new focus. > > > </description> > > > <arg name="serial" type="uint"/> > > > - <arg name="surface" type="object" interface="wl_surface"/> > > > + <arg name="surface" type="object" interface="wl_surface" > > > allow-null="true"/> > > > </event> > > > > > > <event name="motion"> > > > @@ -1430,13 +1431,14 @@ > > > <event name="leave"> > > > <description summary="leave event"> > > > Notification that this seat's keyboard focus is no longer on > > > - a certain surface. > > > + a certain surface. The surface parameter may be null if the > > > + surface has been destroyed. > > > > > > The leave notification is sent before the enter notification > > > for the new focus. > > > </description> > > > <arg name="serial" type="uint"/> > > > - <arg name="surface" type="object" interface="wl_surface"/> > > > + <arg name="surface" type="object" interface="wl_surface" > > > allow-null="true"/> > > > </event> > > > > > > <enum name="key_state"> > > > -- > > > 1.8.1.4 > > > > > > _______________________________________________ > > > wayland-devel mailing list > > > [email protected] > > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > > _______________________________________________ > > wayland-devel mailing list > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
