552516 - avoiding temporary JSContext instance in xpc_CreateSandboxObject

Status: RESOLVED FIXED

(Core :: XPConnect, enhancement)

Description

[Igor Bukanov]

Currently xpc_CreateSandboxObject from js/src/xpconnect/src/xpccomponents.cpp contains the following code:

    XPCAutoJSContext tempcx(JS_NewContext(JS_GetRuntime(cx), 1024), PR_FALSE);
    if (!tempcx)
        return NS_ERROR_OUT_OF_MEMORY;

    JSAutoRequest req(tempcx);
    JSObject *sandbox = JS_NewObject(tempcx, &SandboxClass, nsnull, nsnull);
    if (!sandbox)
        return NS_ERROR_XPC_UNEXPECTED;

    JS_SetGlobalObject(tempcx, sandbox);

with no more references to tempcx. Thus the only purpose of the temporary context is to create the sandbox object with null proto and parent and to root it via setting the global object for tempcx to it. This can be replaced with JS_NewObjectWithGivenProto on the original cx and using one of autoroots to hold the result.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Patch

Sorry for jumping the gun here, igor, but I'm patching this code anyway right now and this patch was too tempting.

Comment 2

[Igor Bukanov]

Comment on attachment 432971 [details] [diff] [review]
Patch

>-    XPCAutoJSContext tempcx(JS_NewContext(JS_GetRuntime(cx), 1024), PR_FALSE);
>-    if (!tempcx)
>-        return NS_ERROR_OUT_OF_MEMORY;

With this change XPCAutoJSContext is only used for ContextHolder. So as a bonus that can be removed and the corresponding code just trivially inlined in the ContextHolder class.

Comment 3

[Igor Bukanov]

(In reply to comment #2)
> (From update of attachment 432971 [details] [diff] [review])
> >-    XPCAutoJSContext tempcx(JS_NewContext(JS_GetRuntime(cx), 1024), PR_FALSE);
> >-    if (!tempcx)
> >-        return NS_ERROR_OUT_OF_MEMORY;
> 
> With this change XPCAutoJSContext is only used for ContextHolder. So as a bonus
> that can be removed and the corresponding code just trivially inlined in the
> ContextHolder class.

I meant that the XPCAutoJSContext class can be removed.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: With that

Good catch!

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

I'll try to check this in, but if a checkin-needed fairy has time when the tree is open, then I'd welcome help.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/8a385caddbef