WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
25467
JavaScript debugger should use function.displayName as the function's name in the call stack
https://bugs.webkit.org/show_bug.cgi?id=25467
Summary
JavaScript debugger should use function.displayName as the function's name in...
Adam Roben (:aroben)
Reported
2009-04-29 13:40:17 PDT
r42478
added a displayName property to functions, which, if it exists, is used in the JavaScript profiler as the function's name. This is especially useful for anonymous functions, which don't otherwise have an intelligible name to be displayed. We should use the displayName property when deciding what name to show for a function in the JavaScript debugger's call stack, too.
Attachments
Adds check of displayName
(5.66 KB, patch)
2009-05-04 22:20 PDT
,
Francisco Tolmasky
aroben
: review-
Details
Formatted Diff
Diff
Removed const, put anonymous string back in js
(4.85 KB, patch)
2009-05-04 22:44 PDT
,
Francisco Tolmasky
aroben
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Adam Roben (:aroben)
Comment 1
2009-04-29 13:40:54 PDT
<
rdar://problem/6840007
>
Francisco Tolmasky
Comment 2
2009-05-04 22:20:44 PDT
Created
attachment 30010
[details]
Adds check of displayName Basically did the same thing we do in profilenodes and checked the displayName of a function as well.
Adam Roben (:aroben)
Comment 3
2009-05-04 22:34:07 PDT
Comment on
attachment 30010
[details]
Adds check of displayName
> Index: JavaScriptCore/ChangeLog > =================================================================== > --- JavaScriptCore/ChangeLog (revision 43204) > +++ JavaScriptCore/ChangeLog (working copy) > @@ -1,3 +1,15 @@ > +2009-05-04 Francisco Tolmasky <
francisco@280north.com
> > + > +
BUG 25467
: JavaScript debugger should use function.displayName as the function's name in the call stack > + <
https://bugs.webkit.org/show_bug.cgi?id=25467
> > + > + Reviewed by NOBODY (OOPS!). > + > + * JavaScriptCore.exp: Added calculatedFunctionName > + * debugger/DebuggerCallFrame.cpp: Added calculatedFunctionName to match existing one in ProfileNode. > + (JSC::DebuggerCallFrame::calculatedFunctionName): > + * debugger/DebuggerCallFrame.h: Added calculatedFunctionName to match existing one in ProfileNode. > + > 2009-05-03 Steve Falkenburg <
sfalken@apple.com
> > > Windows build fix. > Index: JavaScriptCore/JavaScriptCore.exp > =================================================================== > --- JavaScriptCore/JavaScriptCore.exp (revision 43202) > +++ JavaScriptCore/JavaScriptCore.exp (working copy) > @@ -328,6 +328,7 @@ __ZNK3JSC16JSVariableObject16isVariableO > __ZNK3JSC16JSVariableObject21getPropertyAttributesEPNS_9ExecStateERKNS_10IdentifierERj > __ZNK3JSC17DebuggerCallFrame10thisObjectEv > __ZNK3JSC17DebuggerCallFrame12functionNameEv > +__ZNK3JSC17DebuggerCallFrame22calculatedFunctionNameEv > __ZNK3JSC17DebuggerCallFrame4typeEv > __ZNK3JSC17DebuggerCallFrame8evaluateERKNS_7UStringERNS_7JSValueE > __ZNK3JSC4Heap10statisticsEv > Index: JavaScriptCore/debugger/DebuggerCallFrame.cpp > =================================================================== > --- JavaScriptCore/debugger/DebuggerCallFrame.cpp (revision 43202) > +++ JavaScriptCore/debugger/DebuggerCallFrame.cpp (working copy) > @@ -46,6 +46,17 @@ const UString* DebuggerCallFrame::functi > return 0; > return &function->name(&m_callFrame->globalData()); > } > + > +const UString DebuggerCallFrame::calculatedFunctionName() const
The "const" on the return type isn't doing you any good, since callers will just be copying the string anyway.
> +static const char* AnonymousFunction = "(anonymous function)";
We use a lowercase first letter for variable names, even global variables and constants.
> @@ -64,11 +66,11 @@ String JavaScriptCallFrame::functionName > { > ASSERT(m_isValid); > if (!m_isValid) > - return String(); > - const UString* functionName = m_debuggerCallFrame.functionName(); > - if (!functionName) > - return String(); > - return *functionName; > + return AnonymousFunction; > + const UString functionName = m_debuggerCallFrame.calculatedFunctionName(); > + if (functionName.isEmpty()) > + return AnonymousFunction; > + return functionName; > } > > DebuggerCallFrame::Type JavaScriptCallFrame::type() const > Index: WebCore/inspector/front-end/CallStackSidebarPane.js > =================================================================== > --- WebCore/inspector/front-end/CallStackSidebarPane.js (revision 43202) > +++ WebCore/inspector/front-end/CallStackSidebarPane.js (working copy) > @@ -51,7 +51,7 @@ WebInspector.CallStackSidebarPane.protot > do { > switch (callFrame.type) { > case "function": > - title = callFrame.functionName || WebInspector.UIString("(anonymous function)"); > + title = callFrame.functionName;
Why are you moving the "(anonymous function)" string out of JavaScript? This change has made that string unlocalizable, which seems like a step in the wrong direction. If we really do want it to be unlocalizable, we should remove it from localizedStrings.js, too. Thanks for working on this! r- until we figure out the localized string issue.
Francisco Tolmasky
Comment 4
2009-05-04 22:44:49 PDT
Created
attachment 30011
[details]
Removed const, put anonymous string back in js
Geoffrey Garen
Comment 5
2009-05-04 23:03:22 PDT
Comment on
attachment 30011
[details]
Removed const, put anonymous string back in js
> +UString DebuggerCallFrame::calculatedFunctionName() const > +{ > + if (!m_callFrame->codeBlock()) > + return 0; > + > + JSFunction* function = static_cast<JSFunction*>(m_callFrame->callee()); > + if (!function) > + return 0; > + return function->calculatedDisplayName(&m_callFrame->globalData()); > +}
The best way to make a null string is "UString()", rather than "0".
> + UString functionName = m_debuggerCallFrame.calculatedFunctionName(); > + if (functionName.isEmpty()) > return String(); > - return *functionName; > + return functionName; > }
Just plain "return m_debuggerCallFrame.calculatedFunctionName();" should work.
Adam Roben (:aroben)
Comment 6
2009-05-04 23:04:49 PDT
Comment on
attachment 30011
[details]
Removed const, put anonymous string back in js This patch as-is will break the Windows build. We'll need to modify JavaScriptCore[_debug].def to fix it. r=me
Eric Seidel (no email)
Comment 7
2009-05-15 00:40:32 PDT
Assigning to aroben, since I can't land this w/o breaking the windows build.
Timothy Hatcher
Comment 8
2009-05-15 06:21:43 PDT
Adam, can you land this today?
Adam Roben (:aroben)
Comment 9
2009-05-15 11:13:41 PDT
Landed as
r43774
David Kilzer (:ddkilzer)
Comment 10
2009-05-15 14:59:27 PDT
http://trac.webkit.org/changeset/43774
No changes to inspector/front-end/CallStackSidebarPane.js were included in the last patch: + * inspector/front-end/CallStackSidebarPane.js: Remove "|| anonymous function" since it is handled internally just like in profiles + (WebInspector.CallStackSidebarPane.prototype.update): I guess this part of the ChangeLog entry should have been removed.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug