From 9fdceb80b5f7c36cdbcd5861c737b40a73b430f6 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 27 Nov 2023 10:57:22 +0100 Subject: [PATCH] Add environment_construction poison This poison guards against accidental use of EnvironmentFactory and thus ensures low level WebRTC class would use utilities from propagated environment instead of accidentally using a default implementation. This poison extends and thus replaces default task queue poison. Bug: webrtc:15656 Change-Id: I577bef8af08b9c7dd649ad5a2284eb236e6f4a8f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328380 Reviewed-by: Mirko Bonadei Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#41247} --- BUILD.gn | 4 ++-- api/BUILD.gn | 4 ++-- api/environment/BUILD.gn | 5 +---- api/task_queue/BUILD.gn | 5 ++++- media/BUILD.gn | 2 +- pc/BUILD.gn | 4 ++++ sdk/BUILD.gn | 10 +++++----- sdk/android/BUILD.gn | 2 +- webrtc.gni | 8 +++++--- 9 files changed, 25 insertions(+), 19 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index baf8b0951d..571049f3e4 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -802,10 +802,10 @@ rtc_static_library("dcsctp") { group("poison_audio_codecs") { } -group("poison_default_task_queue") { +group("poison_default_echo_detector") { } -group("poison_default_echo_detector") { +group("poison_environment_construction") { } group("poison_software_video_codecs") { diff --git a/api/BUILD.gn b/api/BUILD.gn index e57092ee34..c187a7d296 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -54,7 +54,7 @@ rtc_source_set("enable_media_with_defaults") { visibility = [ "*" ] allow_poison = [ "audio_codecs", - "default_task_queue", + "environment_construction", "software_video_codecs", ] sources = [ @@ -77,7 +77,7 @@ rtc_source_set("enable_media_with_defaults") { if (!build_with_chromium) { rtc_library("create_peerconnection_factory") { visibility = [ "*" ] - allow_poison = [ "default_task_queue" ] + allow_poison = [ "environment_construction" ] sources = [ "create_peerconnection_factory.cc", "create_peerconnection_factory.h", diff --git a/api/environment/BUILD.gn b/api/environment/BUILD.gn index 19d52be3e5..c2b73b327e 100644 --- a/api/environment/BUILD.gn +++ b/api/environment/BUILD.gn @@ -21,10 +21,7 @@ rtc_source_set("environment") { rtc_library("environment_factory") { visibility = [ "*" ] - - # TODO: bugs.webrtc.org/15656 - introduce new poison for environment - # construction and consume default_task_queue poison with it. - allow_poison = [ "default_task_queue" ] + poisonous = [ "environment_construction" ] sources = [ "environment_factory.cc", "environment_factory.h", diff --git a/api/task_queue/BUILD.gn b/api/task_queue/BUILD.gn index e0e2e50514..246164c68b 100644 --- a/api/task_queue/BUILD.gn +++ b/api/task_queue/BUILD.gn @@ -83,7 +83,10 @@ rtc_library("task_queue_test") { rtc_library("default_task_queue_factory") { visibility = [ "*" ] if (!is_ios && !is_android) { - poisonous = [ "default_task_queue" ] + # Internally webrtc shouldn't rely on any specific TaskQueue implementation + # and should create TaskQueue using TaskQueueFactory interface. + # TaskQueueFactory interface can be propagated with Environment. + poisonous = [ "environment_construction" ] } sources = [ "default_task_queue_factory.h" ] deps = [ diff --git a/media/BUILD.gn b/media/BUILD.gn index d12c9296ae..6b8eaf700b 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -582,7 +582,7 @@ rtc_library("rtc_media_engine_defaults") { visibility = [ "*" ] allow_poison = [ "audio_codecs", - "default_task_queue", + "environment_construction", "software_video_codecs", ] sources = [ diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 1ff7b99713..dfb98ff2c4 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -739,6 +739,7 @@ rtc_library("media_protocol_names") { rtc_source_set("peerconnection") { # TODO(bugs.webrtc.org/13661): Reduce visibility if possible visibility = [ "*" ] # Used by Chromium and others + allow_poison = [ "environment_construction" ] cflags = [] sources = [] @@ -1496,6 +1497,7 @@ rtc_library("media_stream_observer") { rtc_source_set("peer_connection_factory") { # TODO(bugs.webrtc.org/13661): Reduce visibility if possible visibility = [ "*" ] # Known to be used externally + allow_poison = [ "environment_construction" ] sources = [ "peer_connection_factory.cc", "peer_connection_factory.h", @@ -1520,6 +1522,7 @@ rtc_source_set("peer_connection_factory") { "../api:rtp_parameters", "../api:scoped_refptr", "../api:sequence_checker", + "../api/environment:environment_factory", "../api/metronome", "../api/neteq:neteq_api", "../api/rtc_event_log:rtc_event_log", @@ -2061,6 +2064,7 @@ rtc_source_set("legacy_stats_collector_interface") { rtc_source_set("libjingle_peerconnection") { # TODO(bugs.webrtc.org/13661): Reduce visibility if possible visibility = [ "*" ] # Used by Chrome and others + allow_poison = [ "environment_construction" ] deps = [ ":peerconnection", diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn index bff9b91680..e28bdcc5a2 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -254,7 +254,7 @@ if (is_ios || is_mac) { visibility = [ "*" ] allow_poison = [ "audio_codecs", # TODO(bugs.webrtc.org/8396): Remove. - "default_task_queue", + "environment_construction", ] sources = [ "objc/components/renderer/opengl/RTCDisplayLinkTimer.h", @@ -482,7 +482,7 @@ if (is_ios || is_mac) { rtc_library("audio_device_objc") { visibility = [ "*" ] - allow_poison = [ "default_task_queue" ] + allow_poison = [ "environment_construction" ] sources = [ "objc/native/src/objc_audio_device.h", "objc/native/src/objc_audio_device.mm", @@ -515,7 +515,7 @@ if (is_ios || is_mac) { rtc_library("objc_audio_device_module") { visibility = [ "*" ] - allow_poison = [ "default_task_queue" ] + allow_poison = [ "environment_construction" ] sources = [ "objc/native/api/objc_audio_device_module.h", "objc/native/api/objc_audio_device_module.mm", @@ -598,7 +598,7 @@ if (is_ios || is_mac) { visibility = [ "*" ] allow_poison = [ "audio_codecs", # TODO(bugs.webrtc.org/8396): Remove. - "default_task_queue", + "environment_construction", ] sources = [ "objc/components/renderer/metal/RTCMTLI420Renderer.h", @@ -920,7 +920,7 @@ if (is_ios || is_mac) { visibility = [ "*" ] allow_poison = [ "audio_codecs", # TODO(bugs.webrtc.org/8396): Remove. - "default_task_queue", + "environment_construction", ] configs += [ "..:no_global_constructors" ] sources = [ diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index b050933aac..e8e8075a76 100644 --- a/sdk/android/BUILD.gn +++ b/sdk/android/BUILD.gn @@ -109,6 +109,7 @@ if (is_android) { visibility = [ "*" ] allow_poison = [ "audio_codecs", # TODO(bugs.webrtc.org/8396): Remove. + "environment_construction", "software_video_codecs", # TODO(bugs.webrtc.org/7925): Remove. ] public_deps = [ # no-presubmit-check TODO(webrtc:8603) @@ -826,7 +827,6 @@ if (current_os == "linux" || is_android) { ":generated_metrics_jni", ":native_api_jni", ":peerconnection_jni", - "../../pc:peerconnection", "../../rtc_base:stringutils", "../../system_wrappers:metrics", ] diff --git a/webrtc.gni b/webrtc.gni index 173d66c791..447aae4096 100644 --- a/webrtc.gni +++ b/webrtc.gni @@ -474,12 +474,14 @@ all_poison_types = [ # Encoders and decoders for specific audio codecs such as Opus and iSAC. "audio_codecs", - # Default task queue implementation. - "default_task_queue", - # Default echo detector implementation. "default_echo_detector", + # Implementations of the utilities exposed through `Environment`. + # Most webrtc classes must use propagated `Environment`. Only few top-level + # classes are allowed to create `Environment` from individual utilities. + "environment_construction", + # Software video codecs (VP8 and VP9 through libvpx). "software_video_codecs", ]