Add extension name and ID to content script sandbox names to make debugging easier

RESOLVED FIXED in Firefox 59

Status

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: kmag, Assigned: kmag)

Tracking

unspecified
mozilla59
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

Last year
No description provided.
Comment hidden (mozreview-request)

Comment 2

Last year
mozreview-review
Comment on attachment 8941584 [details]
Bug 1429567: Add more detailed add-on info to sandbox names.

https://reviewboard.mozilla.org/r/211852/#review217914

Care to normalize a couple of other places where we set sandboxName?

https://searchfox.org/mozilla-central/search?q=sandboxName&path=components/extensions

::: toolkit/components/extensions/ExtensionContent.jsm:429
(Diff revision 1)
>        // should be the contentWindow itself. We create a sandbox with the
>        // contentWindow as principal and with X-rays disabled because it
>        // enables us to create the APIs object in this sandbox object and then
>        // copying it into the iframe's window.  See bug 1214658.
>        this.sandbox = Cu.Sandbox(contentWindow, {
> -        sandboxName: `Content Script ExtensionPage ${this.extension.id}`,
> +        sandboxName: `Extension Content Page ${extension.policy.debugName}`,

nit: we call them simply "Extension Pages", even on MDN.

::: toolkit/components/extensions/extension-process-script.js:345
(Diff revision 1)
>                              extension.manifest.background.scripts),
>  
>          contentScripts: extension.contentScripts.map(parseScriptOptions),
>        });
>  
> +      policy.debugName = `"${policy.name}" (ID: ${policy.id}, ${policy.getURL()})`;

What are the targets where this will be displayed?  If this is only for `about:memory`, that already includes the moz-extension url in the principal, so it might be redundant here.
Attachment #8941584 - Flags: review?(tomica) → review+
Assignee

Comment 3

Last year
mozreview-review-reply
Comment on attachment 8941584 [details]
Bug 1429567: Add more detailed add-on info to sandbox names.

https://reviewboard.mozilla.org/r/211852/#review217914

> nit: we call them simply "Extension Pages", even on MDN.

These aren't normal extension pages. They're extension pages running in a web content process with a limited API. We don't really have an official name for them, alas.

> What are the targets where this will be displayed?  If this is only for `about:memory`, that already includes the moz-extension url in the principal, so it might be redundant here.

For the moment, it's only about:memory, but I'm adding this kind of information in other places, too, so I'd like to have a standard string we can use.

Comment 5

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/4806f6814163
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla59

Comment 6

Last year
Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(kmaglione+bmo) → qe-verify-

Updated

Last year
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.