Created attachment 151292 [details] [review]
Attaching patch again

Comment on attachment 151292 [details] [review]
Attaching patch again

>+  if (global_client) {
>+    g_object_unref (global_client);
>+    global_client = NULL;
>+  }
>+
>+  egg_sm_client_set_mode (mode);
>+  client = egg_sm_client_get ();

That's kind of gross. It means that if you (or someone else, eg, the theoretical GtkApplication object) had previously called egg_sm_client_get(), then (a) if you kept a pointer to it, that pointer now points to a freed object, and (b) if you connected to the signals on it, then your signal handlers won't ever actually get called, because you'd connected to the signal handlers on the dummy client, not the real client.

It would be better to create the right EggSMClient right away, but tell it to delay registering. The code used to always delay registering until mainloop time, but that causes problems for apps that aren't autostarted, because they won't have an SM client id until after they finish registering, but you need to call gdk_set_sm_client_id() before creating any windows. So the code got changed. But it could be changed back to delaying registration if it already had a client ID (and the app asked it to).

Created attachment 151295 [details] [review]
Alternative approach

(bah, the browser ate my comment)

You're right, of course. What about this approach?

We detect if a startup is needed in egg_sm_client_set_mode(). And we also make sure we cannot go back to DISABLED if it's too late with some g_return_if_fail() check.

Comment on attachment 151295 [details] [review]
Alternative approach

i didn't look too carefully, but yeah, that seems like it should work

It definitely works with the panel :-) Thanks!