[PATCH] xdg-shell: Align surface life-time rules with that of wl_shell

[Jason Ekstrand]

On Mon, Jul 21, 2014 at 1:47 AM, Sjoerd Simons <
sjoerd.simons at collabora.co.uk> wrote:

> Remove the explicit destroy method from xdg_surface and xdg_popup as
> neither of them can be re-created after being destroyed. As a results a
> wl_surface gets into an odd sort of odd limbo state after a
> xdg_{surface,popup}.destroy call where it not only no longer has an
> xdg_{surface,popup} associated with it but where it's also not possible
> to re-created one.
>
> This also happens to align the life-time rules of xdg_{surface,popup} to
> match those of wl_shell_surface, which is probably good for consistency.
>

Sjoerd,
First of all, making xdg_shell consistent with wl_shell is a non-reason for
doing anything.  The objective to xdg_shell is to *replace* wl_shell so
they are going to have different symantics.

Second, regarding your original patch.  I'm sorry that it never got
reviewed, but implementation details of desktop-shell are, again, not a
good driving force for xdg_shell.  If weston's desktop-shell has
wl_shell_surface symantics too closely tied to xdg_shell, that's a bug in
weston's desktop-shell, not a bug in xdg_shell.  Right now, weston's
desktop-shell is kind of a mess and, yes, it makes a lot of assumptions
about the lifecycles of objects both internal and external.  When I did the
surface/view split, it was a pain because it made the tacit assumption that
a shell surface was a surface was a place on-screen.  Separating these
assumptions will make desktop-shell much cleaner.

Third is the question of whether we want destructors or not.  One of the
things we have learned is that destructors are very hard to properly add
later.  If there is any chance that we will need them, we have to add them
before we stabilize the protocol.  Also, I don't think there's any real
harm except for the possible case where the xdg_surface is hanging around
after the wl_surface is destroyed.  I'd be 100% ok with defining any
requests on the xdg_surface to result in a protocol error if the wl_surface
is destroyed.  It's possible that we could remove the destructor and tie
the two lifecycles together.  However, we need to be *really* careful about
doing so.

--Jason Ekstrand


>
> Signed-off-by: Sjoerd Simons <sjoerd.simons at collabora.co.uk>
> ---
>  desktop-shell/shell.c  | 25 ++-----------------------
>  protocol/xdg-shell.xml | 24 ++++--------------------
>  2 files changed, 6 insertions(+), 43 deletions(-)
>
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index 593c7f3..efec100 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -3370,13 +3370,6 @@ static const struct wl_shell_interface
> shell_implementation = {
>   * xdg-shell implementation */
>
>  static void
> -xdg_surface_destroy(struct wl_client *client,
> -                   struct wl_resource *resource)
> -{
> -       wl_resource_destroy(resource);
> -}
> -
> -static void
>  xdg_surface_set_parent(struct wl_client *client,
>                        struct wl_resource *resource,
>                        struct wl_resource *parent_resource)
> @@ -3534,7 +3527,6 @@ xdg_surface_set_minimized(struct wl_client *client,
>  }
>
>  static const struct xdg_surface_interface xdg_surface_implementation = {
> -       xdg_surface_destroy,
>         xdg_surface_set_parent,
>         xdg_surface_set_title,
>         xdg_surface_set_app_id,
> @@ -3662,18 +3654,6 @@ shell_surface_is_xdg_surface(struct shell_surface
> *shsurf)
>  }
>
>  /* xdg-popup implementation */
> -
> -static void
> -xdg_popup_destroy(struct wl_client *client,
> -                 struct wl_resource *resource)
> -{
> -       wl_resource_destroy(resource);
> -}
> -
> -static const struct xdg_popup_interface xdg_popup_implementation = {
> -       xdg_popup_destroy,
> -};
> -
>  static void
>  xdg_popup_send_configure(struct weston_surface *surface,
>                          int32_t width, int32_t height)
> @@ -3753,8 +3733,7 @@ xdg_get_xdg_popup(struct wl_client *client,
>                 wl_resource_create(client,
>                                    &xdg_popup_interface, 1, id);
>         wl_resource_set_implementation(shsurf->resource,
> -                                      &xdg_popup_implementation,
> -                                      shsurf,
> shell_destroy_shell_surface);
> +                                      NULL, shsurf,
> shell_destroy_shell_surface);
>  }
>
>  static void
> @@ -3771,7 +3750,7 @@ shell_surface_is_xdg_popup(struct shell_surface
> *shsurf)
>  {
>         return wl_resource_instance_of(shsurf->resource,
>                                        &xdg_popup_interface,
> -                                      &xdg_popup_implementation);
> +                                      NULL);
>  }
>
>  static const struct xdg_shell_interface xdg_implementation = {
> diff --git a/protocol/xdg-shell.xml b/protocol/xdg-shell.xml
> index bd36231..ea005bc 100644
> --- a/protocol/xdg-shell.xml
> +++ b/protocol/xdg-shell.xml
> @@ -127,16 +127,6 @@
>        the wl_surface object.
>      </description>
>
> -    <request name="destroy" type="destructor">
> -      <description summary="remove xdg_surface interface">
> -       The xdg_surface interface is removed from the wl_surface object
> -       that was turned into a xdg_surface with
> -       xdg_shell.get_xdg_surface request. The xdg_surface properties,
> -       like maximized and fullscreen, are lost. The wl_surface loses
> -       its role as a xdg_surface. The wl_surface is unmapped.
> -      </description>
> -    </request>
> -
>      <request name="set_parent">
>        <description summary="surface is a child of another surface">
>         Child surfaces are stacked above their parents, and will be
> @@ -388,17 +378,11 @@
>        parent surface, in surface local coordinates.
>
>        xdg_popup surfaces are always transient for another surface.
> -    </description>
>
> -    <request name="destroy" type="destructor">
> -      <description summary="remove xdg_surface interface">
> -       The xdg_surface interface is removed from the wl_surface object
> -       that was turned into a xdg_surface with
> -       xdg_shell.get_xdg_surface request. The xdg_surface properties,
> -       like maximized and fullscreen, are lost. The wl_surface loses
> -       its role as a xdg_surface. The wl_surface is unmapped.
> -      </description>
> -    </request>
> +      On the server side the object is automatically destroyed when
> +      the related wl_surface is destroyed. On client side,
> xdg_popup.destroy()
> +      must be called before destroying the wl_surface object.
> +    </description>
>
>      <event name="popup_done">
>        <description summary="popup interaction is done">
> --
> 2.0.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140721/7deb8f88/attachment-0001.html>