584355 - Function.caller property strange behaviour

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[antonglv]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.9.2.8) Gecko/20100722 Firefox/3.6.8
Build Identifier: 

It seems Function.caller does not implement 'toString' method in some cases.

Reproducible: Always

Steps to Reproduce:
1. Open attached test.xul file in browser
2. Click both buttons
3.
Actual Results:  
The first button throws exception:
Error: uncaught exception: [Exception... "Could not convert JavaScript argument arg 0 [nsIDOMWindowInternal.alert]"  nsresult: "0x80570009 (NS_ERROR_XPC_BAD_CONVERT_JS)"  location: "JS frame :: test.xul :: ff :: line 8"  data: no]

and the second button shows message:

function f() {
    ff();
}

Expected Results:  
Both buttons should show message:

function f() {
    ff();
}

Comment 1

[antonglv]

Attachment: 'test.xul' file to reproduce the bug

Comment 2

[Brendan Eich [:brendan]]

This is a regression from the patch for bug 494235. The fix is coming in Paul's patch for bug 578158 (once I sr it ;-). Noting dependencies, attaching a spot-fix patch in a sec.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: spot-fix against m-c tip

Comment 4

[Brendan Eich [:brendan]]

Attachment: spot-fix with jsreftest

Comment 5

[Paul Biggar]

Attachment: Patch built on attachement 462838

This integrates Brendan's fix with bug 578158. It's built on top of attachment 462838 [details] [diff] [review].

Comment 6

[Brendan Eich [:brendan]]

Giving this bug to Paul -- hope it's ok.

/be

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 462843 [details] [diff] [review]
Patch built on attachement 462838

>-WrapEscapingClosure(JSContext *cx, JSStackFrame *fp, JSObject *funobj, JSFunction *fun)
>+WrapEscapingClosure(JSContext *cx, JSStackFrame *fp, JSFunction *fun)

Good catch -- no need for funobj (nor was there ever).

> NewObject(JSContext *cx, js::Class *clasp, JSObject *proto, JSObject *parent)
> {
>     if (clasp == &js_FunctionClass)
>-        return NewFunction(cx, NULL, parent);
>-
>-    return NewNonFunction<withGivenProto>(cx, clasp, proto, parent);
>+        return detail::NewObject<withGivenProto, true>(cx, clasp, proto, parent);
>+    else
>+        return detail::NewObject<withGivenProto, false>(cx, clasp, proto, parent);

else after return is a non-sequitur, avoid.

But in cases like this one, return isFunction() ? ... : ... seems better yet. Must the C++ template parameter be true or false literally, or could you pass isFunction() and common even more code? I'm dreaming, I know.

/be

Comment 8

[Paul Biggar]

Attachment: With review changes.

With the fixes from Brendan's review.

We can't put a run-time value into a template instantiation, as it expects a compile-time constant.

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 463117 [details] [diff] [review]
With review changes.

Need the test I included in my second attached patch -- r=me with that included.

/be

Comment 10

[Paul Biggar]

Attachment: Change being committed (now has testcase)

Now with testcase.

Comment 11

[Paul Biggar]

http://hg.mozilla.org/tracemonkey/rev/6387329315b2

Comment 12

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/6387329315b2