Prepare for WebRTC perfetto integration [chromium/src : main]
0 views
Skip to first unread message
Mirko Bonadei (Gerrit)
Apr 24, 2024, 5:31:42 AMApr 24
to Evan Shrubsole, Mikhail Khokhlov, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Attention needed from Evan Shrubsole and Mikhail Khokhlov
Mirko Bonadei added 2 comments![]( "Open in Gerrit")
File third_party/webrtc_overrides/init_webrtc.h
Line 10, Patchset 2 (Latest):
// Initialize WebRTC. Call this explicitly to initialize WebRTC module
// before initializing the sandbox in Chrome.
RTC_EXPORT bool InitializeWebRtcModuleBeforeSandbox();
// Hooks up Chrome+WebRTC integration such as logging and tracing. Must be run
// after tracing is initialized.
RTC_EXPORT void InitializeWebRtcModule();Mirko Bonadei . unresolved
What is the error scenario in case one is not called or they are called in the wrong order?
I am worried about future refactoring that move code around at the call site.
File third_party/webrtc_overrides/init_webrtc.cc
Line 40, Patchset 2 (Latest):
void InitializeWebRtcTracing() {
#if defined(RTC_USE_PERFETTO)
webrtc::SetupEventTracer();
#else
webrtc::SetupEventTracer(&GetCategoryGroupEnabled, &AddTraceEvent);
#endif // defined(RTC_USE_PERFETTO)
}Mirko Bonadei . unresolved
Inline below at line 60?
Related details
Attention is currently required from:
- Evan Shrubsole
- Mikhail Khokhlov
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mikhail Khokhlov (Gerrit)
Apr 24, 2024, 6:58:33 AMApr 24
to Evan Shrubsole, Mirko Bonadei, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Attention needed from Evan Shrubsole
Mikhail Khokhlov voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Evan Shrubsole
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Evan Shrubsole (Gerrit)
Apr 24, 2024, 8:07:37 AMApr 24
to Mikhail Khokhlov, Mirko Bonadei, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Attention needed from Mirko Bonadei
Evan Shrubsole added 2 comments![]( "Open in Gerrit")
File third_party/webrtc_overrides/init_webrtc.h
Line 10, Patchset 2:
// Initialize WebRTC. Call this explicitly to initialize WebRTC module
// before initializing the sandbox in Chrome.
RTC_EXPORT bool InitializeWebRtcModuleBeforeSandbox();
// Hooks up Chrome+WebRTC integration such as logging and tracing. Must be run
// after tracing is initialized.
RTC_EXPORT void InitializeWebRtcModule();Mirko Bonadei . resolved
What is the error scenario in case one is not called or they are called in the wrong order?
I am worried about future refactoring that move code around at the call site.
Evan Shrubsole
There is no error scenario for the order of these two.
File third_party/webrtc_overrides/init_webrtc.cc
void InitializeWebRtcTracing() {
#if defined(RTC_USE_PERFETTO)
webrtc::SetupEventTracer();
#else
webrtc::SetupEventTracer(&GetCategoryGroupEnabled, &AddTraceEvent);
#endif // defined(RTC_USE_PERFETTO)
}Mirko Bonadei . resolved
Inline below at line 60?
Attention is currently required from:
- Mirko Bonadei
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Evan Shrubsole (Gerrit)
Apr 24, 2024, 9:32:42 AMApr 24
to Dave Tapuska, Mikhail Khokhlov, Mirko Bonadei, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Attention needed from Dave Tapuska and Mirko Bonadei
Evan Shrubsole added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/modules/modules_initializer.cc
Line 236, Patchset 3 (Latest):
::InitializeWebRtcModule();Evan Shrubsole . unresolved
To reviewer: I am open to moving this somewhere else, I was not sure where the best place was and thought at blink init made the most sense.
Related details
Attention is currently required from:
- Dave Tapuska
- Mirko Bonadei
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Apr 24, 2024, 10:07:43 AMApr 24
to Evan Shrubsole, Mikhail Khokhlov, Mirko Bonadei, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Attention needed from Evan Shrubsole and Mirko Bonadei
Dave Tapuska voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dave Tapuska
| Code-Review | +1 |
1 comment
File third_party/blink/renderer/modules/modules_initializer.cc
Line 236, Patchset 3 (Latest):
::InitializeWebRtcModule();Evan Shrubsole . resolved
To reviewer: I am open to moving this somewhere else, I was not sure where the best place was and thought at blink init made the most sense.
Attention is currently required from:
- Evan Shrubsole
- Mirko Bonadei
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mirko Bonadei (Gerrit)
Apr 24, 2024, 10:16:08 AMApr 24
to Evan Shrubsole, Dave Tapuska, Mikhail Khokhlov, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Attention needed from Evan Shrubsole
Mirko Bonadei added 1 comment![]( "Open in Gerrit")
File third_party/webrtc_overrides/init_webrtc.h
Line 10, Patchset 2:
// Initialize WebRTC. Call this explicitly to initialize WebRTC module
// before initializing the sandbox in Chrome.
RTC_EXPORT bool InitializeWebRtcModuleBeforeSandbox();
// Hooks up Chrome+WebRTC integration such as logging and tracing. Must be run
// after tracing is initialized.
RTC_EXPORT void InitializeWebRtcModule();Mirko Bonadei . unresolved
Evan ShrubsoleWhat is the error scenario in case one is not called or they are called in the wrong order?
I am worried about future refactoring that move code around at the call site.
There is no error scenario for the order of these two.
Mirko Bonadei
I no sure I am following. Does it mean it does't fail, just Perfetto not working?
Related details
Attention is currently required from:
- Evan Shrubsole
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Evan Shrubsole (Gerrit)
Apr 24, 2024, 10:29:18 AMApr 24
to Dave Tapuska, Mikhail Khokhlov, Mirko Bonadei, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Attention needed from Mirko Bonadei
Evan Shrubsole added 1 comment![]( "Open in Gerrit")
File third_party/webrtc_overrides/init_webrtc.h
Line 10, Patchset 2:
// Initialize WebRTC. Call this explicitly to initialize WebRTC module
// before initializing the sandbox in Chrome.
RTC_EXPORT bool InitializeWebRtcModuleBeforeSandbox();
// Hooks up Chrome+WebRTC integration such as logging and tracing. Must be run
// after tracing is initialized.
RTC_EXPORT void InitializeWebRtcModule();Mirko Bonadei . unresolved
Evan ShrubsoleWhat is the error scenario in case one is not called or they are called in the wrong order?
I am worried about future refactoring that move code around at the call site.
Mirko BonadeiThere is no error scenario for the order of these two.
I no sure I am following. Does it mean it does't fail, just Perfetto not working?
Evan Shrubsole
Exactly, otherwise perfetto traces won't reach Chrome. I'll clarify.
Related details
Attention is currently required from:
- Mirko Bonadei
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mirko Bonadei (Gerrit)
Apr 24, 2024, 10:30:06 AMApr 24
to Evan Shrubsole, Dave Tapuska, Mikhail Khokhlov, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Attention needed from Evan Shrubsole
Mirko Bonadei voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Evan Shrubsole
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mirko Bonadei (Gerrit)
Apr 24, 2024, 10:30:25 AMApr 24
to Evan Shrubsole, Dave Tapuska, Mikhail Khokhlov, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Attention needed from Evan Shrubsole
Mirko Bonadei added 1 comment![]( "Open in Gerrit")
File third_party/webrtc_overrides/init_webrtc.h
Line 10, Patchset 2:
// Initialize WebRTC. Call this explicitly to initialize WebRTC module
// before initializing the sandbox in Chrome.
RTC_EXPORT bool InitializeWebRtcModuleBeforeSandbox();
// Hooks up Chrome+WebRTC integration such as logging and tracing. Must be run
// after tracing is initialized.
RTC_EXPORT void InitializeWebRtcModule();Mirko Bonadei . resolved
Evan ShrubsoleWhat is the error scenario in case one is not called or they are called in the wrong order?
I am worried about future refactoring that move code around at the call site.
Mirko BonadeiThere is no error scenario for the order of these two.
Evan ShrubsoleI no sure I am following. Does it mean it does't fail, just Perfetto not working?
Exactly, otherwise perfetto traces won't reach Chrome. I'll clarify.
Mirko Bonadei
Thanks.
Related details
Attention is currently required from:
- Evan Shrubsole
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Evan Shrubsole (Gerrit)
Apr 24, 2024, 10:57:18 AMApr 24
to Mirko Bonadei, Dave Tapuska, Mikhail Khokhlov, Chromium LUCI CQ, chromium...@chromium.org, blink-...@chromium.org
Evan Shrubsole voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Apr 24, 2024, 11:38:33 AMApr 24
to Evan Shrubsole, Mirko Bonadei, Dave Tapuska, Mikhail Khokhlov, chromium...@chromium.org, blink-...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Prepare for WebRTC perfetto integration
This CL makes some changes to webrtc_overrides/init_webrtc,
* Splits initialization into 2 methods to make it clear that
one must occur before sandbox.
* The second method must occur after logging and tracing are prepared,
and may occur after sandbox.
Bug: webrtc:15917
Change-Id: I4597fc49aa6908b3654f9471cbab3f04e13b7761
Reviewed-by: Mirko Bonadei <mbon...@chromium.org>
Commit-Queue: Evan Shrubsole <es...@google.com>
Reviewed-by: Dave Tapuska <dtap...@chromium.org>
Reviewed-by: Mikhail Khokhlov <khok...@google.com>
Cr-Commit-Position: refs/heads/main@{#1291885}
Files:
- M content/app/content_main_runner_impl.cc
- M content/renderer/renderer_main.cc
- M third_party/blink/renderer/modules/modules_initializer.cc
- M third_party/webrtc_overrides/init_webrtc.cc
- M third_party/webrtc_overrides/init_webrtc.h
Change size: S
Delta: 5 files changed, 27 insertions(+), 8 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Mikhail Khokhlov, +1 by Dave Tapuska, +1 by Mirko Bonadei
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages