After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 694876 - we register with session manager too earlier
we register with session manager too earlier
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2013-02-28 15:54 UTC by Ray Strode [halfline]
Modified: 2013-03-01 16:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
core: make session registration an explicit step (7.01 KB, patch)
2013-02-28 15:54 UTC, Ray Strode [halfline]
reviewed Details | Review
main: register with session manager explicitly (2.38 KB, patch)
2013-02-28 15:55 UTC, Ray Strode [halfline]
committed Details | Review
core: make session registration an explicit step (7.48 KB, patch)
2013-02-28 18:15 UTC, Ray Strode [halfline]
committed Details | Review

Description Ray Strode [halfline] 2013-02-28 15:54:19 UTC
gnome-session waits until the shell registers with the session manager before continuing startup.

This synchronization point is important because the things that are started after the shell depend on the shell being fully functional.

Unfortunately, we tell the session manager too early that we're ready. mutter does it implicitly as soon as we start the main loop. For instance, in classic mode, nautilus is started before we've initialized the workspace layout causing weird behavior.

We either need to move all "important" initialization to before we go into the main loop, or we need to explicitly register with the session manager with a new mutter api.
Comment 1 Ray Strode [halfline] 2013-02-28 15:54:49 UTC
Created attachment 237618 [details] [review]
core: make session registration an explicit step

gnome-shell shouldn't announce to the session manager it's
"ready" until it's fully initialized.  It currently tells
the session manager it's ready as soon as it hits the main
loop. This causes nautilus in classic mode to start before
we have workspaces initialized.
Comment 2 Ray Strode [halfline] 2013-02-28 15:55:03 UTC
Created attachment 237619 [details] [review]
main: register with session manager explicitly

Mutter now makes session registration an explicit required
step.  This is so we can tell the session manager when
we're ready to move on to the next phase.

This commit calls the new Meta.register() api after we're
initialized.
Comment 3 Giovanni Campagna 2013-02-28 17:33:40 UTC
Review of attachment 237618 [details] [review]:

::: src/core/main.c
@@ +449,2 @@
 /**
+ * meta_register:

Can we call this meta_register_session (or another non ambiguous name)?

@@ -512,3 @@
-  
-  /* Connect to SM as late as possible - but before managing display,
-   * or we might try to manage a window before we have the session

^ This comment.
We must load the session info before we start managing windows, so it must be decoupled from session registration.
Comment 4 Giovanni Campagna 2013-02-28 17:35:41 UTC
Review of attachment 237619 [details] [review]:

Ok.
Comment 5 Ray Strode [halfline] 2013-02-28 18:04:53 UTC
(In reply to comment #3)
> Review of attachment 237618 [details] [review]:
> 
> ::: src/core/main.c
> @@ +449,2 @@
>  /**
> + * meta_register:
> 
> Can we call this meta_register_session (or another non ambiguous name)?
sure.

> @@ -512,3 @@
> -  
> -  /* Connect to SM as late as possible - but before managing display,
> -   * or we might try to manage a window before we have the session
> 
> ^ This comment.
> We must load the session info before we start managing windows, so it must be
> decoupled from session registration.
don't grok what you're saying here
Comment 6 Ray Strode [halfline] 2013-02-28 18:15:14 UTC
Created attachment 237641 [details] [review]
core: make session registration an explicit step

gnome-shell shouldn't announce to the session manager it's
"ready" until it's fully initialized.  It currently tells
the session manager it's ready as soon as it hits the main
loop. This causes nautilus in classic mode to start before
we have workspaces initialized.
Comment 7 Giovanni Campagna 2013-02-28 18:17:09 UTC
I'm saying that meta_session_init() loads the session state, which is needed before we start managing windows (otherwise they don't get matched correctly, and their position and state is not restored). So you need to split meta_session_init() in a part that loads the state and a part that connects to gnome-session.

Arguably, we could ditch the whole of XSMP, require client-side saving and move to DBus...
Comment 8 Ray Strode [halfline] 2013-02-28 18:19:09 UTC
we're not displaying any windows until this phase of startup is over
Comment 9 Jasper St. Pierre (not reading bugmail) 2013-02-28 18:19:27 UTC
Well, we could ditch the XML session saving for a start, without ditching XSMP entirely.
Comment 10 Ray Strode [halfline] 2013-02-28 18:21:58 UTC
we can't ditch XSMP (including the half of the protocol responsibility the window manager is in charge of) without laying the stage for a replacement, which is a big task and separate from this bug.
Comment 11 Florian Müllner 2013-02-28 18:24:43 UTC
(In reply to comment #10)
> we can't ditch XSMP [...] without laying the stage for a replacement,
> which is [...] separate from this bug.

Not to mention completely out of question for 3.8 ...
Comment 12 Ray Strode [halfline] 2013-02-28 18:50:28 UTC
Okay, i'm probably going to push this soon if there are no unaddressed concerns
Comment 13 Ray Strode [halfline] 2013-03-01 16:32:02 UTC
Comment on attachment 237641 [details] [review]
core: make session registration an explicit step

Attachment 237641 [details] pushed as 773ae8d - core: make session registration an explicit step
Comment 14 Ray Strode [halfline] 2013-03-01 16:32:16 UTC
Attachment 237619 [details] pushed as 57eae1b - main: register with session manager explicitly