1934312 - Stop using unique scope for frame scripts and process scripts

Status: NEW

(Core :: DOM: Core & HTML, task, P3)

Description

[Tooru Fujisawa [:arai]]

Once the JSM consumer goes away, the only remaining consumers of NonSyntacticVariablesObject (via js::ExecuteInFrameScriptEnvironment) are the following,

• the frame script (if aRunInUniqueScope == true, which means if aRunInGlobalScope == false)
• the process script

The list of frame scripts and process scripts is known (no external scripts, except for privileged extensions).

Scripts needs unique scope if the script has global variable (var, function), in order to avoid polluting the shared global.
Global lexical variables (let, const, class) can be used, as they're defined into NonSyntacticLexicalEnvironmentObject, and there are more consumers of it.

As far as I can see, most scripts don't use global variables, and scripts that uses global variables can easily be rewritten.
(so far, only content-process.js uses global variable with function).

So, both frame scripts and process scripts can stop using the unique scope, which removes the last consumer of js::ExecuteInFrameScriptEnvironment and NonSyntacticVariablesObject, which allows removing NonSyntacticVariablesObject handling inside SpiderMonkey.

Comment 1

[Tooru Fujisawa [:arai]]

Actually this is some more complex.
While the NonSyntacticVariablesObject is used only by ExecuteInFrameScriptEnvironment and NonSyntacticLexicalEnvironmentObject is used by other cases,
NonSyntacticLexicalEnvironmentObject is created for each call in ExecuteInFrameScriptEnvironment,
and JS_ExecuteScript in the aRunInUniqueScope shares single global lexical environment in all cases.

https://searchfox.org/mozilla-central/rev/6597dd03bad82c891d084eed25cafd0c85fb333e/dom/base/nsFrameMessageManager.cpp#1206-1209,1215-1220

if (aRunInUniqueScope) {
  JS::Rooted<JSObject*> scope(cx);
  bool ok = js::ExecuteInFrameScriptEnvironment(cx, aMessageManager,
                                                script, &scope);
...
  JS::Rooted<JS::Value> rval(cx);
  JS::EnvironmentChain envChain(cx, JS::SupportUnscopables::No);
  if (!envChain.append(aMessageManager)) {
    return;
  }
  Unused << JS_ExecuteScript(cx, envChain, script, &rval);

So, the options here are the following:

• Always put a block statement around the aRunInUniqueScope script, so that all lexical variables are defined in separate block scope (some script already does this)
• Somehow allocate different lexical scope for each script, while not using the NonSyntacticVariablesObject (maybe use another WithEnvironmentObject instead)