Bug 152398 – Web Inspector: Improve names in Debugger Call Stack section when paused

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

* SUMMARY
Improve names in Debugger Call Stack section when paused.

* TEST
<script>
function foo() { debugger; }
foo();
</script>

* STEPS TO REPRODUCE
1. Inspect test page
2. Reload to hit breakpoint
  => Bottom Call Frame in Debugger Sidebar says "(anonymous function)", expected "Global Code"

[Joseph Pecoraro]

Created attachment 267586 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 267587 [details]
[IMAGE] Before

[Joseph Pecoraro]

Created attachment 267588 [details]
[IMAGE] After

[Joseph Pecoraro]

Comment on attachment 267586 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=267586&action=review

> Source/WebInspectorUI/UserInterface/Models/CallFrame.js:132
> +        let nativeCode = undefined; // FIXME: Can we determine this?

Hmm, it seems I left an unanswered question. Right now we are matching existing behavior by just always having this be false. I was unable to get native call frames to show up in the sidebar, but that is likely because of earlier processing (!sourceCodeLocation bailing) in DebuggerManager. I should file a bugzilla bug about this.

[Joseph Pecoraro]

Created attachment 267589 [details]
[PATCH] Proposed Fix

[Blaze Burg]

Comment on attachment 267586 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=267586&action=review

r=me, we can split out the native call frame thing.

> Source/JavaScriptCore/interpreter/CallFrame.cpp:233
> +String CallFrame::friendlyFunctionName()

Is this code covered by a test anywhere?

> Source/JavaScriptCore/interpreter/CallFrame.cpp:248
> +            return getCalculatedDisplayName(this, callee());

We could do a type test here to see if it's a native, builtin, etc function.

> Source/WebInspectorUI/UserInterface/Models/CallFrame.js:130
> +        let thisObject = WebInspector.RemoteObject.fromPayload(payload.this);

payload.this reads very strangely. This is not new though.

> Source/WebInspectorUI/UserInterface/Models/CallFrame.js:146
> +        let programCode = WebInspector.CallFrame.programCodeFromPayload(payload.functionName);

Shouldn't the named argument be a payload instead of function name?

[Blaze Burg]

Comment on attachment 267589 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=267589&action=review

r=me

> Source/WebInspectorUI/UserInterface/Models/CallFrame.js:132
> +        let nativeCode = false;

If you want to keep the fixme, add a bug number.

[Joseph Pecoraro]

(In reply to comment #6)
> Comment on attachment 267586 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=267586&action=review
> 
> r=me, we can split out the native call frame thing.
> 
> > Source/JavaScriptCore/interpreter/CallFrame.cpp:233
> > +String CallFrame::friendlyFunctionName()
> 
> Is this code covered by a test anywhere?

Not explicitly yet, no.


> > Source/WebInspectorUI/UserInterface/Models/CallFrame.js:130
> > +        let thisObject = WebInspector.RemoteObject.fromPayload(payload.this);
> 
> payload.this reads very strangely. This is not new though.

This is actually called "this" in the protocol Debugger.CallFrame:

    "id": "CallFrame",
    "properties": [
        ...
        { "name": "this", "$ref": "Runtime.RemoteObject", "description": "<code>this</code> object for this call frame." }
    ]


> > Source/WebInspectorUI/UserInterface/Models/CallFrame.js:146
> > +        let programCode = WebInspector.CallFrame.programCodeFromPayload(payload.functionName);
> 
> Shouldn't the named argument be a payload instead of function name?

I'll make this change.

[Joseph Pecoraro]

<http://trac.webkit.org/changeset/194247>

[Joseph Pecoraro]

EFL Build Fix: <http://trac.webkit.org/changeset/194249>