[PATCH wayland-protocols] xdg-shell: Add startup notification

[Carlos Garnacho]

Hey :),

On Mon, Feb 15, 2016 at 9:59 AM, Jonas Ådahl <jadahl at gmail.com> wrote:
> On Thu, Feb 11, 2016 at 01:52:36PM +0100, Carlos Garnacho wrote:
>> The xdg_launcher interface is added for the launcher, it's used
>> to notify of the startup ID to be transmitted to the launchee,
>> plus notifications about the startup success/failure.
>>
>> On the launchee side, we now have xdg_shell.set_startup_id,
>> which will notify the compositor of startup finalization.
>>
>> This has been made to be compatible with the XDG Startup
>> Notification spec available for X11, the startup ID is
>> transmitted from the launcher to the launchee in the same
>> ways, so we can launch x11 from wayland applications and
>> viceversa. The notable difference is that wayland launchers
>> receive startup IDs that are guaranteed to be unique, whereas
>> in X11 this is a best effort of the launcher client.
>>
>> Some notes have also been added about focus stealing prevention,
>> although that's mostly up for compositors to implement.
>>
>> Signed-off-by: Carlos Garnacho <carlosg at gnome.org>
>> ---
>>
>> I've got no full implementations yet, so this is mostly an RFC at the
>> moment. I mainly wonder, should we add a "serial" argument to the
>> create_launcher request? that'd at least ensure the launcher application
>> has some sort of focus, although nothing prevents an application from
>> being a fork bomb otherwise.
>
> Hey,
>
> I assume the compositor would have to limit to one startup per event
> or something like that if you add the serial? Doesn't seem to prevent
> fork bombs anyhow, the client can still fork as much as it wants. What
> it does limit is, say, opening an application as a response to not-user
> interaction. I don't have any reasonable use cases except remote
> controlled clients not being able to do this properly.

Right, I guessed that'd prevent processes from being "fake startup"
bombs, but that seems somewhat light compared to the real thing :).

>
> Overall, I'd like to see this being added as a separate extension. The
> reason is that I don't think this belongs in a "core" xdg shell
> interface, which we should try to keep as minimal as reasonable. It
> could for example be a "xdg_startup_notification" global (well,
> zxdg_startup_notification_v1 until later), which contains the requests
> you added to xdg_shell.

Yeah, probably makes sense to have this as a separate protocol. I
guess it's safe to keep using xdg_surface in arguments, and rely that
this protocol shall be used together with xdg-shell?

>
>>
>>  unstable/xdg-shell/xdg-shell-unstable-v5.xml | 71 +++++++++++++++++++++++++++-
>>  1 file changed, 70 insertions(+), 1 deletion(-)
>>
>> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v5.xml b/unstable/xdg-shell/xdg-shell-unstable-v5.xml
>> index 542491f..1c4ef54 100644
>> --- a/unstable/xdg-shell/xdg-shell-unstable-v5.xml
>> +++ b/unstable/xdg-shell/xdg-shell-unstable-v5.xml
>> @@ -27,7 +27,7 @@
>>      DEALINGS IN THE SOFTWARE.
>>    </copyright>
>>
>> -  <interface name="xdg_shell" version="1">
>> +  <interface name="xdg_shell" version="2">
>>      <description summary="create desktop-style surfaces">
>>        xdg_shell allows clients to turn a wl_surface into a "real window"
>>        which can be dragged, resized, stacked, and moved around by the
>> @@ -135,6 +135,35 @@
>>        </description>
>>        <arg name="serial" type="uint" summary="serial of the ping event"/>
>>      </request>
>> +
>> +    <!-- version 2 additions -->
>> +    <request name="create_launcher" since="2">
>> +      <description summary="create a new launcher">
>> +        Creates a new launcher context.
>> +
>> +        The surface argument is the toplevel where the application
>> +        was launched from, compositors may want to place the launched
>> +        application relative to the launcher surface.
>> +
>> +        Compositors that desire to implement focus stealing prevention
>> +        can mark the time this request is received as the "startup" time.
>
> Not sure paragraph this belongs here. Compositors may do more things,
> and doesn't seem to be useful to list those things here. Maybe it would
> be good to add some high level blurb about how focus stealing prevention
> could be done in some generic place (for example <description> in
> <protocol> if it's its own extension protocol).

Right.

>
>> +      </description>
>> +      <arg name="id" type="new_id" interface="xdg_launcher"/>
>> +      <arg name="surface" type="object" interface="xdg_surface"/>
>> +    </request>
>> +
>> +    <request name="set_startup_id" since="2">
>> +      <description summary="set the application startup_id">
>> +        Notifies the compositor of the startup ID of this launched application.
>> +        Applications will typically receive this through the DESKTOP_STARTUP_ID
>> +        environment variable as set by its launcher, and should unset the
>> +        environment variable right after this request, in order to avoid
>> +        propagating it to child processes.
>> +
>> +        Compositors will ignore unknown startup IDs.
>> +      </description>
>> +      <arg name="startup_id" type="string"/>
>> +    </request>
>
> How does this work when the application was already running? For example
> if the launcher opened gedit with a new file, but gedit was already, how
> is it communicated that the launched application was actually already
> launched? Does gedit need to communicate internally and call this
> request again? Or does the "hey gedit, wake-up!" process need to make an
> additional Wayland connection and call this?

This is all implementation dependent. IIRC what happens in the gtk+
world is that the startup ID is transferred through dbus to the
main/running process. So there's indeed scenarios where this may be
called several times from a same client, and even meaning to focus the
same surface.

>
>>    </interface>
>>
>>    <interface name="xdg_surface" version="1">
>> @@ -622,4 +651,44 @@
>>      </event>
>>
>>    </interface>
>> +
>> +  <interface name="xdg_launcher" version="2">
>> +    <description summary="context for launching applications">
>> +      xdg_launcher allows clients to get the necessary context to launch
>> +      applications, so the compositor can provide feedback about the
>> +      application being launched.
>> +    </description>
>> +
>> +    <request name="destroy" type="destructor">
>> +      <description summary="destroy xdg_launcher">
>> +        Destroys this xdg_launcher object.
>> +      </description>
>> +    </request>
>> +
>> +    <event name="startup_id" since="2">
>> +      <description summary="startup ID for the launched application">
>> +        Notifies of an unique startup_id (eg. UUIDs) to be used for the
>> +        application about to be launched.
>> +
>> +        In order to guarantee interoperation with the XDG Startup Notification
>> +        spec, this startup_id is recommended to be transmitted to the launched
>> +        application through the DESKTOP_STARTUP_ID environment variable.
>
> It is unclear when this event will be received.
>
> I assume the flow of the client is:
>
> 1. client decides it wants to start application X
> 2. <- xdg_shell.create_launcher
> 3. -> xdg_launcher.startup_id("XYZ123")
> 4. fork(); setenv("DESKTOP_STARTUP_ID", "XYZ123"); exec("/path/to/Application X");
>
> ... either some timeout(?) or the new appication called
>     xdg_shell.set_startup_id("XYZ123")
>
> 5. <- xdg_launcher.done / xdg_launcher.cancelled
>
> This flow should be spelled out semewhere (or the correct flow if I got
> it wrong). By adding it as a separate extension, a good place would be
> the <description> inside the <protocol>.

Right. That was indeed the behavior intended.

Cheers,
  Carlos