1492584 - Stop registering the JS component loader

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Comment 1

[Andrew McCreight [:mccr8]]

I have some patches that carry out a series of simplifications to the loader code:

- Eliminate xpcIJSModuleLoader in favor of methods on Components.Utils.
- Remove a few completely unused things from nsComponentManager.
- Eliminate the unused generality in nsComponentManager, by making it only support "js" file endings, and have it call directly into mozJSComponentLoader, and do a bit of cleanup.
- Instead of having XPCOMInit create the module loader (and thus have its lifetime managed by the component manager), have the module loader use a static refptr to keep itself alive.

Once that is done, the XPCOM registrations for the module loader are not needed and can be removed.

Comment 2

[Andrew McCreight [:mccr8]]

These patches feel more XPCOM-y, though about half of them happen in xpconnect/loader code.

Comment 3

[Kris Maglione [:kmag]]

This sounds like it should simplify my plans for moving static component registrations to a static hashtable. The JS component loader code adds a lot of thorns to that process.

Comment 4

[Andrew McCreight [:mccr8]]

The most relevant thing there is that I eliminate nsComponentManagerImpl::KnownModule::mLoader and nsComponentManagerImpl::mLoaderMap.

Comment 5

[Andrew McCreight [:mccr8]]

I used the code coverage stuff to help figure out what was entirely unused, and my patches get rid of at least one block of unused code.

Comment 6

[Andrew McCreight [:mccr8]]

I have patches in progress for this, but I got stalled out because every test suite fails when run in automation, but not when run locally. My theory is that my patches are changing when the component manager is created or destroyed in a way that causes problems. Hopefully I'll get back to this soon.

Comment 7

[Andrew McCreight [:mccr8]]

I think I finally figured out why tests were not failing locally, but they were failing in automation. It appears that when you do testing locally, you load some manifests like httpd.manifest which have invalid entries in them (see bug 1498404). When you hit an invalid entry, then it logs an error message using some XPCOM service, which starts up XPConnect and you don't get the error. If I fix those errors, then I hit the same crash as I see in automation, so in automation it must somehow be using a different set of manifests.

Comment 8

[Kris Maglione [:kmag]]

Try running `mach package` and then running tests with `--appname=dist`. That should get you a pretty similar setup to what automation uses. Unless it's an xpcshell test, in which case there's no easy way to do something similar and you have to spend a half hour hacking around until you figure out the correct set of hacks and incantations to run the packaged tests.

Comment 9

[Andrew McCreight [:mccr8]]

(In reply to Kris Maglione [:kmag] from comment #8)
> Try running `mach package` and then running tests with `--appname=dist`.
> That should get you a pretty similar setup to what automation uses.

Ah, thanks! I wasn't sure in what way I was failing to run things like they do in automation. I forgot about mach package.

> Unless it's an xpcshell test, 

Funnily enough, XPCShell tests were the only ones that were working.

Comment 10

[Andrew McCreight [:mccr8]]

Attachment: Bug 1492584, part 1 - Add eager loading flag to mozilla::Module

Initialization of nsLayoutModule carries out major Gecko startup tasks
like starting XPConnect and running
nsLayoutStatics::Initialize(). Because all XPCOM modules are loaded
lazily, it ends up being carried out when some random code uses an
XPCOM service that is part of nsLayoutModule. This can result in
XPConnect starting up at different points depending on whether or not,
say, there is a parsing error in a manifest file, which in turn can
cause packaged builds to crash where local builds do not.

To avoid this problem, we should eagerly load nsLayoutModule. Lazy
loading is nice when we're not sure if something will be needed or
not, but we can be sure that nsLayoutModule is needed if it is
registered.

One approach would be to directly call the initialization method
somewhere in XPCOM startup, but it is possible that somebody is using
XPCOM without using JS or layout, so instead I have added a flag to
Module that allows a module to specify if it should be loaded shortly
after registration and use it for nsLayoutModule.

Comment 11

[Andrew McCreight [:mccr8]]

Attachment: Bug 1492584, part 2 - Make mozJSComponentLoader::sSelf a strong reference.

The JS component loader is an XPCOM component, so it is held alive by
the component manager. In order to be able to make it no longer be an
XPCOM component, we have to keep it alive some other way. I decided to
simply keep it alive as long as XPConnect itself is alive.

Depends on D8756

Comment 12

[Andrew McCreight [:mccr8]]

Attachment: Bug 1492584, part 3 - Remove JS component loader registration.

After the previous patches, we no longer rely on the component manager
to incidentally start up XPConnect when we load the JS loader service
or to hold the JS component loader alive, so the do_GetService() call
for the JS loader in XPCOMInit.cpp can be removed. After that is done,
the JS loader is no longer used as an XPCOM component, so all of the
boilerplate for that can be removed.

Depends on D8757

Comment 13

[Andrew McCreight [:mccr8]]

Attachment: Bug 1492584, part 1 - Eagerly initialize nsLayoutModule in the component manager.

nsLayoutModule must be initialized in order to call into JS, but I
don't want to have to rely on calling a service in that
module. Instead, always initialize the module very early in component
manager initialization. This also makes initialization more
consistent, so things like errors in manifests won't affect when it
happens, which can result in different behavior in different builds.

I also made nsLayoutModule initialization infallible, because I can't
imagine that we can do much that is useful without it.

Another change I made is that gInitialized is set to true even in a
GPU process. This simplifies checking whether initialization has
happened already when we start up the layout module.

Comment 14

[Andrew McCreight [:mccr8]]

My new version of part 1 no longer uses an eager initialization flag. Instead, it directly calls nsLayoutModule's initialization during component manager initialization. Part 2 and 3 are the same modulo rebasing differences.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70984251b174
part 1 - Eagerly initialize nsLayoutModule in the component manager. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/2df4dcfce65f
part 2 - Make mozJSComponentLoader::sSelf a strong reference. r=kmag
https://hg.mozilla.org/integration/autoland/rev/2c675ab661ca
part 3 - Remove JS component loader registration. r=kmag

Comment 16

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/70984251b174
https://hg.mozilla.org/mozilla-central/rev/2df4dcfce65f
https://hg.mozilla.org/mozilla-central/rev/2c675ab661ca