[PATCH wayland] protocol: Add a surface_has_existing_role error

[Pekka Paalanen]

On Fri, 22 Aug 2014 10:48:02 -0700
Jason Ekstrand <jason at jlekstrand.net> wrote:

> As I mentioned on IRC, I don't really like adding and using an enum value
> without bumping protocol version.  However, the only time we ever use it is
> to kill the client so the worst thing that can happen is that the client
> doesn't report the error correctly.  I think I'm ok with it.

Yeah, I don't see a problem in adding values to an error enum.

However, I do wonder if this is logical.

Let's say xdg_shell used this. A client issues request
xdg_shell at 33.get_xdg_surface(wl_surface at 20). There are quite likely
several other requests on wl_surface at 20, and since I assume
get_xdg_surface will be raising a protocol error, there will also be an
earlier request on some other interface already giving a role to
wl_surface at 20. And all these requests can be in the same burst.

Using the below enum value, the compositor would send the error on
wl_surface at 20, but you don't know which interface call caused it.

Using an error enum value in xdg_shell, the compositor would be sending
the error on xdg_shell at 33, but you don't know for which surface.

One solution to both "don't know"s is to add the missing information to
the error message for humans to see.

Both ways could work in practice, but this would be the first (legal)
case of sending the error on a different object than whose request
caused it to trigger.

It may make sense in this one case, but if we do it here, we generally
allow it everywhere, which I believe can bring confusion on what errors
can be raised where. It will no longer be true, that the possible
errors are the ones defined in the interface containing the request,
plus wl_display errors. You will be also adding all errors on all
argument objects to the set, per request.

That is why I am hesitant to commit this. I'm sure someone will find
a case where this would make something like writing language bindings
more difficult for him, even if it seems obscure or even silly to us.

It's not like we share e.g. pixel format enums between, say, wl_shm and
wl_drm, even though they are the same AFAIK, and there is a
considerable amount of duplication. (Yes, that is a bit different case,
because there we would be breaking also the namespacing.)


Thanks,
pq

> On Fri, Aug 22, 2014 at 10:37 AM, Jasper St. Pierre <jstpierre at mecheye.net>
> wrote:
> 
> > This will be used by implementations to send out errors if clients try
> > to set roles on surfaces that already have roles.
> > ---
> >  protocol/wayland.xml | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > index bb457bc..f6eb54d 100644
> > --- a/protocol/wayland.xml
> > +++ b/protocol/wayland.xml
> > @@ -983,6 +983,7 @@
> >        </description>
> >        <entry name="invalid_scale" value="0" summary="buffer scale value
> > is invalid"/>
> >        <entry name="invalid_transform" value="1" summary="buffer transform
> > value is invalid"/>
> > +      <entry name="surface_has_existing_role" value="2" summary="the
> > surface passed already has a role"/>
> >      </enum>