368714 - [FIX]Sandbox evaluation should handle branch callbacks

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Boris Zbarsky [:bzbarsky]]

In particular, it should call the DOM branch callback if that's relevant, so that if the script hangs the user can interrupt it.  See bug 368655 for a testcase.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Though if we're worried about someone unsetting the branch callback on the original cx from inside the sandbox, we could reget it on every branch or something...

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 253351 [details] [diff] [review]
Proposed fix


>+        // Now cache the original branch callback
>+        mOrigBranchCallback = JS_SetBranchCallback(aOuterCx, nsnull);
>+        JS_SetBranchCallback(aOuterCx, mOrigBranchCallback);

Sorry I didn't add a getter to the JS API.

>+JSBool JS_DLL_CALLBACK
>+ContextHolder::ContextHolderBranchCallback(JSContext *cx, JSScript *script)
>+{
>+    ContextHolder* thisObject =
>+        NS_STATIC_CAST(ContextHolder*, JS_GetContextPrivate(cx));
>+    NS_ASSERTION(thisObject, "How did that happen?");
>+
>+    if (thisObject->mOrigBranchCallback) {
>+        return (thisObject->mOrigBranchCallback)(thisObject->mOrigCx, script);

Nit: no need to parenthesize the callable expression.  Very old C hackers remember when one had to not only parenthesize, but also put a * after the (, but those were the bad old days ;-).

/be

Comment 3

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 253351 [details] [diff] [review]
Proposed fix

I think we should consider this for branches as well...

Comment 5

[Daniel Veditz [:dveditz]]

Comment on attachment 253351 [details] [diff] [review]
Proposed fix

approved for 1.8/1.8.0 branches, a=dveditz for drivers

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Merged to branch

Comment 7

[Boris Zbarsky [:bzbarsky]]

Fixed on branches.

Comment 8

[juan becerra [:juanb]]

Verified on: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070206 BonEcho/2.0.0.2pre
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.10pre) Gecko/20070206
Firefox/1.5.0.10pre

Using testcase url in bug 368655. Also tested on 1.8.1.2 in mac and linux.