Closed Bug 1251139 Opened 5 years ago Closed 5 years ago

Support running a function in the parent with loadChromeScript

Categories

(Testing :: Mochitest, enhancement)

enhancement
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: MattN, Assigned: MattN)

References

Details

Attachments

(1 file)

Sometimes you need to run different code in the parent for each test and creating a new .js file for use with loadChromeScript feels too heavyweight. The workaround is usually to use a data URI (like on mm.loadFrameScript) but that doesn't work with loadChromeScript since _readUrlAsString requires being able to do a sync open on the channel. It seems that isn't supported for data URIs and nsIChannel implementations aren't required to support that[1].

ContentTaskUtils supports providing a function (which will be stringified) to run in the content process and that seems to work well and is lighter-weight than a new file so it would be nice if loadChromeScript also supported loading functions.

[1] https://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsIChannel.idl?rev=72b4f68bc170#147
Attachment #8723396 - Flags: review?(ted)
https://reviewboard.mozilla.org/r/36517/#review34753

::: testing/specialpowers/content/SpecialPowersObserverAPI.js:464
(Diff revision 1)
> -                              { id: id, url: url, err: err, message: message,
> +                              { id, name: scriptName, err, message,

before we had a field named id, now we just pass in id without a name.

It appears we still reference aMessage.json.id, that indicates we have a named field id.

::: testing/specialpowers/content/specialpowersAPI.js:463
(Diff revision 1)
> +    let scriptArgs = { id };

here I think we are just sending in an id, not a named field.
https://reviewboard.mozilla.org/r/36517/#review34753

> before we had a field named id, now we just pass in id without a name.
> 
> It appears we still reference aMessage.json.id, that indicates we have a named field id.

This is just new property syntax to avoid redundancy when the property name is the same as a local variable. `id: id,` is equivalent to `id,`

> here I think we are just sending in an id, not a named field.

Right, that's intentional. The name property is on the function object that gets added on in the block directly below.
Comment on attachment 8723396 [details]
MozReview Request: Bug 1251139 - Support running a function in the parent with loadChromeScript. r=ted

Joel, did you see my reply?
Attachment #8723396 - Flags: review?(jmaher)
Comment on attachment 8723396 [details]
MozReview Request: Bug 1251139 - Support running a function in the parent with loadChromeScript. r=ted

https://reviewboard.mozilla.org/r/36517/#review35555

thanks for clarifying the 'id'
Attachment #8723396 - Flags: review?(jmaher) → review+
https://hg.mozilla.org/mozilla-central/rev/a21e11fe4222
https://hg.mozilla.org/mozilla-central/rev/9643098c3d84
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment on attachment 8723396 [details]
MozReview Request: Bug 1251139 - Support running a function in the parent with loadChromeScript. r=ted

Apologies for not getting to this review in a timely manner.
Attachment #8723396 - Flags: review?(ted)
You need to log in before you can comment on or make changes to this bug.