1603089 - sessionstore-windows-restored is overloaded to mean "late on startup" (and used by all and sundry to that effect) but doesn't fire for non-browser Firefox startups (e.g. FXR, Site-Specific-Browsers)

Status: RESOLVED WONTFIX

(Firefox :: General, task, P3)

Description

[:Gijs (he/him)]

We worked around the extension part of this for FXR in bug 1599846 because it was urgent, but I think the problem is wider than that, and we should come up with a more generic solution.

We rely (in BrowserGlue.jsm and various other places) on this notification as being an indicator of when all the critical parts of startup have finished, and initialize services / code that should really (also) be run if we're starting up in FXR mode or with a site-specific browser.

The best I can come up with is adding a level of indirection, e.g.:

1. create a LateStartupHandler in BrowserGlue.jsm
2. make it listen for sessionstore-windows-restored
3. make it fire some other notification (maybe browser-startup-complete or whatever) and set a publicly accessible flag on itself so people can check whether the notification has fired or not.
4. switch consumers of sessionstore-windows-restored to the new notification (and make them check the flag before adding a listener)
5. for FXR and SSB, add separate notifications (fxr-started, ssb-started) and make the LateStartupHandler listen for those, too, again dispatching the browser-startup-complete thing unless that already happened.

Does that sound sensible or am I missing an edgecase?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Dave Townsend [:mossop]]

So this notification is currently used for two different things (perhaps three?).

1. Startup is complete (all windows that are going to open at startup are open).
2. All browsing windows that are going to be shown on startup are now open.

So we're going to have to separate out the listeners into those two camps. 2 is mostly used by code that needs to add things to the browsing UI, notification bars etc. That tends to happen in response to a specific startup so we're going to have to be careful that if 2 never happens, those consumers still do their thing on the next startup. That may be complicated depending on the thing.

Your proposal makes sense for 1. It might make sense to include some flag to say the "kind" of startup, browser, ssb, fxr etc so callers can know whether browser windows are expected to open during startup or not. Not sure if we need to add multiple observer notifications or if ssb/fxr should just call LateStartupHandler directly to tell it that they're done.

Comment 3

[Tomislav Jovanovic :zombie]

(In reply to :Gijs (he/him) from comment #0)

3. make it fire some other notification (maybe browser-startup-complete or whatever) and set a publicly accessible flag on itself so people can check whether the notification has fired or not.
4. switch consumers of sessionstore-windows-restored to the new notification (and make them check the flag before adding a listener)

If we're already planning a public flag, could it be simpler to make it a public BrowserStartupComplete promise?

Comment 4

[Dave Townsend [:mossop]]

I was thinking a little about this this morning and I agree that a promise could be good here, but I was also thinking that an approach similar to AsyncShutdown might be helpful. Essentially things that happen during startup that would block the notion of startup being complete could register a promise to say when they're done. Taken to the extreme we could have various stages of startup but I don't know if that is necessary right now. It might even be possible to just do this using the AsyncShutdown module.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 6

[Dave Townsend [:mossop]]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #5)

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Not really.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 8

[:Gijs (he/him)]

(The bugbug issue is filed as https://github.com/mozilla/relman-auto-nag/issues/877 )

Comment 9

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

We ran across this issue in general triage. Since FXR and site-specific-browsers are gone, we think it might make sense to wontfix this for now. Feel free to reopen if we are mistaken. Thanks!