1277135 - Add an rval argument to JS::CloneAndExecuteScript()

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[(no longer active)]

Attachment: Add an rval argument to JS::CloneAndExecuteScript()

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 8758520 [details] [diff] [review]
Add an rval argument to JS::CloneAndExecuteScript()

Review of attachment 8758520 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the change mentioned below.

::: js/src/jsapi-tests/testScriptObject.cpp
@@ +202,5 @@
> +        cls_CloneAndExecuteScript other;
> +        other.rt = rt;
> +        CHECK(other.setup());
> +        {
> +            JSAutoCompartment ac(cx, global);

Was this supposed to say `other.global`?

I think instead of factoring out setup/teardown and using an extra cls_CloneAndExecuteScript object, I would write this:

    {
        JS::RootedObject global2(cx, createGlobal());
        CHECK(global2);
        JSAutoCompartment ac(cx, global2);
        ...
    }

It seems shorter and more to the point; also, we've just eliminated support for multiple contexts on the same thread.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/317aecd284b8
Add an rval argument to JS::CloneAndExecuteScript(); r=jorendorff

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/317aecd284b8