Created attachment 306880 [details] [review]
gapplication: terrible launching hacks

Created attachment 306881 [details] [review]
gtkapplication: terrible launching hacks

Owen / Ryan / Alex: thoughts?

Here is how this should work:

app "A" spawns app "B" using dbus activation as per: http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s07.html

1) A sets the desktop-startup-id field in the platform_data argument in the Open call based on its  _NET_WM_USER_TIME of the launching window
2) B takes this and sets it as the  _NET_WM_USER_TIME on the new window

app "A" spawns "B" with fork+exec

3) "A" creates a startup notification operation by sending x messages to the WM. These involve specifying the last timestamp of "A".
4) A sets DESKTOP_STARTUP_ID in the forked process to the name of the startup nofication thing created in 3, it was historically a pure unique id, but at some point it change to be of the form "<whatever>_TIME<timestamp>" thus also encoding the timestamp to use for the child.
5) "B" takes the timestamp from the DESKTOP_STARTUP_ID an applies it to _NET_WM_USER_TIME on the new window. It also sets _NET_STARTUP_ID to the full startup id. If the _TIMExxx prefix was not there, the WM itself is supposed to know from the _NET_STARTUP_ID and the TIME set in 3 the time to use for focus-stealing prevention.

If we fork+exec a GtkApplication app we will get *both* of these, as the initiating app will do 3-5 and then the forked app will do 1-2, propagating the _TIME prefix into the platform_data. This breaks if _TIME prefix is not there, as it fails to propagate the _NET_STARTUP_ID.

So, how is this traditionally done in glib/gtk? 
3-4: Spawning is done via GDesktopAppInfo, which takes a GAppLaunchContext, which gets the current time in gdk_x11_app_launch_context_get_startup_notify_id() and is later set as an env var in g_desktop_app_info_launch_uris_with_spawn()
5: gdk_x11_display_make_default() calls gdk_x11_display_set_startup_notification_id() with the DESKTOP_STARTUP_ID env var which is then unset. This parses the _TIME prefix to set latest user time, as well as setting _NET_STARTUP_ID on the first mapped window.

Similarily, with GtkApplication use via DBusActivate=true in desktop file

1: 
  GApplaunchContext (via X11 impl) sends X messages and creates startup id with time prefix.
  g_desktop_app_info_launch_uris_with_dbus() builds the platform data from the above launch context startup id and passes it to the target
2: 
   gtk_application_impl_x11_before_emit() gets the platform data and sets the last user timestamp and startup id for next window
   emit signal and open window
   gtk_application_after_emit calls gdk_notify_startup_complete() to ensure it was called (even if we didn't show a window i guess?)

In the fork+exec converted to dbus call case we transfer the env var to platform_data in gtk_application_add_platform_data()   

Now, there are many cases where this breaks. Here are a list of some:

* the env var or platform_data is never set, because the app was not spawned correctly (typically it was started from the terminal)
* Wayland has no implementation of any of these in glib/gtk+, as well as not really having a global time concept.
* Spawning app did not create a GdkAppLaunchContext, so we have to way to plumb in the X11 user time.
* App used the old protocol version which did not prefix the id with _TIME...

In these cases I guess the best approach is that as early as possible when we detect this case try to get a global timestamp from the Xserver and use that instead. That way any user interactions on other windows after this will be protected from focus stealing. Not sure where this could easiest be plumbed though.

I have no idea what to do on wayland though.

Perhaps the easiest place to do this would be to special-case g_app_info_launch for a NULL launch context meaning "look for DESKTOP_STARTUP_ID if it exists in the environment and has a _TIME prefix", with a fallback to "fetch the most timestamp from the server".

This would fix gvfs-open and xdg-open. It wouldn't fix launching the app manually on the terminal, though.

There are two solutions here.  One is the good one and one is the bad one.

First, the bad one:

If xdg-open cannot figure out a timestamp, it needs to connect to the X server, create an input-only window, watch PropertyNotify and then change a property on that window.  Property change notifications come with a timestamp, so now you have a valid timestamp.  startup-notification should then be used with that timestamp (and whether D-Bus or fork/exec is used is not really very interesting).

This is cheating and, by definition, it will do the wrong thing sometimes.

The good one:

Fix Firefox, Chrome, etc.  This simply must be done, and saying "we can't do it" doesn't change that.

While we're at it, we should fix gnome-terminal to forward the timestamp associated with the [Enter] keypress that resulted in an application being launched from the commandline.

Created attachment 307009 [details] [review]
GtkApplication: avoid using stale timestamps

Avoid using a stale timestamp (from the last user interaction with the
application) when a message arrives from D-Bus requesting that a new
window be created.

In this case the most-correct thing that we can do is to use no
timestamp at all.

We modify gdk_x11_display_set_startup_notification_id() to allow a NULL
value to mean "reset everything" and then call this function
unconditionally on receipt of D-Bus activation requests.  The result
will be that a missing desktop-startup-id in the platform-data struct
will reset the timestamp.

Under their default configuration metacity and mutter will both map
windows presented with no timestamp in the foreground.  This could
result in false-positive, but there is very little we can do about that
without the original timestamp from the user event.

Test:

In terminal:

 $ gedit &

   type some stuff into the gedit window

 $ sleep 5; gedit

   switch to a non-terminal window and interact with it (to avoid the WM's special treatment of terminals)



Without the patch, gedit doesn't get raised.  After the patch, it raises.

This doesn't fix the issue, however.  gedit may be being raised inappropriately in this situation, and the only real fix is to ensure that the timestamp information is forwarded correctly.

Review of attachment 307009 [details] [review]:

There is a teeny tiny race condition where if you interacted with the app, and its doing something slow which will eventually pop up a window. If inbetween you get a request to open a new window then both the new windows will get no timestamp. However, this is not a critical problem, plus it is very unlikely, so this seems like a good approach to me.

Attachment 307009 [details] pushed as a00a5ed - GtkApplication: avoid using stale timestamps