If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Many content/frame scripts unintentionally pollute the per-tab global scope by using `this`

NEW
Unassigned

Status

()

Core
DOM: Content Processes
P2
normal
8 months ago
13 days ago

People

(Reporter: MattN, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

`XPCOMUtils.defineLazyModuleGetter(this, "SomeName", …)` is a very common pattern to reduce memory usage for modules which aren't always needed. I understand that `this` refers to the per-tab global, not the "special per-script object". So the problem is that many frame scripts are attaching the lazy module getter to the per-tab global (thus polluting that namespace) when they're usually only needed within the script file.

It seems like having `this` refer to the "special per-script object" and having a separate `tabGlobal` object available would have avoided this problem and been more natural IMO. Why is it done the way it is?

If we can't change the meaning of `this` to be less of a footgun or provide a name to reference the object where frame variables are defined (to use with defineLazyModuleGetter in place of `this`) then we should probably audit the code to find unintentional scope pollution. For example, here are a bunch of uses of defineLazyModuleGetter with `this` in frame scripts:
https://dxr.mozilla.org/mozilla-central/search?q=define+this+file%3A%40content+-path%3Atest+-path%3Ab2g+ext%3Ajs&redirect=false

[1] https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Message_Manager/Frame_script_environment
My current patches for bug 1186409 actually change this, for .jsms. What is needed is to change the case for NonSyntacticVariablesObject in js::GetThisValue() to return the NSVO, rather than recurse into enclosingEnvironment(). My understanding (which may be wrong) is that the current behavior for frame scripts is needed for addon compatibility.
See Also: → bug 1186409

Updated

13 days ago
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.