On Tue, Jun 19, 2018 at 11:19 AM Markus Ongyerth <[email protected]> wrote: > > Hey Roman, > > first a general remark: > I don't see any mention of which state is supposed to be restored. The > workspace (virtual desktops), geometry (session-restore), just client-side > window (the embedded usecase)? > Is this left implementation-defined intentionally? If so, I think it would be > nice to have a note about that. Should allow restoration of workspace and geometry by compositor and client-side window by client.
> While the sleep version of this has a way for a client ot overwrite and > restore itself, I'm missing that functionality on quit/shutdown. > The current state is always compositor triggered on those. While it might make > sense for e.g. multi-window applications or other fancy things to use a > restoration protocol (at least for window geometry) on their own startup. > It *should* work as is, but I'd like to see it explicitly. The rational was that as soon as the client has agreed to the quit/shutdown state, it should have no business anymore to change this. Because in general it is meant to now quit the client connection and its own process only waiting for the Restore call by the compositor. If there is no process, it can no longer cancel the state. So it's not allowed. A client which will quit itself after some internal process ended should therefore not go into the quit state, otherwise it might get restored at some later point although it has nothing more to do. > On 2018/6月/18 05:05, Roman Gilg wrote: > > * using D-Bus interface only to secure against sandboxed clients > What? Why exactly? When I first read this, I expected that the client is > supposed to use the portal stuff to call out of a sandbox, but reading through > the actual protocol, I think the only usage of D-Bus is to launch the > applications from the compositor? > Since this needs explicit client support either way, IMO this could be > implmented by adding a required cmdline argument (e.g. > --org-freedsktop-restore=UID) instead. > Parsing .desktop files, and launching applications in a helper shouldn't bee > too much of a burdon. > I know that GNOME/KDE employ D-Bus either way, but others want to avoid it as > much as possible. The first draft was written exactly this way: parse the desktop file, execute the Exec line with a special argument. But GNOME people said, that D-Bus Activation should be used. And I think they are right in theory. But if there is a large potion of potential users not willing to support D-Bus Activation one could offer the possibility to execute the Exec line or do D-Bus Activation. Client and compositor just need to make sure that they support whatever method they want to use (and if they don't use at least one of the methods at the same time, don't try to use the protocol). Should be easy to do with another event in the manager interface and another argument to its register_session_suspension call. And personally I would at least recommend in the specs to use D-Bus Activation instead of Exec. > > > > <enum name="error"> > > <entry name="already_exists" value="0" > > summary="an object for the reference already exists"/> > > <entry name="defunct_children" value="1" > > summary="parent object destroyed before child objects"/> > > <entry name="suspension_violation" value="1" > value="2" might be better to differentiate this from defunct_children :) > > summary="unallowed request sent while session suspended"/> > I'd suggest another entry: > <entry name="unsupported_suspension" value="3" > summary="client tried to create a suspesion > object that's not > uspported by the compositor"/> My plan in this case was to just not send the respective event (i.e let the client create the sleep object but not send the sleep event if the compositor does not support sleep). But probably it's nicer to put out an error directly. > > [0] https://specifications.freedesktop.org/desktop-entry-spec > > </description> > > <arg name="id" type="new_id" interface="zxdg_session_suspension" > > summary="register a new restore session"/> > > <arg name="restore_id" type="uint" > > summary="for restoration must match client_id of previous > > session"/> > What if there is no previous session? Then it should be 0. If we decide to use strings instead, it's an empty string. Needs to get mentioned in the description though. > > Calling this request on an xdg_toplevel for which a > > zxdg_session_surface > > object already exists results in a protocol error. > > </description> > > <arg name="id" type="new_id" interface="zxdg_session_surface" > > summary="session surface object"/> > > <arg name="surface" type="object" interface="xdg_toplevel"/> > > </request> > What's your rational to have the surface_restore_id server allocated and sent > as event? This should be in a "client namespace" (i.e. bound to the > protocol-global restore_id) either way, so the client can savely manage them > as well. You are right, that the surface ids could be generated by the client, since they are bound by the restore_id. In this case the compositor would need to make sure though, that the client does not send for different surfaces the same id. Maybe there doesn't need to be sent any id and client and compositor can just agree on counting upwards starting at 1 for every new surface registered? > > <request name="restore_surface"> > > <description summary="ask to restore the surface"> > > Requests the provided xdg_toplevel surface to be restored with the > > data > > the compositor has saved for the surface_restore_id in combination > > with > > the restore_id set earlier in the call to create the > > zxdg_session_suspension object. > > > > If the compositor does not find any data for this restore_id, > > surface_restore_id combination, it will ignore the request without > > further notice. > > > > This request must be sent before the first commit to the > > xdg_toplevel > > and before a register_surface request for the surface has been sent, > > otherwise the request will be ignored by the compositor. > > > > Calling this request while the client is in a suspension state > > results > > in a protocol error. > > > > If the compositor applies the saved settings associated with the > > surface_restore_id it will eventually only send a normal configure > > event to the xdg_toplevel. > > </description> > > <arg name="surface" type="object" interface="xdg_toplevel"/> > > <arg name="surface_restore_id" type="uint" > > summary="must match surface_id from before suspension"/> > I would guess (really no solid data) that clients that restore would like to > register the freshly restored surfaces to upcoming restoration as well, so I'd > create a fresh zxdg_session_surface here as well. I think it's cleaner to do this explicitly in a second call to register_surface. > > <enum name="suspension_level" bitfield="true"> > > <description summary="TODO"> > > Enum bitfield of session suspension levels. Used in the registered > > event to indicate which levels the compositor supports. > > </description> > > <entry name="sleep" value="0"> > > </entry> > > <entry name="quit" value="1"> > > </entry> > > <entry name="shutdown" value="2"> > > </entry> > > </enum> > > > <event name="registered"> > > <description summary="client state being tracked"> > Most protocols that have such an event have a line like "this is emitted on > object creation" and a note about whether it can be emitted at a later point > if some internal state changes. > It should probably also mention if this can be emitted multiple times with > changes supported_levels or changed (non-zero) client_id. Very true. Must be added to the description. It's meant to be submitted only once on object creation. > > <interface name="zxdg_session_sleep" version="1"> > > <description summary="opt-in to client sleep"> > > While this object exists, the client is registered to the server as > > being > > ready to go into the sleep suspension state. > > > > The sleep suspension state is active after the client has received the > > sleep event and untill it receives the wakeup event or it destroys > > this > > object. > > > > If the client registered to a higher level of suspension as well, it > > must > > be ready to go directly from this state into the higher level without > > receiving the wakeup event before and therefore without restoring its > > surfaces in between. The sleep state is implicitly unset in this case. > > </description> > > > > <request name="destroy" type="destructor"> > > <description summary="destroy the sleep object"> > > Used by the client to notify the server that it will no longer use > > this > > object. That means the client can no longer go into the sleep state > > via > > this object. > > > > If the sleep state is active while this request is sent the client > > will > > have the opportunity to recover its suspended surfaces. > I'm a bit unhappy with this. Just naivly looking at it, it forces the > compositor to keep the state, even if the destroy request was sent. > I think something along the lines of > "if a client wants to wake up on its own, it starts the sequence as if woken > up. This will put the xdg_session_sleep object into an inert state, and it > should be destroyed after full restoration. It will no longer be used to send > events and any other request will result in an error" One could say the client is implicitly woken up. But yea, if the sleep object gets destroyed the compositor doesn't know when it can release the surface restoration information. I mean it's not infinite because the compositor does know that it can destroy them on session_suspension object destruction (if the client did not went into quit or shutdown state in between). Maybe another problem is though what happens when the client requests to restore a surface in this case while at the same time the compositor wants to send it into quit, shutdown or (after a new sleep object has been create) in sleep state again. I don't think the sequence "as if woken up" solves this problem. How is this sequence defined and when does it end? When all surfaces are restored? What if the client doesn't do that? Maybe instead there needs to be a separate wakeup_now request that allows the client to get out of sleep state, make the object inert and can restore its surfaces until it destroys the sleep object. This rule should apply to the wakeup event as well. > > <interface name="zxdg_session_quit" version="1"> > > <description summary="opt-in to connection quitting"> > > While this object exists, the client is registered to the server as > > being > > ready to go into the quit suspension state. > > > > The quit state becomes active, when the client responds with an ack > > request on receiving the quit event. The client can cancel going > > into the quit state by sending the destroy request instead of ack. If > > the > > client does not react to the quit event the compositor might deem it > > broken and cancel any suspension by sending the client_id 0 through > > the > > registered event of the zxdg_session_suspension interface. In order to > > make sure that the server did not deem the client broken, the client > > should therefore listen for one more event cycle that this did not > > happen before destroying the zxdg_session_suspension interface. > All of this is a bit weird and conoluted IMO. This protocol requries the > client to use xdg-shell either way, so it's guaranteed that the PING-PONG > mechanism in that protocol is supported by the client. I don't think it's a > good idea to add another way to become unresponsive here. Especially because > there's some weird restrictions on it. If we can reuse the xdg-shell ping-pong, sure let's go for it. > > interface and its child interfaces or a D-Bus signal to ressurect > > before > > that. In this case the quit state is implicitly unset and the client > > is > > restarted only in a new Wayland session and possibly after a reboot of > > the whole system. > I'm a bit confused here. How would the client even get another state change? > IF I understand this correctly, then the client is supposed to shut down > itself, so there's no way for the compositor to send events, right? Correct. The client would never get another state change. One could say it basically gets out of the shutdown/quit state when the compositor dbus-activates it again (but then having a new client connection it's not the same client anymore so the client from before never gets out). _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
