1880904 - [meta] Move browser window opening/closing handling out of browser.js and improve how it calls its dependencies

Status: NEW

(Firefox :: General, task, P3)

Description

[:Gijs (he/him)]

gBrowserInit is the biggest object (by lines of code) in browser.js, and thus is the biggest part of bug 1880897, but it has some additional motivation that is not there for all the other mess that's in browser.js .

First, it is really only used for browser windows, whereas some other portions of browser.js are used for e.g. the hidden window on macOS, or the sidebar. It's used for all the startup and teardown of a browser window, and it makes sense that this doesn't necessarily live in the same place as all the actual individual parts.

Another aspect is that right now some of that startup/teardown code is frightfully naive. Specifically, it's just big functions that call a lot of other big functions. What's wrong with that? Well, if any of them throw an exception, none of the others run. This bites us (cf. bug 1874114). We should use a better mechanism for this.

Finally, it's somewhat unsatisfying for any new caller that cares about doing something when the browser window opens/closes to be forced to add lines to a single point of failure. A one-step-removed (dependency injection) mechanism like the category manager would probably be a better fit to avoid these direct dependencies.

Comment 1

[:Gijs (he/him)]

Before I file more bugs, Emilio, do we have a more suitable dependency injection mechanism in XPCOM / JS than the category manager, for consumers of browser.js ' onDOMContentLoaded and load and delayedStartup and so on, to load + notify/instantiate consumers based on a static kind of registration?

I found https://searchfox.org/mozilla-central/rev/bf8c7a7d47debb1c22b51a72184d5c703ae57588/xpcom/components/nsCategoryManager.cpp#633 but it doesn't look like we currently have a JS equivalent, though I could write one, of course... (I suppose then I could in principle skip XPCOM beyond the category manager itself, and have the manifest values be resource/chrome URLs for modules to load, or something)

Comment 2

[Dave Townsend [:mossop]]

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

I found https://searchfox.org/mozilla-central/rev/bf8c7a7d47debb1c22b51a72184d5c703ae57588/xpcom/components/nsCategoryManager.cpp#633 but it doesn't look like we currently have a JS equivalent, though I could write one, of course... (I suppose then I could in principle skip XPCOM beyond the category manager itself, and have the manifest values be resource/chrome URLs for modules to load, or something)

Given most of this is in JS land I think we're better off doing that. Not sure of the restrictions on what the category manager accepts but we could do:

category browser-window-init resource:///modules/Foo.sys.mjs init

Which would call an exported init function in resource:///modules/Foo.sys.mjs.

Do we have any ordering problems to worry about or are all these things isolated?

Comment 3

[:Gijs (he/him)]

(In reply to Dave Townsend [:mossop] from comment #2)

Do we have any ordering problems to worry about or are all these things isolated?

It's difficult to be 100% sure off-hand. I would hope we don't have an ordering problem to worry about, and most of the code looks like it's basically just adding event listeners or observers. So we should be OK, but the proof will be in the pudding, I expect.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

I'm not super familiar with the category manager. What Dave mentioned sounds workable but maybe ask Nika if that doesn't work?