Open Bug 1498412 Opened 2 years ago Updated 2 years ago

Make manifest parsing errors assert

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set
normal

Tracking

()

People

(Reporter: mccr8, Unassigned)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

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.
Assignee: nobody → continuation
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.
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.
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?
Flags: needinfo?(nfroyd)
Depends on: 1499155
For posterity, here's the patch I've been using.
(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?
Flags: needinfo?(nfroyd) → needinfo?(kmaglione+bmo)
(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
Flags: needinfo?(kmaglione+bmo)
Assignee: continuation → nobody
You need to log in before you can comment on or make changes to this bug.