Subject: What we do and don't expose to window management

From: Alan Griffiths <[email-masked]>
Date: Thu, 1 Sep 2016 12:02:42 +0100

Inspired by a discussion with Chris I've been reading our code and
spotted some inconsistencies:

    1. In AbstractShell::modify_surface() we carefully consume the
    streams settings /and apply/ them before giving window management a
    chance to consider the modification request. Contrast this with
    AbstractShell::create_surface() which passes the stream settings to
    window management (which /could/ change them).

    Chris has logged lp:1619165 to say that window management shouldn't
    even see these settings. Hiding them in MirAL is probably enough
    until we incorporate it into Mir.

    But hiding the streams settings isn't the whole of the story: should
    we be applying it when window management rejects the request as ill
    formed?

    2. There's a similar story with confine_pointer - except that in
    this case in AbstractShell::modify_surface() we don't apply the
    setting if window management rejects the whole request by throwing
    an exception.

    It seems odd to be processing this property both in
    CanonicalWindowManagerPolicy::handle_modify_surface() and
    AbstractShell::modify_surface() I would expect it to be either
    handled by the policy or by the shell. Also note that there's no
    checking whether the application/surface is active - so any surface
    of any type (even a non-visible gloss of an application without
    current focus) can confine the cursor at any time.

What should the role of window management policy in handling the streams
and confine_pointer properties?

---

From: Andreas Pokorny <[email-masked]>
Date: Fri, 2 Sep 2016 15:23:56 +0200

Hi,

On Thu, Sep 1, 2016 at 1:02 PM, Alan Griffiths <[email-masked]
> wrote:

> Inspired by a discussion with Chris I've been reading our code and spotted
> some inconsistencies:
>
> 1. In AbstractShell::modify_surface() we carefully consume the streams
> settings *and apply* them before giving window management a chance to
> consider the modification request. Contrast this with
> window management (which *could* change them).
>
> Chris has logged lp:1619165 to say that window management shouldn't even
> see these settings. Hiding them in MirAL is probably enough until we
> incorporate it into Mir.
>
> But hiding the streams settings isn't the whole of the story: should we
> be applying it when window management rejects the request as ill formed?
>
>
Since the streams used by a surface have no further semantics beyond a
function to get to a color for a pixel, changing which streams are part of
a surface is similar to changing the buffer content of a stream already
attached to a surface. So neither window management policy nor shell should
need to have access to streams changes. But that still does not answer the
question - if we consider blocking stream changes because window
mananagement requests were rejected we would also have to block any buffer
changes.

>
> 2. There's a similar story with confine_pointer - except that in this
> case in AbstractShell::modify_surface() we don't apply the setting if
> window management rejects the whole request by throwing an exception.
>
> AbstractShell::modify_surface() I would expect it to be either handled by
> the policy or by the shell. Also note that there's no checking whether the
> application/surface is active - so any surface of any type (even a
> non-visible gloss of an application without current focus) can confine the
> cursor at any time.
>
>
My understanding was that AbstractShell only applies the confinement
setting of a surface if the respective surface is currently focused. Thus
hopefullly deactivates it when focus is changed. It sounds reasonable to
let the WindowManagement Policy decide whether a window may configure
confinement - but cannot think of any reasonable filtering policy, beyond
what is already done in unity8 to allow applications be full screen or not.

regards
Andreas

---

From: Brandon Schaefer <[email-masked]>
Date: Fri, 2 Sep 2016 10:48:40 -0700

Alan is right with the modify surface *could* possibly confine a surface
thats unfocused though this would quickly resolve it self if someone where
to switch windows. The issue is we dont check if the surface we are
modifying in AbstractShell:modify_surface() is focused. Ill get a fix up
for that.

The reason we have two cases of handling: WM + AbstractShell.

the handle_modify_surface:
  Is in charge of setting surface to be in a confined state or not ie. if
its allowed for this surface to be confined.

the modify_surface:
  Checks with WM first about the settings
  If it allows for confine state for the surface and (bug to be fixed) is
focused we need to set seat to confine this region.

Now we *would* be able to avoid this if the WM/Surface could change the
Seat but it sounds better to keep it in the abstract shell. The only other
way is to change the surface observer to say we allow confinement for
x,y,w,h now. (which we did and reverted since the observer should state
that something has changed not that it *should* change something now)

On Fri, Sep 2, 2016 at 6:23 AM, Andreas Pokorny <
[email-masked]> wrote:

> Hi,
>
>
> On Thu, Sep 1, 2016 at 1:02 PM, Alan Griffiths <
> [email-masked]> wrote:
>
>> AbstractShell::modify_surface() I would expect it to be either handled
>> by the policy or by the shell. Also note that there's no checking whether
>> the application/surface is active - so any surface of any type (even a
> --
> Mir-devel mailing list
> [email-masked]
> Modify settings or unsubscribe at: https://lists.ubuntu.com/
> mailman/listinfo/mir-devel
>
>