[PATCH] Allow null surface arguments in wl_pointer.leave and wl_keyboard.leave

[Kristian Høgsberg]

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 <krh at bitplanet.net>
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 <jason at jlekstrand.net> 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 <jason at jlekstrand.net>
> > > ---
> > >  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
> > > wayland-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel