Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
870053006:
Provides a second version of addTraceEvent that accepts a timestamp. This is in preparation for the… (Closed)
|
Created:
5 years, 10 months ago by charliea Modified:
5 years, 10 months ago Reviewers:
jam CC:
chromium-reviews, darin-cc_chromium.org, jam, sullivan, nduca, Nate Chapin Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionProvides a second version of addTraceEvent that accepts a timestamp when #WEBKIT_HAS_NEW_ADD_TRACE_EVENT_WITH_TIMESTAMP is defined. This is in preparation for adding the Blink code that writes trace events with user-supplied timestamps.
BUG=345845
Committed: https://crrev.com/4f512f95d91036a2b7765f9b9125a0d6e507fb03
Cr-Commit-Position: refs/heads/master@{#313874}
Patch Set 1 #Patch Set 2 : Removed pre-processor branching at japhet's suggestion. #
Total comments: 4
Patch Set 3 : Changes based on code review. #Patch Set 4 : #Patch Set 5 : Changed to use time based on TimeTicks (monotonicallyIncreasingTime) rather than Time (currentTime). #Messages
Total messages: 30 (12 generated)
charliea@google.com changed reviewers: + nduca@chromium.org
charliea@google.com changed reviewers: + japhet@chromium.org - nduca@chromium.org
charliea@google.com changed reviewers: + jam@chromium.org - japhet@chromium.org
japhet@ said that it looked reasonable, but didn't know enough to feel comfortable giving an LGTM
will the blink code be updated to call the version with the most parameters, and then you'll remove the other ones? https://codereview.chromium.org/870053006/diff/20001/content/child/blink_plat... File content/child/blink_platform_impl.h (right): https://codereview.chromium.org/870053006/diff/20001/content/child/blink_plat... content/child/blink_platform_impl.h:102: nit: remove. when we implement an interface, we add the methods without spaces in between https://codereview.chromium.org/870053006/diff/20001/content/child/blink_plat... content/child/blink_platform_impl.h:150: ditto
On 2015/01/28 23:31:09, jam wrote: > will the blink code be updated to call the version with the most parameters, and > then you'll remove the other ones? Yep, exactly.
Deleted whitespace between declarations. https://codereview.chromium.org/870053006/diff/20001/content/child/blink_plat... File content/child/blink_platform_impl.h (right): https://codereview.chromium.org/870053006/diff/20001/content/child/blink_plat... content/child/blink_platform_impl.h:102: On 2015/01/28 23:31:09, jam wrote: > nit: remove. when we implement an interface, we add the methods without spaces > in between Done. https://codereview.chromium.org/870053006/diff/20001/content/child/blink_plat... content/child/blink_platform_impl.h:150: On 2015/01/28 23:31:09, jam wrote: > ditto Done.
lgtm
The CQ bit was checked by charliea@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/870053006/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by charliea@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/870053006/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by charliea@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/870053006/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by charliea@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/870053006/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg_tests_recipe on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
The CQ bit was checked by charliea@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/870053006/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/4f512f95d91036a2b7765f9b9125a0d6e507fb03 Cr-Commit-Position: refs/heads/master@{#313874} |
