[PATCH weston 3/6] libweston-desktop/xwayland: window type XWAYLAND cannot have a parent

[Quentin Glidic]

On 24/11/2016 13:59, Pekka Paalanen wrote:
> On Thu, 24 Nov 2016 12:29:26 +0000
> Daniel Stone <daniel at fooishbar.org> wrote:
>
>> Hi,
>>
>> On 24 November 2016 at 11:40, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>>> Add an assert to ensure that a window of type XWAYLAND is never
>>> attempted with a parent.
>>>
>>> This essentially adding documentation.
>>>
>>> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>>> ---
>>>  libweston-desktop/xwayland.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
>>> index bd68bc6..0c4ff2b 100644
>>> --- a/libweston-desktop/xwayland.c
>>> +++ b/libweston-desktop/xwayland.c
>>> @@ -74,6 +74,7 @@ weston_desktop_xwayland_surface_change_state(struct weston_desktop_xwayland_surf
>>>         bool to_add = (parent == NULL && state != XWAYLAND);
>>>
>>>         assert(state != NONE);
>>> +       assert(!parent || state != XWAYLAND);
>>
>> From looking at the callers, it seems like it should be (!parent ||
>> state == TRANSIENT). I'm a huge fan of adding documentation via
>> asserts though!
>
> Hi,
>
> hm, Quentin said type XWAYLAND windows (mostly just override-redirect)
> must not have a parent, and that's what I wanted to document since
> XWAYLAND is the one with all the special handling.


That is exactly why I don’t like asserts as docs for such cases.

This is all internal API, and it is supposed to be used that way:
- TOPLEVEL, MAXIMIZED and FULLSCREEN are for toplevel windows, that the 
shell will know about
- XWAYLAND is for special parentless windows (mostly O-R, but currently 
we have some false positive matches)
- TRANSIENT is for grabless windows that the shell will never know 
about, and all of them will have a parent *and a position relative to 
this parent*

If a window has a parent and a relative positione, it must be linked to 
this parent. This way, any created view of the parent will get children 
views created too. Theoretically, an XWAYLAND window could have O-R 
children, if they do not have an absolute position, but they would have 
to be TRANSIENT.

Both your asserts are correct, but they do not “document” the same 
thing. :-)

Maybe it was a mistake to reuse as much code as possible here, since 
XWAYLAND is really special.

Cheers,

-- 

Quentin “Sardem FF7” Glidic