684529 - Remove script object

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

+++ This bug was initially created as a clone of Bug #678830 +++

The bug 678830 mostly stops exposing JSScript object wrappers through JS API. But those objects are still created as a mean to reach global objects corresponding to the script and do security checks. 

We should remove the object wrappers and instead make sure that we always record the global object in the script and use it for security checks.

Comment 1

[Igor Bukanov]

Attachment: v1

Instead of creating the script objects the patch records compile-time global for the script and uses that in all the users of JSScript::u.object. With one global per compartment that would not be unnecessary but for now this avoids the extra memory bloat.

Comment 2

[Igor Bukanov]

Comment on attachment 561708 [details] [diff] [review]
v1

The patch has passed the try server.

Comment 3

[Igor Bukanov]

Attachment: v2

The new version records in JSScript::u.globalObject (former u.object) the global objects for function scripts if any. That removed the need to store the global in a separated slot in DebuggerObject and eliminated the global object parmeter from a few Debugger functions that was present in v1.

Comment 4

[Igor Bukanov]

Attachment: v2 for real

v2 was an incomplete patch, this should the real one build on top of the patch from the bug 687966.

Comment 5

[Igor Bukanov]

Attachment: v3

Here is a rebased patch

Comment 6

[Jason Orendorff [:jorendorff]]

Comment on attachment 564983 [details] [diff] [review]
v3

r=me with some nits.

In jscompartment.h:
>-}
> 
> /* Defined in jsapi.cpp */
>-extern JSClass js_dummy_class;
>+extern Class js_dummy_class;
>+
>+} /* namespace js */

If it's going to live in namespace js, it shouldn't have js_ at the
beginning of the name.

In jsscript.cpp, JSScript::NewScriptFromCG:
>+        script->u.globalObject = fun->getParent() ? fun->getParent()->getGlobal() : NULL;

I assume fun->getParent() is null here iff compilation is not compile-and-go?

In jsscript.h, in struct JSScript:
>     union {
>         /*
>-         * A script object of class ScriptClass, to ensure the script is GC'd.
>+         * A global object for the script.
>          * - All scripts returned by JSAPI functions (JS_CompileScript,
>+         *   JS_CompileFile, etc.) have these globals.

"have a globalObject" or "have a non-null globalObject".

>+         * - Function scripts have these global if the function comes from
>+         *   compile and go scrits.

"A function script has a globalObject if the function comes from a
compile-and-go script."

>          * - Temporary scripts created by obj_eval, JS_EvaluateScript, and
>+         *   similar functions never have these globals; for such scripts
>+         *   the global should be extracted from the JS frame that execute
>+         *   scripts.

"never have the globalObject field set;"

>+    /*
>+     * Return creation time global or null. FIXME - avoid duplication with
>+     * global() that extracts the global from the type information.
>+     */
>+    js::GlobalObject *getGlobalObjectOrNull() const {

How were you planning to fix this?

In vm/Debugger.cpp, DebuggerScript_check:
>-    JS_ASSERT(GetScriptReferent(thisobj));
>-
>+ 
>     return thisobj;
> }

Stray space charater on that line.

Comment 7

[Igor Bukanov]

(In reply to Jason Orendorff [:jorendorff] from comment #6)
> 
> In jsscript.cpp, JSScript::NewScriptFromCG:
> >+        script->u.globalObject = fun->getParent() ? fun->getParent()->getGlobal() : NULL;
> 
> I assume fun->getParent() is null here iff compilation is not compile-and-go?

Yes, this is true.

> >+    /*
> >+     * Return creation time global or null. FIXME - avoid duplication with
> >+     * global() that extracts the global from the type information.
> >+     */
> >+    js::GlobalObject *getGlobalObjectOrNull() const {
> 
> How were you planning to fix this?

I will remove those comments. Compartment-per-global would address this in a better way.

Comment 8

[Igor Bukanov]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d6f9285f623e

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/d6f9285f623e