Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(467)

Issue 2200113003: [Tracing] V8 Tracing Controller - Fix async trace event bug (Closed)

Created:
4 years, 4 months ago by rskang
Modified:
4 years, 4 months ago
Reviewers:
mattloring, lpy, fmeawad, Yang, fadi
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[Tracing] V8 Tracing Controller - Fix async trace event bug Usage of hex IO manipulator for async event IDs corrupts future decimal number outputs. BUG=v8:5261 Committed: https://crrev.com/94ad974df8434844ea179f0a57b52b94dc7bc760 Cr-Commit-Position: refs/heads/master@{#38331}

Patch Set 1 #

Patch Set 2 : Attempt fix for windows pointer output #

Patch Set 3 : Fix windows pointer output #

Total comments: 4

Patch Set 4 : Merge async test as part of TestJSONTraceWriter #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -6 lines) Patch
M src/libplatform/tracing/trace-writer.cc View 1 chunk +2 lines, -1 line 0 comments Download
M test/cctest/libplatform/test-tracing.cc View 1 2 3 2 chunks +5 lines, -5 lines 0 comments Download

Messages

Total messages: 37 (25 generated)
rskang
JSONTraceWriter output for async trace events had a bug: Usage of hex IO manipulator for ...
4 years, 4 months ago (2016-08-03 00:49:43 UTC) #14
mattloring
lgtm
4 years, 4 months ago (2016-08-03 02:48:25 UTC) #15
mattloring
lgtm
4 years, 4 months ago (2016-08-03 02:48:27 UTC) #16
fmeawad
On 2016/08/03 02:48:27, mattloring wrote: > lgtm Please create a bug, and set BUG=v8:...
4 years, 4 months ago (2016-08-03 06:41:07 UTC) #17
fmeawad
LGTM with comments. https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform/test-tracing.cc File test/cctest/libplatform/test-tracing.cc (right): https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform/test-tracing.cc#newcode312 test/cctest/libplatform/test-tracing.cc:312: TEST(TestAsyncTraceEvent) { All test names are ...
4 years, 4 months ago (2016-08-03 06:48:03 UTC) #19
lpy
Also keep 80-column in CL description please.
4 years, 4 months ago (2016-08-03 06:49:32 UTC) #20
rskang
Fixed 80-line character limit for CL description. Added BUG id. https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform/test-tracing.cc File test/cctest/libplatform/test-tracing.cc (right): https://codereview.chromium.org/2200113003/diff/40001/test/cctest/libplatform/test-tracing.cc#newcode312 ...
4 years, 4 months ago (2016-08-03 20:41:46 UTC) #27
rskang
Added @Yang to reviewers.
4 years, 4 months ago (2016-08-04 00:57:37 UTC) #29
Yang
On 2016/08/04 00:57:37, rskang wrote: > Added @Yang to reviewers. lgtm.
4 years, 4 months ago (2016-08-04 06:58:24 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2200113003/60001
4 years, 4 months ago (2016-08-04 08:39:23 UTC) #33
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years, 4 months ago (2016-08-04 08:48:27 UTC) #35
commit-bot: I haz the power
4 years, 4 months ago (2016-08-04 08:49:28 UTC) #37
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/94ad974df8434844ea179f0a57b52b94dc7bc760
Cr-Commit-Position: refs/heads/master@{#38331}

Powered by Google App Engine
This is Rietveld 408576698