1484326 - Load customElements.js even for windowless browsers in xpcshell tests

Status: RESOLVED FIXED

(Toolkit :: General, task)

Description

[Brian Grinstead [:bgrins]]

AFAICT MainProcessSingleton doesn't get loaded for xpcshell tests, but we have some xpcshell tests that create a hidden window and a <browser> element. So when attempting to migrate <browser> to a Custom Element, the CE doesn't get attached (since customElements.js never gets loaded).

I've found at least 2 distinct places this happens in xpcshell tests so far, which is making me think we should look into loading customElements.js somewhere else as opposed to manually requiring it from those cases:

- https://searchfox.org/mozilla-central/rev/246f2b4fab2c1a6cca99418bc2e4d73d1102cc38/toolkit/components/extensions/ExtensionParent.jsm#1117
- https://searchfox.org/mozilla-central/rev/246f2b4fab2c1a6cca99418bc2e4d73d1102cc38/toolkit/components/extensions/ExtensionXPCShellUtils.jsm#121

Comment 1

[Kris Maglione [:kmag]]

The general pattern here is to just register such scripts in ExtensionXPCShellUtils.jsm. That module is more or less always required to be loaded when running WebExtensions from xpcshell in order to deal with these sorts of things.

xpcshell in general is meant to start out with as minimal an environment as possible so different tests can decide which code they want running for any given test, so loading this for *all* xpcshell tests doesn't seem like the right solution.

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Kris Maglione [:kmag] from comment #1)
> The general pattern here is to just register such scripts in
> ExtensionXPCShellUtils.jsm. That module is more or less always required to
> be loaded when running WebExtensions from xpcshell in order to deal with
> these sorts of things.
> 
> xpcshell in general is meant to start out with as minimal an environment as
> possible so different tests can decide which code they want running for any
> given test, so loading this for *all* xpcshell tests doesn't seem like the
> right solution.

I was considering directly loading customElements.js from C++ wherever we dispatch document-element-inserted so it could continue to be done in one place (since I was thinking we'd have to do it in each place that a browser got created). If we can do it only in ExtensionXPCShellUtils.jsm that seems reasonable. I'll give it a try and see if anything fails.

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to Kris Maglione [:kmag] from comment #1)
> xpcshell in general is meant to start out with as minimal an environment as
> possible so different tests can decide which code they want running for any
> given test, so loading this for *all* xpcshell tests doesn't seem like the
> right solution.

We'll only want to load this script in response to document-element-inserted notifications which presumably won't fire in all xpcshell tests, only those that create browsers.

Comment 4

[Kris Maglione [:kmag]]

(In reply to Brian Grinstead [:bgrins] from comment #3)
> (In reply to Kris Maglione [:kmag] from comment #1)
> > xpcshell in general is meant to start out with as minimal an environment as
> > possible so different tests can decide which code they want running for any
> > given test, so loading this for *all* xpcshell tests doesn't seem like the
> > right solution.
> 
> We'll only want to load this script in response to document-element-inserted
> notifications which presumably won't fire in all xpcshell tests, only those
> that create browsers.

Well, it will fire in any xpcshell test that creates *any* document, even those that will never include XUL. We have a bunch of xpcshell tests that create windowless browsers to do arbitrary things, at this point, and I suspect a lot of them won't want arbitrary scripts loaded into them.

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: Bug 1484326 - Load customElements.js even for windowless browsers in xpcshell tests;r=kmag

Comment 6

[Kris Maglione [:kmag]]

Comment on attachment 9002105 [details]
Bug 1484326 - Load customElements.js even for windowless browsers in xpcshell tests;r=kmag

Kris Maglione [:kmag] has approved the revision.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c218b23035ed
Load customElements.js even for windowless browsers in xpcshell tests;r=kmag

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/c218b23035ed