1498412 - Make manifest parsing errors assert

Status: NEW

(Core :: XPCOM, enhancement)

Description

[Andrew McCreight [:mccr8]]

If the manifest parser encounters an error, it logs it, but then keeps going. We should assert, as I expect that all of the manifest files we care about in the modern era are in the tree.

I actually ended up changing when we load XPConnect by accident because I changed the manifest parser which caused some manifests to become invalid, so we log an error early, which causes us to load XPConnect a little earlier.

Comment 1

[Andrew McCreight [:mccr8]]

Local testing (with an assert) showed only one additional manifest error besides the extra manifest entries: FxAccountsComponents.manifest only specifies a component for some push thing in the main process, but defines the contract in all processes. Making the contract also process=main fixes the error.

Comment 2

[Andrew McCreight [:mccr8]]

I'm not sure this is landable right now. I have a Linux debug run that is green, but there are four XPCShell tests that are failing, in the chrome/test/unit directory: test_bug380398.js, test_bug399707.js, test_bug401153.js, test_bug564667.js.

I looked at the first one a bit. It looks like it has intentional failures, on a line that looks something like:
content  test11  test/  appversion

It doesn't look like appversion appears in any non-test manifest in m-c or c-c, and I'm not sure if we support random external manifests any more, but dredging out obsolete manifest code is probably not a good use of my time right now.

Comment 3

[Andrew McCreight [:mccr8]]

Nathan, am I right in thinking that given the deprecation of XPCOM addons, we can remove any feature from XPCOM manifests that aren't used by Firefox (and Thunderbird) itself? Or should I ask an addon person if they are used for anything now?

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Assert for malformed manifests.

For posterity, here's the patch I've been using.

Comment 5

[Nathan Froyd [:froydnj]]

(In reply to Andrew McCreight [:mccr8] from comment #3)
> Nathan, am I right in thinking that given the deprecation of XPCOM addons,
> we can remove any feature from XPCOM manifests that aren't used by Firefox
> (and Thunderbird) itself? Or should I ask an addon person if they are used
> for anything now?

I believe that's correct.  Everything interesting should go in a webext manifest?

Comment 6

[Kris Maglione [:kmag]]

(In reply to Nathan Froyd [:froydnj] from comment #5)
> (In reply to Andrew McCreight [:mccr8] from comment #3)
> > Nathan, am I right in thinking that given the deprecation of XPCOM addons,
> > we can remove any feature from XPCOM manifests that aren't used by Firefox
> > (and Thunderbird) itself? Or should I ask an addon person if they are used
> > for anything now?
> 
> I believe that's correct.  Everything interesting should go in a webext
> manifest?

Yup, although note that we currently map some extension manifest entries to chrome manifest entries using [1]. Also, Thunderbird people might not appreciate it, but that's not a blocker.

[1]: https://searchfox.org/mozilla-central/rev/3d989d097fa35afe19e814c6d5bc2f2bf10867e1/toolkit/mozapps/extensions/amIAddonManagerStartup.idl#24-39