786711 - Stack frame listed as anonymous rather than the expected name

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Kevin Dangoor]

STR:

1. go to http://todomvc.com/architecture-examples/backbone/
2. set a breakpoint in todo.js around line 21 (this.save in the toggle function)
3. add a todo item
4. click the checkbox to toggle it
5. note that the stack frames are all "anonymous" when what you really want is for the frames to have useful names

expected result:
the top frame should be called "toggle".

Comment 1

[Victor Porof [:vporof][:vp]]

(In reply to Kevin Dangoor from comment #0)
> STR:
> 
> 1. go to http://todomvc.com/architecture-examples/backbone/
> 2. set a breakpoint in todo.js around line 21 (this.save in the toggle
> function)
> 3. add a todo item
> 4. click the checkbox to toggle it
> 5. note that the stack frames are all "anonymous" when what you really want
> is for the frames to have useful names
> 
> expected result:
> the top frame should be called "toggle".

The function is actually anonymous. "toggle" is the property name in the container object (app.Todo, or the thing that results via calling Backbone.Model.extend), which has the value of an anonymous function expression (not a named expression, and not a declaration).

This is less than optimal, for sure, but that's how js works.

So
{ foo: function(){} }
would create the anonymous function, via an anonymous function expression

and
{ foo: function bar(){} }
would create the function foo, via a named function expression

Comment 2

[Victor Porof [:vporof][:vp]]

("the function bar", not "foo" in my last example)

Comment 3

[Kevin Dangoor]

Sure, I agree that the function is anonymous as far as JavaScript is concerned. As far as the user is concerned, however, it is not anonymous... it's "toggle". And, according to the other browser tools it's "toggle".

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

Alex, Jason: where are the statically-inferred names from bug 433529 surfacing in JS? Isn't fn.name updated with that information? Does that work cover the case in this bug?

Comment 5

[Jason Orendorff [:jorendorff]]

fn.displayName is what you want. Jim says it's not in the protocol yet.

Comment 6

[u443197]

If you're using the C++ api, you can also use JS_GetFunctionDisplayId instead of JS_GetFunctionId

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Jason Orendorff [:jorendorff] from comment #5)
> fn.displayName is what you want. Jim says it's not in the protocol yet.

I wonder why this doesn't work then:

http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/debugger/server/dbg-script-actors.js#1688

Is displayName set on the prototype, when it's set by the engine?

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

Duh, probably because I'm using the Debugger API there...

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Simple fix

Thanks to Alex's work this was too easy. Try:
https://tbpl.mozilla.org/?tree=Try&rev=836b3125544e

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 657798 [details] [diff] [review]
Simple fix

Since Rob is on PTO, maybe you guys can take a look at this. It's pretty simple actually.

Comment 11

[Victor Porof [:vporof][:vp]]

Comment on attachment 657798 [details] [diff] [review]
Simple fix

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

Looks good to me.

::: browser/devtools/debugger/test/browser_dbg_displayName.js
@@ +58,5 @@
> +      is(frames.querySelectorAll(".dbg-stackframe").length, 3,
> +        "Should have three frames.");
> +
> +      is(frames.querySelector("#stackframe-0 .dbg-stackframe-name").getAttribute("value"),
> +        "a/<", "Frame name should be a/<");

I love the way these inferred names show up.

Comment 12

[Mihai Sucan [:msucan]]

Comment on attachment 657798 [details] [diff] [review]
Simple fix

Looks good!

Comment 13

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/20bb14ce0b55

Comment 14

[Jim Blandy :jimb]

Comment on attachment 657798 [details] [diff] [review]
Simple fix

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

::: toolkit/devtools/debugger/server/dbg-script-actors.js
@@ +1690,4 @@
>      let desc = aFunction.getOwnPropertyDescriptor("displayName");
>      if (desc && desc.value && typeof desc.value == "string") {
>        name = desc.value;
> +    } else if ("displayName" in aFunction) {

Every Debugger.Object instance should have a "displayName" property --- its value will be undefined, if we couldn't infer any name for it at all, but it will be present. So this 'in' test will always return true.

Comment 15

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/mozilla-central/rev/20bb14ce0b55

Comment 16

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Remove redundant check

(In reply to Jim Blandy :jimb from comment #14)
> ::: toolkit/devtools/debugger/server/dbg-script-actors.js
> @@ +1690,4 @@
> >      let desc = aFunction.getOwnPropertyDescriptor("displayName");
> >      if (desc && desc.value && typeof desc.value == "string") {
> >        name = desc.value;
> > +    } else if ("displayName" in aFunction) {
> 
> Every Debugger.Object instance should have a "displayName" property --- its
> value will be undefined, if we couldn't infer any name for it at all, but it
> will be present. So this 'in' test will always return true.

OK, I just converted the "else if" to an "else" then.

Comment 17

[Jim Blandy :jimb]

Comment on attachment 659984 [details] [diff] [review]
Remove redundant check

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

displayName will still return null if the function is anonymous and SpiderMonkey couldn't infer a name for it. Is that all right?

Comment 18

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Jim Blandy :jimb from comment #17)
> Comment on attachment 659984 [details] [diff] [review]
> Remove redundant check
> 
> Review of attachment 659984 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> displayName will still return null if the function is anonymous and
> SpiderMonkey couldn't infer a name for it. Is that all right?

Yes, in that case the frontend will just display <anonymous> as before.

Comment 19

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/93e7a6cb66ff

Comment 20

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/93e7a6cb66ff