Bug 56965 - [chromium] Add traceEvents to compositor
Summary: [chromium] Add traceEvents to compositor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nat Duca
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-23 15:13 PDT by Nat Duca
Modified: 2011-03-24 12:02 PDT (History)
4 users (show)

See Also:

Attachments
Patch (8.01 KB, patch)
2011-03-23 15:18 PDT, Nat Duca 		no flags Details | Formatted Diff | Diff
Patch (10.40 KB, patch)
2011-03-23 19:07 PDT, Nat Duca 		no flags Details | Formatted Diff | Diff
Patch (10.39 KB, patch)
2011-03-24 10:25 PDT, Nat Duca 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nat Duca 2011-03-23 15:13:17 PDT 
[chromium] Add traceEvents to compositor
Comment 1 Nat Duca 2011-03-23 15:18:05 PDT 
Created attachment 86705 [details]
Patch
Comment 2 Kenneth Russell 2011-03-23 17:46:07 PDT 
Comment on attachment 86705 [details]
Patch

I think it would be better to add a helper class in WebCore/platform/chromium/ which calls PlatformBridge's traceEventBegin in its constructor and traceEventEnd in its destructor, and stack allocate it where desired. This patch seems too brittle.
Comment 3 Nat Duca 2011-03-23 19:07:43 PDT 
Created attachment 86732 [details]
Patch
Comment 4 Nat Duca 2011-03-23 19:08:35 PDT 
Not sure how to address the style bot warning about MAKE_UNIQUE_IDENTIFIER.
Comment 5 WebKit Review Bot 2011-03-23 19:10:09 PDT 
Attachment 86732 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/chromium/TraceEvent.h:39:  TRACE_EVENT_MAKE_UNIQUE_IDENTIFIER is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Kenneth Russell 2011-03-23 19:40:23 PDT 
Comment on attachment 86732 [details]
Patch

You should feel free to file a bug against check-webkit-style, but it looks like it doesn't handle multi-line #defines correctly. If you just make the TRACE_EVENT macro a single line it will clean up the style error. This is worth doing.
Comment 7 Nat Duca 2011-03-24 10:25:52 PDT 
Created attachment 86792 [details]
Patch
Comment 8 Kenneth Russell 2011-03-24 10:30:14 PDT 
Comment on attachment 86792 [details]
Patch

Looks great.
Comment 9 WebKit Commit Bot 2011-03-24 12:02:48 PDT 
Comment on attachment 86792 [details]
Patch

Clearing flags on attachment: 86792

Committed r81881: <http://trac.webkit.org/changeset/81881>
Comment 10 WebKit Commit Bot 2011-03-24 12:02:52 PDT 
All reviewed patches have been landed.  Closing bug.