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

Issue 1088673003: CompressedStringDataSink for trace_controller. (Closed)

Created:
5 years, 8 months ago by shatch
Modified:
5 years, 8 months ago
Reviewers:
dsinclair, nduca, no sievers
CC:
chromium-reviews, darin-cc_chromium.org, jam, oysteine, tracing+reviews_chromium.org, wfh+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Added a new CompressedStringDataSink for compressing traces via the trace controller. Also added a TraceDataEndpoint class that I'm thinking can be used to decouple the creation of the final trace (since there's some special logic in there for adding '{}'s and some specific names like 'traceEvent') from where it gets written. This is used right now by the new BackgroundTracingManager for Slow Reports and Deep Reports, but we could also refactor the existing String and File sinks this way as a follow up CL (WIP here: https://codereview.chromium.org/1002103004/). Then for Deep Reports we can specify the data sink like so: TraceController::DisableRecording( TraceController::CreateCompressedStringSink( TraceController::CreateFileEndpoint(...))); Committed: https://crrev.com/329f14fe8729f500bad122cafba31ff075b788dd Cr-Commit-Position: refs/heads/master@{#326783}

Patch Set 1 : #

Patch Set 2 : Added test. #

Patch Set 3 : Moved to new file. #

Patch Set 4 : Fix win compile and tests. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+368 lines, -133 lines) Patch
M content/browser/tracing/tracing_controller_browsertest.cc View 1 2 3 4 chunks +68 lines, -0 lines 0 comments Download
M content/browser/tracing/tracing_controller_impl.cc View 1 2 2 chunks +0 lines, -133 lines 0 comments Download
A content/browser/tracing/tracing_controller_impl_data_sinks.cc View 1 2 3 1 chunk +280 lines, -0 lines 0 comments Download
M content/content_browser.gypi View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/browser/tracing_controller.h View 1 chunk +18 lines, -0 lines 0 comments Download

Messages

Total messages: 31 (17 generated)
shatch
Hey Nat, added compression in the trace controller, ptal
5 years, 8 months ago (2015-04-16 19:36:56 UTC) #9
nduca
seems cool but this is getting big enough maybe time to put it in its ...
5 years, 8 months ago (2015-04-21 17:10:20 UTC) #10
nduca
actually lgtm maybe have dan give you a local-timezone final review once you've pulled it ...
5 years, 8 months ago (2015-04-21 17:11:07 UTC) #11
shatch
+dsinclair ptal!
5 years, 8 months ago (2015-04-21 19:18:34 UTC) #13
dsinclair
lgtm. If there is no bug, remove the BUG= line.
5 years, 8 months ago (2015-04-21 19:21:22 UTC) #14
shatch
+sievers Hi Daniel, can I get a stamp for content/public/browser/tracing_controller.h?
5 years, 8 months ago (2015-04-21 19:32:06 UTC) #16
no sievers
On 2015/04/21 19:32:06, shatch wrote: > +sievers > > Hi Daniel, can I get a ...
5 years, 8 months ago (2015-04-22 21:35:19 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1088673003/150004
5 years, 8 months ago (2015-04-23 16:42:49 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/10575)
5 years, 8 months ago (2015-04-23 17:30:10 UTC) #22
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1088673003/170001
5 years, 8 months ago (2015-04-23 20:40:41 UTC) #25
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 8 months ago (2015-04-23 21:45:26 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1088673003/170001
5 years, 8 months ago (2015-04-24 13:41:39 UTC) #29
commit-bot: I haz the power
Committed patchset #4 (id:170001)
5 years, 8 months ago (2015-04-24 13:42:32 UTC) #30
commit-bot: I haz the power
5 years, 8 months ago (2015-04-24 13:43:42 UTC) #31
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/329f14fe8729f500bad122cafba31ff075b788dd
Cr-Commit-Position: refs/heads/master@{#326783}

Powered by Google App Engine
This is Rietveld 408576698