Closed Bug 1484326 Opened 2 years ago Closed 2 years ago

Load customElements.js even for windowless browsers in xpcshell tests

Categories

(Toolkit :: General, task)

task
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file)

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
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.
(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.
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
(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.
(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 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.
Attachment #9002105 - Flags: review+
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c218b23035ed
Load customElements.js even for windowless browsers in xpcshell tests;r=kmag
https://hg.mozilla.org/mozilla-central/rev/c218b23035ed
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.