[tracing] Remove TrackRegistry::InitializeInstance in prod [chromium/src : main]
0 views
Skip to first unread message
Etienne Pierre-Doray (Gerrit)
Aug 25, 2025, 4:43:40 PMAug 25
to Chromium LUCI CQ, chromium...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Joe Mason
Etienne Pierre-Doray voted and added 1 comment![]( "Open in Gerrit")
Votes added by Etienne Pierre-Doray
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Joe Mason
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Aug 26, 2025, 11:44:54 AMAug 26
to AyeAye, Chromium LUCI CQ, chromium...@chromium.org, android-web...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Etienne Pierre-Doray and Joe Mason
Message from Etienne Pierre-Doray![]( "Open in Gerrit")
Attention is currently required from:
- Etienne Pierre-Doray
- Joe Mason
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joe Mason (Gerrit)
Aug 26, 2025, 12:41:24 PMAug 26
to Etienne Pierre-Doray, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, android-web...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Etienne Pierre-Doray
Joe Mason voted and added 3 comments![]( "Open in Gerrit")
Votes added by Joe Mason
| Code-Review | +1 |
3 comments
File content/browser/tracing/tracing_end_to_end_browsertest.cc
Line 482, Patchset 4 (Latest):
tracing::TrackNameRecorder::SetRecordHostAppPackageName(true);Joe Mason . unresolved
Nit: please use a scoper (eg. absl::Cleanup) to reset to the initial value at the end of the test. (Probably need to add a static getter to TrackNameRecorder.)
File services/tracing/public/cpp/perfetto/track_name_recorder.h
Line 88, Patchset 4 (Latest):
static bool record_host_app_package_name_;Joe Mason . resolved
Making this static means that it won't get reset to `false` after each test.
Although I guess that's not a regression since TrackNameRecorder wasn't reset between tests either. Nevermind.
File services/tracing/public/cpp/perfetto/track_name_recorder.cc
Line 168, Patchset 4 (Latest):
DCHECK(perfetto::Tracing::IsInitialized());Joe Mason . unresolved
Nit: current guidance is to use CHECK here since this isn't in performance-critical code.
Related details
Attention is currently required from:
- Etienne Pierre-Doray
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Aug 26, 2025, 4:16:15 PMAug 26
to AyeAye, Chromium LUCI CQ, chromium...@chromium.org, android-web...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Etienne Pierre-Doray voted and added 2 comments![]( "Open in Gerrit")
Votes added by Etienne Pierre-Doray
| Commit-Queue | +1 |
2 comments
File content/browser/tracing/tracing_end_to_end_browsertest.cc
Line 482, Patchset 4:
tracing::TrackNameRecorder::SetRecordHostAppPackageName(true);Joe Mason . resolved
Nit: please use a scoper (eg. absl::Cleanup) to reset to the initial value at the end of the test. (Probably need to add a static getter to TrackNameRecorder.)
Etienne Pierre-Doray
I don't think we need to worry about scoping things in browser tests (for example most of tracing machinery involved here isn't well isolated, unlike TracingEnvironment in unittests)
File services/tracing/public/cpp/perfetto/track_name_recorder.cc
Nit: current guidance is to use CHECK here since this isn't in performance-critical code.
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Aug 26, 2025, 4:16:45 PMAug 26
to Nate Fischer, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, android-web...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Nate Fischer
Etienne Pierre-Doray added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 5 (Latest):
Etienne Pierre-Doray . resolved
+ntfschr@ for android_webview/lib
Related details
Attention is currently required from:
- Nate Fischer
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nate Fischer (Gerrit)
Aug 26, 2025, 5:41:03 PMAug 26
to Etienne Pierre-Doray, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, android-web...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Attention needed from Etienne Pierre-Doray
Nate Fischer voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Etienne Pierre-Doray
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Aug 26, 2025, 6:02:41 PMAug 26
to Nate Fischer, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, android-web...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Etienne Pierre-Doray voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Aug 26, 2025, 6:06:14 PMAug 26
to Etienne Pierre-Doray, Nate Fischer, AyeAye, chromium...@chromium.org, android-web...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[tracing] Remove TrackRegistry::InitializeInstance in prod
This CL removes TrackRegistry::InitializeInstance (which is meant
to be internal to perfetto) in
InitTracingPostFeatureList; the comment in stale,
and a few changes in TrackNameRecorder are enough to
work around.
Change-Id: Ib67c3fdc2432eabbdcf9bd5fe7cd1e4c560653ac
Commit-Queue: Etienne Pierre-Doray <etie...@chromium.org>
Reviewed-by: Joe Mason <joenot...@google.com>
Reviewed-by: Nate Fischer <ntf...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1506794}
Files:
- M android_webview/lib/aw_main_delegate.cc
- M content/browser/tracing/tracing_end_to_end_browsertest.cc
- M services/tracing/public/cpp/perfetto/track_name_recorder.cc
- M services/tracing/public/cpp/perfetto/track_name_recorder.h
- M services/tracing/public/cpp/trace_startup.cc
Change size: S
Delta: 5 files changed, 14 insertions(+), 28 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Joe Mason, +1 by Nate Fischer
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sarah Verduzco (Gerrit)
Aug 26, 2025, 6:07:26 PMAug 26
to Etienne Pierre-Doray, Chromium LUCI CQ, Nate Fischer, AyeAye, chromium...@chromium.org, android-web...@chromium.org, spang...@chromium.org, tracing...@chromium.org, wfh+...@chromium.org
Sarah Verduzco added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages