Bug 56965 - [chromium] Add traceEvents to compositor
Summary: [chromium] Add traceEvents to compositor
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
Depends on:
Reported: 2011-03-23 15:13 PDT by Nat Duca
Modified: 2011-03-24 12:02 PDT (History)
4 users (show)

See Also:

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

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]
Comment 2 Kenneth Russell 2011-03-23 17:46:07 PDT
Comment on attachment 86705 [details]

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]
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]

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]
Comment 8 Kenneth Russell 2011-03-24 10:30:14 PDT
Comment on attachment 86792 [details]

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

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.