As per bug 1063052 comment 81: - It seems to me the main part of the failure in this bug comes from the fact that chrome manifests outside the omni.ja are treated after the chrome manifests inside the omni.ja. What about doing the opposite? If something is registered from an extracted omni.ja, it will be overridden by the contents of omni.ja. If people really want to use the contents of the extracted omni.ja, they'll have to remove omni.ja.

In case of multiple "resource" manifest entries for the same keyword, the last registered one now takes precedence, like any other chrome manifest entry. The latter is required, well, first because it's awkward that the last chrome manifest entry takes precedence for all chrome types except resources. But more importantly, in the case of bug 1063052, one part of the failure, at least one I got by upgrading 31 to nightly after unzipping omni.ja, is that resource://gre-resource resolves to the unzipped tree and doesn't contain some of the new, required files, while omni.ja registers resource://gre-resources/. So to summarize, without the component manager changes: chrome://foo/ points to (old) unzipped resource://foo/ points to (old) unzipped layout code loads resource://gre-resources/counterstyles.css which doesn't exist with the component manager changes: chrome://foo/ points to omni.ja resource://foo/ points to (old) unzipped layout code loads resource://gre-resources/counterstyles.css which doesn't exist Neither of the above are desirable. With the complete changes from this patch: chrome://foo/ points to omni.ja resource://foo/ points to omni.ja layout code properly loads resource://gre-resources/counterstyles.css. Note that, alternatively, we could just have actual chrome.manifest files near each omni.ja with a manifest entry pointing to the chrome.manifest inside the omni.ja. So in practice, we'd always have a chrome.manifest and always be overwritting whatever people would be doing to their chrome.manifest. That's slightly more work, as I'm not sure "manifest" entries support jar urls so maybe support for that would be needed (although if that's needed, that shouldn't be hard), and that would also require packager changes.

Comment on attachment 8494406 [details] [diff] [review] Change chrome.manifest read order This is correct, but I still feel like we're doing this in a weird order within this method. Is there a reason we can't reorder this method so that things are registered in the order they will be processed? so: RegisterModule(&kXPCOMModule); // needed for mozilla::Monijar to work, right? greDir greOmnijar appDir appOmnijar I'm going to mark r+ on this version but I'd really prefer that form better!

In case of multiple "resource" manifest entries for the same keyword, the last registered one now takes precedence, like any other chrome manifest entry.

Comment on attachment 8495014 [details] [diff] [review] Change chrome.manifest read order This unfortunately breaks m-e10s bc3 and xpcshell.

So, the xpcshell problem was just a test dans was explicitly testing the current behavior that registering resources has. But the best of all is the e10s test that somehow changes an existing error from a warning to an error because the test loads the superpowers addon that registers the superpowers resource that already is registered by marionette. So before, that registration wasn't happening and the error about AboutProtocolParent.tryUnregisterFactory not existing ends up a warning. Now, the registration happens and that error ends up an actual error that turns the test red.