Bug 25171 - It should be possible to manually set the name of an anonymous function
Summary: It should be possible to manually set the name of an anonymous function
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-13 18:11 PDT by Francisco Tolmasky
Modified: 2009-07-21 06:40 PDT (History)
1 user (show)

See Also:


Attachments
Patch to add displayName property to functions (8.46 KB, patch)
2009-04-13 18:20 PDT, Francisco Tolmasky
oliver: review-
Details | Formatted Diff | Diff
Moved new code out of name and into calculatedDisplayName (10.88 KB, patch)
2009-04-13 20:22 PDT, Francisco Tolmasky
oliver: review-
Details | Formatted Diff | Diff
Removed static UString pointer for AnonymousFunction. (10.25 KB, patch)
2009-04-13 21:10 PDT, Francisco Tolmasky
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Francisco Tolmasky 2009-04-13 18:11:08 PDT
Currently anonymous functions show up as "(anonymous function)" in the profiler, which isn't too useful when they're all anonymous (generated). Aside from making WebKit smarter about auto-generating names, there should be a way to just set it yourself.
Comment 1 Francisco Tolmasky 2009-04-13 18:20:08 PDT
Created attachment 29451 [details]
Patch to add displayName property to functions

I went ahead with the displayName approach, since it may still be valuable to have access to the actual name of the function as well.
Comment 2 Oliver Hunt 2009-04-13 18:59:05 PDT
Comment on attachment 29451 [details]
Patch to add displayName property to functions

You shouldn't change the behaviour of Function.name you should make the profiler do the work of checking for name vs displayName, or alternatively add a displayName function to InternalFunction and call that.
Comment 3 Francisco Tolmasky 2009-04-13 20:22:27 PDT
Created attachment 29454 [details]
Moved new code out of name and into calculatedDisplayName

I added displayName and calculatedDisplayName (matching the CSS pattern for these things). I moved AnonymousFunction string into calculatedDisplayName since this returns the final result.
Comment 4 Oliver Hunt 2009-04-13 20:26:51 PDT
Comment on attachment 29454 [details]
Moved new code out of name and into calculatedDisplayName

static const UString * AnonymousFunction = new UString("(anonymous function)");

This shouldn't be heap allocated otherwise it will cause leak warnings, it's also potentially not threadsafe -- I recommend putting an additional string constant on GlobalData to contain "(anonymous function)"
Comment 5 Francisco Tolmasky 2009-04-13 21:10:52 PDT
Created attachment 29455 [details]
Removed static UString pointer for AnonymousFunction.

Removed static UString pointer for AnonymousFunction.
Comment 6 Oliver Hunt 2009-04-13 22:04:41 PDT
Comment on attachment 29455 [details]
Removed static UString pointer for AnonymousFunction.

Should be const UString explicitName = displayName(globalData); (eg. not a reference)

Will fix when i land
Comment 7 Oliver Hunt 2009-04-13 23:25:14 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/profiler/Profiler.cpp
	M	JavaScriptCore/runtime/CommonIdentifiers.h
	M	JavaScriptCore/runtime/InternalFunction.cpp
	M	JavaScriptCore/runtime/InternalFunction.h
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/profiler/anonymous-functions-with-display-names-expected.txt
	A	LayoutTests/fast/profiler/anonymous-functions-with-display-names.html
	A	LayoutTests/fast/profiler/named-functions-with-display-names-expected.txt
	A	LayoutTests/fast/profiler/named-functions-with-display-names.html
Committed r42478
Comment 8 Darin Adler 2009-04-14 10:27:08 PDT
Should just be "UString", not "const UString".