diff --git a/api/ice_transport_interface.h b/api/ice_transport_interface.h index f202658acf..c82027a427 100644 --- a/api/ice_transport_interface.h +++ b/api/ice_transport_interface.h @@ -27,6 +27,7 @@ class IceControllerFactoryInterface; } // namespace cricket namespace webrtc { +class WebRtcKeyValueConfig; // An ICE transport, as represented to the outside world. // This object is refcounted, and is therefore alive until the @@ -83,6 +84,11 @@ struct IceTransportInit final { return ice_controller_factory_; } + const WebRtcKeyValueConfig* field_trials() { return field_trials_; } + void set_field_trials(const WebRtcKeyValueConfig* field_trials) { + field_trials_ = field_trials; + } + private: cricket::PortAllocator* port_allocator_ = nullptr; AsyncDnsResolverFactoryInterface* async_dns_resolver_factory_ = nullptr; @@ -90,6 +96,7 @@ struct IceTransportInit final { AsyncResolverFactory* async_resolver_factory_ = nullptr; RtcEventLog* event_log_ = nullptr; cricket::IceControllerFactoryInterface* ice_controller_factory_ = nullptr; + const WebRtcKeyValueConfig* field_trials_ = nullptr; // TODO(https://crbug.com/webrtc/12657): Redesign to have const members. }; diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 66da247ade..758f8bfa33 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -97,6 +97,7 @@ rtc_library("rtc_p2p") { "../api/rtc_event_log", "../api/task_queue", "../api/transport:enums", + "../api/transport:field_trial_based_config", "../api/transport:stun_types", "../logging:ice_log", "../rtc_base", diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc index 1d674e2bcb..efc2143977 100644 --- a/p2p/base/connection.cc +++ b/p2p/base/connection.cc @@ -33,7 +33,6 @@ #include "rtc_base/string_utils.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/third_party/base64/base64.h" -#include "system_wrappers/include/field_trial.h" namespace { @@ -191,8 +190,7 @@ void ConnectionRequest::Prepare(StunMessage* request) { request->AddAttribute(std::make_unique( STUN_ATTR_GOOG_NETWORK_INFO, network_info)); - if (webrtc::field_trial::IsEnabled( - "WebRTC-PiggybackIceCheckAcknowledgement") && + if (connection_->field_trials_->piggyback_ice_check_acknowledgement && connection_->last_ping_id_received()) { request->AddAttribute(std::make_unique( STUN_ATTR_GOOG_LAST_ICE_CHECK_RECEIVED, @@ -605,8 +603,7 @@ void Connection::HandleStunBindingOrGoogPingRequest(IceMessage* msg) { RTC_DCHECK_RUN_ON(network_thread_); // This connection should now be receiving. ReceivedPing(msg->transaction_id()); - if (webrtc::field_trial::IsEnabled("WebRTC-ExtraICEPing") && - last_ping_response_received_ == 0) { + if (field_trials_->extra_ice_ping && last_ping_response_received_ == 0) { if (local_candidate().type() == RELAY_PORT_TYPE || local_candidate().type() == PRFLX_PORT_TYPE || remote_candidate().type() == RELAY_PORT_TYPE || @@ -695,8 +692,7 @@ void Connection::HandleStunBindingOrGoogPingRequest(IceMessage* msg) { } } - if (webrtc::field_trial::IsEnabled( - "WebRTC-PiggybackIceCheckAcknowledgement")) { + if (field_trials_->piggyback_ice_check_acknowledgement) { HandlePiggybackCheckAcknowledgementIfAny(msg); } } diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 5d7f895b61..a8d84b987d 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -25,6 +25,7 @@ #include "api/async_dns_resolver.h" #include "api/candidate.h" #include "api/task_queue/queued_task.h" +#include "api/webrtc_key_value_config.h" #include "logging/rtc_event_log/ice_logger.h" #include "p2p/base/basic_async_resolver_factory.h" #include "p2p/base/basic_ice_controller.h" @@ -44,7 +45,6 @@ #include "rtc_base/third_party/sigslot/sigslot.h" #include "rtc_base/time_utils.h" #include "rtc_base/trace_event.h" -#include "system_wrappers/include/field_trial.h" #include "system_wrappers/include/metrics.h" namespace { @@ -60,12 +60,15 @@ cricket::PortInterface::CandidateOrigin GetOrigin( return cricket::PortInterface::ORIGIN_OTHER_PORT; } -uint32_t GetWeakPingIntervalInFieldTrial() { - uint32_t weak_ping_interval = ::strtoul( - webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(), - nullptr, 10); - if (weak_ping_interval) { - return static_cast(weak_ping_interval); +uint32_t GetWeakPingIntervalInFieldTrial( + const webrtc::WebRtcKeyValueConfig* field_trials) { + if (field_trials != nullptr) { + uint32_t weak_ping_interval = + ::strtoul(field_trials->Lookup("WebRTC-StunInterPacketDelay").c_str(), + nullptr, 10); + if (weak_ping_interval) { + return static_cast(weak_ping_interval); + } } return cricket::WEAK_PING_INTERVAL; } @@ -118,25 +121,28 @@ std::unique_ptr P2PTransportChannel::Create( transport_name, component, init.port_allocator(), nullptr, std::make_unique( init.async_resolver_factory()), - init.event_log(), init.ice_controller_factory())); + init.event_log(), init.ice_controller_factory(), init.field_trials())); } else { return absl::WrapUnique(new P2PTransportChannel( transport_name, component, init.port_allocator(), init.async_dns_resolver_factory(), nullptr, init.event_log(), - init.ice_controller_factory())); + init.ice_controller_factory(), init.field_trials())); } } -P2PTransportChannel::P2PTransportChannel(const std::string& transport_name, - int component, - PortAllocator* allocator) +P2PTransportChannel::P2PTransportChannel( + const std::string& transport_name, + int component, + PortAllocator* allocator, + const webrtc::WebRtcKeyValueConfig* field_trials) : P2PTransportChannel(transport_name, component, allocator, /* async_dns_resolver_factory= */ nullptr, /* owned_dns_resolver_factory= */ nullptr, /* event_log= */ nullptr, - /* ice_controller_factory= */ nullptr) {} + /* ice_controller_factory= */ nullptr, + field_trials) {} // Private constructor, called from Create() P2PTransportChannel::P2PTransportChannel( @@ -147,7 +153,8 @@ P2PTransportChannel::P2PTransportChannel( std::unique_ptr owned_dns_resolver_factory, webrtc::RtcEventLog* event_log, - IceControllerFactoryInterface* ice_controller_factory) + IceControllerFactoryInterface* ice_controller_factory, + const webrtc::WebRtcKeyValueConfig* field_trials) : transport_name_(transport_name), component_(component), allocator_(allocator), @@ -165,6 +172,7 @@ P2PTransportChannel::P2PTransportChannel( ice_role_(ICEROLE_UNKNOWN), tiebreaker_(0), gathering_state_(kIceGatheringNew), + weak_ping_interval_(GetWeakPingIntervalInFieldTrial(field_trials)), config_(RECEIVING_TIMEOUT, BACKUP_CONNECTION_PING_INTERVAL, GATHER_ONCE /* continual_gathering_policy */, @@ -175,7 +183,6 @@ P2PTransportChannel::P2PTransportChannel( RECEIVING_SWITCHING_DELAY) { TRACE_EVENT0("webrtc", "P2PTransportChannel::P2PTransportChannel"); RTC_DCHECK(allocator_ != nullptr); - weak_ping_interval_ = GetWeakPingIntervalInFieldTrial(); // Validate IceConfig even for mostly built-in constant default values in case // we change them. RTC_DCHECK(ValidateIceConfig(config_).ok()); @@ -191,6 +198,8 @@ P2PTransportChannel::P2PTransportChannel( this, &P2PTransportChannel::OnCandidateFilterChanged); ice_event_log_.set_event_log(event_log); + ParseFieldTrials(field_trials); + IceControllerFactoryArgs args{ [this] { return GetState(); }, [this] { return GetIceRole(); }, [this](const Connection* connection) { @@ -199,8 +208,9 @@ P2PTransportChannel::P2PTransportChannel( return IsPortPruned(connection->port()) || IsRemoteCandidatePruned(connection->remote_candidate()); }, - &field_trials_, - webrtc::field_trial::FindFullName("WebRTC-IceControllerFieldTrials")}; + &ice_field_trials_, + field_trials ? field_trials->Lookup("WebRTC-IceControllerFieldTrials") + : ""}; if (ice_controller_factory != nullptr) { ice_controller_ = ice_controller_factory->Create(args); } else { @@ -267,7 +277,7 @@ void P2PTransportChannel::AddConnection(Connection* connection) { had_connection_ = true; connection->set_ice_event_log(&ice_event_log_); - connection->SetIceFieldTrials(&field_trials_); + connection->SetIceFieldTrials(&ice_field_trials_); LogCandidatePairConfig(connection, webrtc::IceCandidatePairConfigType::kAdded); @@ -685,72 +695,6 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { << config.stun_keepalive_interval_or_default(); } - if (webrtc::field_trial::IsEnabled("WebRTC-ExtraICEPing")) { - RTC_LOG(LS_INFO) << "Set WebRTC-ExtraICEPing: Enabled"; - } - if (webrtc::field_trial::IsEnabled("WebRTC-TurnAddMultiMapping")) { - RTC_LOG(LS_INFO) << "Set WebRTC-TurnAddMultiMapping: Enabled"; - } - - webrtc::StructParametersParser::Create( - // go/skylift-light - "skip_relay_to_non_relay_connections", - &field_trials_.skip_relay_to_non_relay_connections, - // Limiting pings sent. - "max_outstanding_pings", &field_trials_.max_outstanding_pings, - // Delay initial selection of connection. - "initial_select_dampening", &field_trials_.initial_select_dampening, - // Delay initial selection of connections, that are receiving. - "initial_select_dampening_ping_received", - &field_trials_.initial_select_dampening_ping_received, - // Reply that we support goog ping. - "announce_goog_ping", &field_trials_.announce_goog_ping, - // Use goog ping if remote support it. - "enable_goog_ping", &field_trials_.enable_goog_ping, - // How fast does a RTT sample decay. - "rtt_estimate_halftime_ms", &field_trials_.rtt_estimate_halftime_ms, - // Make sure that nomination reaching ICE controlled asap. - "send_ping_on_switch_ice_controlling", - &field_trials_.send_ping_on_switch_ice_controlling, - // Make sure that nomination reaching ICE controlled asap. - "send_ping_on_selected_ice_controlling", - &field_trials_.send_ping_on_selected_ice_controlling, - // Reply to nomination ASAP. - "send_ping_on_nomination_ice_controlled", - &field_trials_.send_ping_on_nomination_ice_controlled, - // Allow connections to live untouched longer that 30s. - "dead_connection_timeout_ms", &field_trials_.dead_connection_timeout_ms, - // Stop gathering on strongly connected. - "stop_gather_on_strongly_connected", - &field_trials_.stop_gather_on_strongly_connected) - ->Parse(webrtc::field_trial::FindFullName("WebRTC-IceFieldTrials")); - - if (field_trials_.dead_connection_timeout_ms < 30000) { - RTC_LOG(LS_WARNING) << "dead_connection_timeout_ms set to " - << field_trials_.dead_connection_timeout_ms - << " increasing it to 30000"; - field_trials_.dead_connection_timeout_ms = 30000; - } - - if (field_trials_.skip_relay_to_non_relay_connections) { - RTC_LOG(LS_INFO) << "Set skip_relay_to_non_relay_connections"; - } - - if (field_trials_.max_outstanding_pings.has_value()) { - RTC_LOG(LS_INFO) << "Set max_outstanding_pings: " - << *field_trials_.max_outstanding_pings; - } - - if (field_trials_.initial_select_dampening.has_value()) { - RTC_LOG(LS_INFO) << "Set initial_select_dampening: " - << *field_trials_.initial_select_dampening; - } - - if (field_trials_.initial_select_dampening_ping_received.has_value()) { - RTC_LOG(LS_INFO) << "Set initial_select_dampening_ping_received: " - << *field_trials_.initial_select_dampening_ping_received; - } - webrtc::BasicRegatheringController::Config regathering_config; regathering_config.regather_on_failed_networks_interval = config_.regather_on_failed_networks_interval_or_default(); @@ -761,18 +705,95 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { ice_controller_->SetIceConfig(config_); + RTC_DCHECK(ValidateIceConfig(config_).ok()); +} + +void P2PTransportChannel::ParseFieldTrials( + const webrtc::WebRtcKeyValueConfig* field_trials) { + if (field_trials == nullptr) { + return; + } + + if (field_trials->IsEnabled("WebRTC-ExtraICEPing")) { + RTC_LOG(LS_INFO) << "Set WebRTC-ExtraICEPing: Enabled"; + } + if (field_trials->IsEnabled("WebRTC-TurnAddMultiMapping")) { + RTC_LOG(LS_INFO) << "Set WebRTC-TurnAddMultiMapping: Enabled"; + } + + webrtc::StructParametersParser::Create( + // go/skylift-light + "skip_relay_to_non_relay_connections", + &ice_field_trials_.skip_relay_to_non_relay_connections, + // Limiting pings sent. + "max_outstanding_pings", &ice_field_trials_.max_outstanding_pings, + // Delay initial selection of connection. + "initial_select_dampening", &ice_field_trials_.initial_select_dampening, + // Delay initial selection of connections, that are receiving. + "initial_select_dampening_ping_received", + &ice_field_trials_.initial_select_dampening_ping_received, + // Reply that we support goog ping. + "announce_goog_ping", &ice_field_trials_.announce_goog_ping, + // Use goog ping if remote support it. + "enable_goog_ping", &ice_field_trials_.enable_goog_ping, + // How fast does a RTT sample decay. + "rtt_estimate_halftime_ms", &ice_field_trials_.rtt_estimate_halftime_ms, + // Make sure that nomination reaching ICE controlled asap. + "send_ping_on_switch_ice_controlling", + &ice_field_trials_.send_ping_on_switch_ice_controlling, + // Make sure that nomination reaching ICE controlled asap. + "send_ping_on_selected_ice_controlling", + &ice_field_trials_.send_ping_on_selected_ice_controlling, + // Reply to nomination ASAP. + "send_ping_on_nomination_ice_controlled", + &ice_field_trials_.send_ping_on_nomination_ice_controlled, + // Allow connections to live untouched longer that 30s. + "dead_connection_timeout_ms", + &ice_field_trials_.dead_connection_timeout_ms, + // Stop gathering on strongly connected. + "stop_gather_on_strongly_connected", + &ice_field_trials_.stop_gather_on_strongly_connected) + ->Parse(field_trials->Lookup("WebRTC-IceFieldTrials")); + + if (ice_field_trials_.dead_connection_timeout_ms < 30000) { + RTC_LOG(LS_WARNING) << "dead_connection_timeout_ms set to " + << ice_field_trials_.dead_connection_timeout_ms + << " increasing it to 30000"; + ice_field_trials_.dead_connection_timeout_ms = 30000; + } + + if (ice_field_trials_.skip_relay_to_non_relay_connections) { + RTC_LOG(LS_INFO) << "Set skip_relay_to_non_relay_connections"; + } + + if (ice_field_trials_.max_outstanding_pings.has_value()) { + RTC_LOG(LS_INFO) << "Set max_outstanding_pings: " + << *ice_field_trials_.max_outstanding_pings; + } + + if (ice_field_trials_.initial_select_dampening.has_value()) { + RTC_LOG(LS_INFO) << "Set initial_select_dampening: " + << *ice_field_trials_.initial_select_dampening; + } + + if (ice_field_trials_.initial_select_dampening_ping_received.has_value()) { + RTC_LOG(LS_INFO) + << "Set initial_select_dampening_ping_received: " + << *ice_field_trials_.initial_select_dampening_ping_received; + } + // DSCP override, allow user to specify (any) int value // that will be used for tagging all packets. webrtc::StructParametersParser::Create("override_dscp", - &field_trials_.override_dscp) - ->Parse(webrtc::field_trial::FindFullName("WebRTC-DscpFieldTrial")); + &ice_field_trials_.override_dscp) + ->Parse(field_trials->Lookup("WebRTC-DscpFieldTrial")); - if (field_trials_.override_dscp) { - SetOption(rtc::Socket::OPT_DSCP, *field_trials_.override_dscp); + if (ice_field_trials_.override_dscp) { + SetOption(rtc::Socket::OPT_DSCP, *ice_field_trials_.override_dscp); } std::string field_trial_string = - webrtc::field_trial::FindFullName("WebRTC-SetSocketReceiveBuffer"); + field_trials->Lookup("WebRTC-SetSocketReceiveBuffer"); int receive_buffer_size_kb = 0; sscanf(field_trial_string.c_str(), "Enabled-%d", &receive_buffer_size_kb); if (receive_buffer_size_kb > 0) { @@ -781,7 +802,11 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { SetOption(rtc::Socket::OPT_RCVBUF, receive_buffer_size_kb * 1024); } - RTC_DCHECK(ValidateIceConfig(config_).ok()); + ice_field_trials_.piggyback_ice_check_acknowledgement = + field_trials->IsEnabled("WebRTC-PiggybackIceCheckAcknowledgement"); + + ice_field_trials_.extra_ice_ping = + field_trials->IsEnabled("WebRTC-ExtraICEPing"); } const IceConfig& P2PTransportChannel::config() const { @@ -795,7 +820,7 @@ const IceConfig& P2PTransportChannel::config() const { RTCError P2PTransportChannel::ValidateIceConfig(const IceConfig& config) { if (config.ice_check_interval_strong_connectivity_or_default() < config.ice_check_interval_weak_connectivity.value_or( - GetWeakPingIntervalInFieldTrial())) { + GetWeakPingIntervalInFieldTrial(nullptr))) { return RTCError(RTCErrorType::INVALID_PARAMETER, "Ping interval of candidate pairs is shorter when ICE is " "strongly connected than that when ICE is weakly " @@ -1179,7 +1204,8 @@ void P2PTransportChannel::OnNominated(Connection* conn) { return; } - if (field_trials_.send_ping_on_nomination_ice_controlled && conn != nullptr) { + if (ice_field_trials_.send_ping_on_nomination_ice_controlled && + conn != nullptr) { PingConnection(conn); MarkConnectionPinged(conn); } @@ -1419,7 +1445,7 @@ bool P2PTransportChannel::CreateConnection(PortInterface* port, return false; } - if (field_trials_.skip_relay_to_non_relay_connections) { + if (ice_field_trials_.skip_relay_to_non_relay_connections) { if ((port->Type() != remote_candidate.type()) && (port->Type() == RELAY_PORT_TYPE || remote_candidate.type() == RELAY_PORT_TYPE)) { @@ -1535,8 +1561,8 @@ void P2PTransportChannel::RememberRemoteCandidate( // port objects. int P2PTransportChannel::SetOption(rtc::Socket::Option opt, int value) { RTC_DCHECK_RUN_ON(network_thread_); - if (field_trials_.override_dscp && opt == rtc::Socket::OPT_DSCP) { - value = *field_trials_.override_dscp; + if (ice_field_trials_.override_dscp && opt == rtc::Socket::OPT_DSCP) { + value = *ice_field_trials_.override_dscp; } OptionMap::iterator it = options_.find(opt); @@ -1855,9 +1881,9 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn, } if (conn != nullptr && ice_role_ == ICEROLE_CONTROLLING && - ((field_trials_.send_ping_on_switch_ice_controlling && + ((ice_field_trials_.send_ping_on_switch_ice_controlling && old_selected_connection != nullptr) || - field_trials_.send_ping_on_selected_ice_controlling)) { + ice_field_trials_.send_ping_on_selected_ice_controlling)) { PingConnection(conn); MarkConnectionPinged(conn); } @@ -2116,7 +2142,7 @@ void P2PTransportChannel::OnConnectionStateChange(Connection* connection) { // the connection is at the latest generation. It is not enough to check // that the connection becomes weakly connected because the connection may be // changing from (writable, receiving) to (writable, not receiving). - if (field_trials_.stop_gather_on_strongly_connected) { + if (ice_field_trials_.stop_gather_on_strongly_connected) { bool strongly_connected = !connection->weak(); bool latest_generation = connection->local_candidate().generation() >= allocator_session()->generation(); diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 58bd1fb978..24c4b85b8c 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -109,45 +109,13 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { int component, webrtc::IceTransportInit init); - // TODO(jonaso): This is deprecated and will be removed. - static std::unique_ptr Create( - const std::string& transport_name, - int component, - PortAllocator* allocator, - webrtc::AsyncDnsResolverFactoryInterface* async_dns_resolver_factory, - webrtc::RtcEventLog* event_log = nullptr, - IceControllerFactoryInterface* ice_controller_factory = nullptr) { - webrtc::IceTransportInit init; - init.set_port_allocator(allocator); - init.set_async_dns_resolver_factory(async_dns_resolver_factory); - init.set_event_log(event_log); - init.set_ice_controller_factory(ice_controller_factory); - return Create(transport_name, component, std::move(init)); - } - // For testing only. // TODO(zstein): Remove once AsyncDnsResolverFactory is required. - P2PTransportChannel(const std::string& transport_name, - int component, - PortAllocator* allocator); - - ABSL_DEPRECATED("bugs.webrtc.org/12598") P2PTransportChannel( const std::string& transport_name, int component, PortAllocator* allocator, - webrtc::AsyncResolverFactory* async_resolver_factory, - webrtc::RtcEventLog* event_log = nullptr, - IceControllerFactoryInterface* ice_controller_factory = nullptr) - : P2PTransportChannel( - transport_name, - component, - allocator, - nullptr, - std::make_unique( - async_resolver_factory), - event_log, - ice_controller_factory) {} + const webrtc::WebRtcKeyValueConfig* field_trials = nullptr); ~P2PTransportChannel() override; @@ -278,8 +246,9 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { // on release, this pointer is set. std::unique_ptr owned_dns_resolver_factory, - webrtc::RtcEventLog* event_log = nullptr, - IceControllerFactoryInterface* ice_controller_factory = nullptr); + webrtc::RtcEventLog* event_log, + IceControllerFactoryInterface* ice_controller_factory, + const webrtc::WebRtcKeyValueConfig* field_trials); bool IsGettingPorts() { RTC_DCHECK_RUN_ON(network_thread_); return allocator_session()->IsGettingPorts(); @@ -431,6 +400,8 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { int64_t ComputeEstimatedDisconnectedTimeMs(int64_t now, Connection* old_connection); + void ParseFieldTrials(const webrtc::WebRtcKeyValueConfig* field_trials); + webrtc::ScopedTaskSafety task_safety_; std::string transport_name_ RTC_GUARDED_BY(network_thread_); int component_ RTC_GUARDED_BY(network_thread_); @@ -531,7 +502,8 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { // from connection->last_data_received() that uses rtc::TimeMillis(). int64_t last_data_received_ms_ = 0; - IceFieldTrials field_trials_; + // Parsed field trials. + IceFieldTrials ice_field_trials_; }; } // namespace cricket diff --git a/p2p/base/p2p_transport_channel_ice_field_trials.h b/p2p/base/p2p_transport_channel_ice_field_trials.h index f05623dd36..f19823b21e 100644 --- a/p2p/base/p2p_transport_channel_ice_field_trials.h +++ b/p2p/base/p2p_transport_channel_ice_field_trials.h @@ -19,6 +19,9 @@ namespace cricket { // put in separate file so that they can be shared e.g // with Connection. struct IceFieldTrials { + // This struct is built using the FieldTrialParser, and then not modified. + // TODO(jonaso) : Consider how members of this struct can be made const. + bool skip_relay_to_non_relay_connections = false; absl::optional max_outstanding_pings; @@ -64,6 +67,9 @@ struct IceFieldTrials { // DSCP taging. absl::optional override_dscp; + + bool piggyback_ice_check_acknowledgement = false; + bool extra_ice_ping = false; }; } // namespace cricket diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 9acd7b1fa0..2685d59751 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -42,7 +42,7 @@ #include "rtc_base/thread.h" #include "rtc_base/virtual_socket_server.h" #include "system_wrappers/include/metrics.h" -#include "test/field_trial.h" +#include "test/scoped_key_value_config.h" namespace { @@ -451,9 +451,13 @@ class P2PTransportChannelTestBase : public ::testing::Test, int component, const IceParameters& local_ice, const IceParameters& remote_ice) { - auto channel = P2PTransportChannel::Create( - "test content name", component, GetAllocator(endpoint), + webrtc::IceTransportInit init; + init.set_port_allocator(GetAllocator(endpoint)); + init.set_async_dns_resolver_factory( GetEndpoint(endpoint)->async_dns_resolver_factory_); + init.set_field_trials(&field_trials_); + auto channel = P2PTransportChannel::Create("test content name", component, + std::move(init)); channel->SignalReadyToSend.connect( this, &P2PTransportChannelTestBase::OnReadyToSend); channel->SignalCandidateGathered.connect( @@ -994,6 +998,8 @@ class P2PTransportChannelTestBase : public ::testing::Test, void OnNominated(Connection* conn) { nominated_ = true; } bool nominated() { return nominated_; } + webrtc::test::ScopedKeyValueConfig field_trials_; + private: std::unique_ptr vss_; std::unique_ptr nss_; @@ -1257,7 +1263,7 @@ class P2PTransportChannelTestWithFieldTrials public ::testing::WithParamInterface { public: void Test(const Result& expected) override { - webrtc::test::ScopedFieldTrials field_trials(GetParam()); + webrtc::test::ScopedKeyValueConfig field_trials(field_trials_, GetParam()); P2PTransportChannelTest::Test(expected); } }; @@ -2423,8 +2429,8 @@ TEST_F(P2PTransportChannelTest, // acknowledgement in the connectivity check from the remote peer. TEST_F(P2PTransportChannelTest, CanConnectWithPiggybackCheckAcknowledgementWhenCheckResponseBlocked) { - webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-PiggybackIceCheckAcknowledgement/Enabled/"); + webrtc::test::ScopedKeyValueConfig field_trials( + field_trials_, "WebRTC-PiggybackIceCheckAcknowledgement/Enabled/"); rtc::ScopedFakeClock clock; ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts); IceConfig ep1_config; @@ -4057,10 +4063,10 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) { // that sends a ping directly when a connection has been nominated // i.e on the ICE_CONTROLLED-side. TEST_F(P2PTransportChannelPingTest, TestPingOnNomination) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IceFieldTrials/send_ping_on_nomination_ice_controlled:true/"); FakePortAllocator pa(rtc::Thread::Current(), nullptr); - P2PTransportChannel ch("receiving state change", 1, &pa); + P2PTransportChannel ch("receiving state change", 1, &pa, &field_trials); PrepareChannel(&ch); ch.SetIceConfig(ch.config()); ch.SetIceRole(ICEROLE_CONTROLLED); @@ -4097,10 +4103,10 @@ TEST_F(P2PTransportChannelPingTest, TestPingOnNomination) { // that sends a ping directly when switching to a new connection // on the ICE_CONTROLLING-side. TEST_F(P2PTransportChannelPingTest, TestPingOnSwitch) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IceFieldTrials/send_ping_on_switch_ice_controlling:true/"); FakePortAllocator pa(rtc::Thread::Current(), nullptr); - P2PTransportChannel ch("receiving state change", 1, &pa); + P2PTransportChannel ch("receiving state change", 1, &pa, &field_trials); PrepareChannel(&ch); ch.SetIceConfig(ch.config()); ch.SetIceRole(ICEROLE_CONTROLLING); @@ -4134,10 +4140,10 @@ TEST_F(P2PTransportChannelPingTest, TestPingOnSwitch) { // that sends a ping directly when selecteing a new connection // on the ICE_CONTROLLING-side (i.e also initial selection). TEST_F(P2PTransportChannelPingTest, TestPingOnSelected) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IceFieldTrials/send_ping_on_selected_ice_controlling:true/"); FakePortAllocator pa(rtc::Thread::Current(), nullptr); - P2PTransportChannel ch("receiving state change", 1, &pa); + P2PTransportChannel ch("receiving state change", 1, &pa, &field_trials); PrepareChannel(&ch); ch.SetIceConfig(ch.config()); ch.SetIceRole(ICEROLE_CONTROLLING); @@ -4881,10 +4887,10 @@ TEST_F(P2PTransportChannelPingTest, TestPortDestroyedAfterTimeoutAndPruned) { } TEST_F(P2PTransportChannelPingTest, TestMaxOutstandingPingsFieldTrial) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IceFieldTrials/max_outstanding_pings:3/"); FakePortAllocator pa(rtc::Thread::Current(), nullptr); - P2PTransportChannel ch("max", 1, &pa); + P2PTransportChannel ch("max", 1, &pa, &field_trials); ch.SetIceConfig(ch.config()); PrepareChannel(&ch); ch.MaybeStartGathering(); @@ -4922,8 +4928,10 @@ class P2PTransportChannelMostLikelyToWorkFirstTest P2PTransportChannel& StartTransportChannel( bool prioritize_most_likely_to_work, - int stable_writable_connection_ping_interval) { - channel_.reset(new P2PTransportChannel("checks", 1, allocator())); + int stable_writable_connection_ping_interval, + const webrtc::WebRtcKeyValueConfig* field_trials = nullptr) { + channel_.reset( + new P2PTransportChannel("checks", 1, allocator(), field_trials)); IceConfig config = channel_->config(); config.prioritize_most_likely_candidate_pairs = prioritize_most_likely_to_work; @@ -5090,9 +5098,9 @@ TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest, // I.e that we never create connection between relay and non-relay. TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest, TestSkipRelayToNonRelayConnectionsFieldTrial) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IceFieldTrials/skip_relay_to_non_relay_connections:true/"); - P2PTransportChannel& ch = StartTransportChannel(true, 500); + P2PTransportChannel& ch = StartTransportChannel(true, 500, &field_trials); EXPECT_TRUE_WAIT(ch.ports().size() == 2, kDefaultTimeout); EXPECT_EQ(ch.ports()[0]->Type(), LOCAL_PORT_TYPE); EXPECT_EQ(ch.ports()[1]->Type(), RELAY_PORT_TYPE); @@ -5143,8 +5151,10 @@ TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest, TestTcpTurn) { TEST(P2PTransportChannelResolverTest, HostnameCandidateIsResolved) { ResolverFactoryFixture resolver_fixture; FakePortAllocator allocator(rtc::Thread::Current(), nullptr); - auto channel = - P2PTransportChannel::Create("tn", 0, &allocator, &resolver_fixture); + webrtc::IceTransportInit init; + init.set_port_allocator(&allocator); + init.set_async_dns_resolver_factory(&resolver_fixture); + auto channel = P2PTransportChannel::Create("tn", 0, std::move(init)); Candidate hostname_candidate; SocketAddress hostname_address("fake.test", 1000); hostname_candidate.set_address(hostname_address); @@ -5906,7 +5916,8 @@ TEST_F(P2PTransportChannelTest, // i.e surface_ice_candidates_on_ice_transport_type_changed requires // coordination outside of webrtc to function properly. TEST_F(P2PTransportChannelTest, SurfaceRequiresCoordination) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( + field_trials_, "WebRTC-IceFieldTrials/skip_relay_to_non_relay_connections:true/"); rtc::ScopedFakeClock clock; @@ -5970,7 +5981,7 @@ TEST_F(P2PTransportChannelTest, SurfaceRequiresCoordination) { } TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening0) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IceFieldTrials/initial_select_dampening:0/"); constexpr int kMargin = 10; @@ -5978,7 +5989,7 @@ TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening0) { clock.AdvanceTime(webrtc::TimeDelta::Seconds(1)); FakePortAllocator pa(rtc::Thread::Current(), nullptr); - P2PTransportChannel ch("test channel", 1, &pa); + P2PTransportChannel ch("test channel", 1, &pa, &field_trials); PrepareChannel(&ch); ch.SetIceConfig(ch.config()); ch.MaybeStartGathering(); @@ -5994,7 +6005,7 @@ TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening0) { } TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IceFieldTrials/initial_select_dampening:100/"); constexpr int kMargin = 10; @@ -6002,7 +6013,7 @@ TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening) { clock.AdvanceTime(webrtc::TimeDelta::Seconds(1)); FakePortAllocator pa(rtc::Thread::Current(), nullptr); - P2PTransportChannel ch("test channel", 1, &pa); + P2PTransportChannel ch("test channel", 1, &pa, &field_trials); PrepareChannel(&ch); ch.SetIceConfig(ch.config()); ch.MaybeStartGathering(); @@ -6018,7 +6029,7 @@ TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening) { } TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampeningPingReceived) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IceFieldTrials/initial_select_dampening_ping_received:100/"); constexpr int kMargin = 10; @@ -6026,7 +6037,7 @@ TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampeningPingReceived) { clock.AdvanceTime(webrtc::TimeDelta::Seconds(1)); FakePortAllocator pa(rtc::Thread::Current(), nullptr); - P2PTransportChannel ch("test channel", 1, &pa); + P2PTransportChannel ch("test channel", 1, &pa, &field_trials); PrepareChannel(&ch); ch.SetIceConfig(ch.config()); ch.MaybeStartGathering(); @@ -6043,7 +6054,7 @@ TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampeningPingReceived) { } TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampeningBoth) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IceFieldTrials/" "initial_select_dampening:100,initial_select_dampening_ping_received:" "50/"); @@ -6053,7 +6064,7 @@ TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampeningBoth) { clock.AdvanceTime(webrtc::TimeDelta::Seconds(1)); FakePortAllocator pa(rtc::Thread::Current(), nullptr); - P2PTransportChannel ch("test channel", 1, &pa); + P2PTransportChannel ch("test channel", 1, &pa, &field_trials); PrepareChannel(&ch); ch.SetIceConfig(ch.config()); ch.MaybeStartGathering(); @@ -6128,8 +6139,12 @@ class ForgetLearnedStateControllerFactory TEST_F(P2PTransportChannelPingTest, TestForgetLearnedState) { ForgetLearnedStateControllerFactory factory; FakePortAllocator pa(rtc::Thread::Current(), nullptr); - auto ch = P2PTransportChannel::Create("ping sufficiently", 1, &pa, nullptr, - nullptr, &factory); + webrtc::IceTransportInit init; + init.set_port_allocator(&pa); + init.set_ice_controller_factory(&factory); + auto ch = + P2PTransportChannel::Create("ping sufficiently", 1, std::move(init)); + PrepareChannel(ch.get()); ch->MaybeStartGathering(); ch->AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1)); @@ -6246,7 +6261,7 @@ TEST_P(GatherAfterConnectedTest, GatherAfterConnected) { const std::string field_trial = std::string("WebRTC-IceFieldTrials/stop_gather_on_strongly_connected:") + (stop_gather_on_strongly_connected ? "true/" : "false/"); - webrtc::test::ScopedFieldTrials field_trials(field_trial); + webrtc::test::ScopedKeyValueConfig field_trials(field_trials_, field_trial); rtc::ScopedFakeClock clock; // Use local + relay @@ -6307,7 +6322,7 @@ TEST_P(GatherAfterConnectedTest, GatherAfterConnectedMultiHomed) { const std::string field_trial = std::string("WebRTC-IceFieldTrials/stop_gather_on_strongly_connected:") + (stop_gather_on_strongly_connected ? "true/" : "false/"); - webrtc::test::ScopedFieldTrials field_trials(field_trial); + webrtc::test::ScopedKeyValueConfig field_trials(field_trials_, field_trial); rtc::ScopedFakeClock clock; // Use local + relay diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc index 6ab40a66ad..23b11a7a3d 100644 --- a/p2p/base/port_unittest.cc +++ b/p2p/base/port_unittest.cc @@ -63,7 +63,6 @@ #include "rtc_base/thread.h" #include "rtc_base/time_utils.h" #include "rtc_base/virtual_socket_server.h" -#include "test/field_trial.h" #include "test/gtest.h" using rtc::AsyncListenSocket; diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index bde5bf603c..9b09ae8360 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -29,7 +29,6 @@ #include "rtc_base/socket_address.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/task_utils/to_queued_task.h" -#include "system_wrappers/include/field_trial.h" namespace cricket { @@ -223,7 +222,8 @@ TurnPort::TurnPort(rtc::Thread* thread, const ProtocolAddress& server_address, const RelayCredentials& credentials, int server_priority, - webrtc::TurnCustomizer* customizer) + webrtc::TurnCustomizer* customizer, + const webrtc::WebRtcKeyValueConfig* field_trials) : Port(thread, RELAY_PORT_TYPE, factory, network, username, password), server_address_(server_address), tls_cert_verifier_(nullptr), @@ -236,7 +236,8 @@ TurnPort::TurnPort(rtc::Thread* thread, state_(STATE_CONNECTING), server_priority_(server_priority), allocate_mismatch_retries_(0), - turn_customizer_(customizer) { + turn_customizer_(customizer), + field_trials_(field_trials) { request_manager_.SignalSendPacket.connect(this, &TurnPort::OnSendStunPacket); } @@ -253,7 +254,8 @@ TurnPort::TurnPort(rtc::Thread* thread, const std::vector& tls_alpn_protocols, const std::vector& tls_elliptic_curves, webrtc::TurnCustomizer* customizer, - rtc::SSLCertificateVerifier* tls_cert_verifier) + rtc::SSLCertificateVerifier* tls_cert_verifier, + const webrtc::WebRtcKeyValueConfig* field_trials) : Port(thread, RELAY_PORT_TYPE, factory, @@ -275,7 +277,8 @@ TurnPort::TurnPort(rtc::Thread* thread, state_(STATE_CONNECTING), server_priority_(server_priority), allocate_mismatch_retries_(0), - turn_customizer_(customizer) { + turn_customizer_(customizer), + field_trials_(field_trials) { request_manager_.SignalSendPacket.connect(this, &TurnPort::OnSendStunPacket); } @@ -338,7 +341,7 @@ void TurnPort::PrepareAddress() { server_address_.address.SetPort(TURN_DEFAULT_PORT); } - if (!AllowedTurnPort(server_address_.address.port())) { + if (!AllowedTurnPort(server_address_.address.port(), field_trials_)) { // This can only happen after a 300 ALTERNATE SERVER, since the port can't // be created with a disallowed port number. RTC_LOG(LS_ERROR) << "Attempt to start allocation with disallowed port# " @@ -930,7 +933,9 @@ rtc::DiffServCodePoint TurnPort::StunDscpValue() const { } // static -bool TurnPort::AllowedTurnPort(int port) { +bool TurnPort::AllowedTurnPort( + int port, + const webrtc::WebRtcKeyValueConfig* field_trials) { // Port 53, 80 and 443 are used for existing deployments. // Ports above 1024 are assumed to be OK to use. if (port == 53 || port == 80 || port == 443 || port >= 1024) { @@ -938,7 +943,7 @@ bool TurnPort::AllowedTurnPort(int port) { } // Allow any port if relevant field trial is set. This allows disabling the // check. - if (webrtc::field_trial::IsEnabled("WebRTC-Turn-AllowSystemPorts")) { + if (field_trials && field_trials->IsEnabled("WebRTC-Turn-AllowSystemPorts")) { return true; } return false; @@ -1228,7 +1233,8 @@ bool TurnPort::CreateOrRefreshEntry(const rtc::SocketAddress& addr, RTC_DCHECK(GetConnection(addr)); } - if (webrtc::field_trial::IsEnabled("WebRTC-TurnAddMultiMapping")) { + if (field_trials_ && + field_trials_->IsEnabled("WebRTC-TurnAddMultiMapping")) { if (entry->get_remote_ufrag() != remote_ufrag) { RTC_LOG(LS_INFO) << ToString() << ": remote ufrag updated." @@ -1627,7 +1633,8 @@ void TurnCreatePermissionRequest::Prepare(StunMessage* request) { request->SetType(TURN_CREATE_PERMISSION_REQUEST); request->AddAttribute(std::make_unique( STUN_ATTR_XOR_PEER_ADDRESS, ext_addr_)); - if (webrtc::field_trial::IsEnabled("WebRTC-TurnAddMultiMapping")) { + if (port_->field_trials_ && + port_->field_trials_->IsEnabled("WebRTC-TurnAddMultiMapping")) { request->AddAttribute(std::make_unique( STUN_ATTR_MULTI_MAPPING, remote_ufrag_)); } diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index 172dcef5ad..797d19096e 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -63,7 +63,8 @@ class TurnPort : public Port { const ProtocolAddress& server_address, const RelayCredentials& credentials, int server_priority, - webrtc::TurnCustomizer* customizer) { + webrtc::TurnCustomizer* customizer, + const webrtc::WebRtcKeyValueConfig* field_trials = nullptr) { // Do basic parameter validation. if (credentials.username.size() > kMaxTurnUsernameLength) { RTC_LOG(LS_ERROR) << "Attempt to use TURN with a too long username " @@ -71,15 +72,15 @@ class TurnPort : public Port { return nullptr; } // Do not connect to low-numbered ports. The default STUN port is 3478. - if (!AllowedTurnPort(server_address.address.port())) { + if (!AllowedTurnPort(server_address.address.port(), field_trials)) { RTC_LOG(LS_ERROR) << "Attempt to use TURN to connect to port " << server_address.address.port(); return nullptr; } // Using `new` to access a non-public constructor. - return absl::WrapUnique( - new TurnPort(thread, factory, network, socket, username, password, - server_address, credentials, server_priority, customizer)); + return absl::WrapUnique(new TurnPort( + thread, factory, network, socket, username, password, server_address, + credentials, server_priority, customizer, field_trials)); } // TODO(steveanton): Remove once downstream clients have moved to `Create`. @@ -93,9 +94,11 @@ class TurnPort : public Port { const ProtocolAddress& server_address, const RelayCredentials& credentials, int server_priority, - webrtc::TurnCustomizer* customizer) { + webrtc::TurnCustomizer* customizer, + const webrtc::WebRtcKeyValueConfig* field_trials) { return Create(thread, factory, network, socket, username, password, - server_address, credentials, server_priority, customizer); + server_address, credentials, server_priority, customizer, + field_trials); } // Create a TURN port that will use a new socket, bound to `network` and @@ -114,7 +117,8 @@ class TurnPort : public Port { const std::vector& tls_alpn_protocols, const std::vector& tls_elliptic_curves, webrtc::TurnCustomizer* customizer, - rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr) { + rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr, + const webrtc::WebRtcKeyValueConfig* field_trials = nullptr) { // Do basic parameter validation. if (credentials.username.size() > kMaxTurnUsernameLength) { RTC_LOG(LS_ERROR) << "Attempt to use TURN with a too long username " @@ -122,7 +126,7 @@ class TurnPort : public Port { return nullptr; } // Do not connect to low-numbered ports. The default STUN port is 3478. - if (!AllowedTurnPort(server_address.address.port())) { + if (!AllowedTurnPort(server_address.address.port(), field_trials)) { RTC_LOG(LS_ERROR) << "Attempt to use TURN to connect to port " << server_address.address.port(); return nullptr; @@ -131,7 +135,7 @@ class TurnPort : public Port { return absl::WrapUnique(new TurnPort( thread, factory, network, min_port, max_port, username, password, server_address, credentials, server_priority, tls_alpn_protocols, - tls_elliptic_curves, customizer, tls_cert_verifier)); + tls_elliptic_curves, customizer, tls_cert_verifier, field_trials)); } // TODO(steveanton): Remove once downstream clients have moved to `Create`. @@ -149,11 +153,12 @@ class TurnPort : public Port { const std::vector& tls_alpn_protocols, const std::vector& tls_elliptic_curves, webrtc::TurnCustomizer* customizer, - rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr) { + rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr, + const webrtc::WebRtcKeyValueConfig* field_trials = nullptr) { return Create(thread, factory, network, min_port, max_port, username, password, server_address, credentials, server_priority, tls_alpn_protocols, tls_elliptic_curves, customizer, - tls_cert_verifier); + tls_cert_verifier, field_trials); } ~TurnPort() override; @@ -264,7 +269,8 @@ class TurnPort : public Port { const ProtocolAddress& server_address, const RelayCredentials& credentials, int server_priority, - webrtc::TurnCustomizer* customizer); + webrtc::TurnCustomizer* customizer, + const webrtc::WebRtcKeyValueConfig* field_trials = nullptr); TurnPort(rtc::Thread* thread, rtc::PacketSocketFactory* factory, @@ -279,7 +285,8 @@ class TurnPort : public Port { const std::vector& tls_alpn_protocols, const std::vector& tls_elliptic_curves, webrtc::TurnCustomizer* customizer, - rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr); + rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr, + const webrtc::WebRtcKeyValueConfig* field_trials = nullptr); // NOTE: This method needs to be accessible for StunPort // return true if entry was created (i.e channel_number consumed). @@ -304,7 +311,8 @@ class TurnPort : public Port { typedef std::map SocketOptionsMap; typedef std::set AttemptedServerSet; - static bool AllowedTurnPort(int port); + static bool AllowedTurnPort(int port, + const webrtc::WebRtcKeyValueConfig* field_trials); void OnMessage(rtc::Message* pmsg) override; bool CreateTurnClientSocket(); @@ -410,6 +418,8 @@ class TurnPort : public Port { // must outlive the TurnPort's lifetime. webrtc::TurnCustomizer* turn_customizer_ = nullptr; + const webrtc::WebRtcKeyValueConfig* field_trials_; + // Optional TurnLoggingId. // An identifier set by application that is added to TURN_ALLOCATE_REQUEST // and can be used to match client/backend logs. diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index 44d258363e..30ad2c009e 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -41,8 +41,8 @@ #include "rtc_base/thread.h" #include "rtc_base/time_utils.h" #include "rtc_base/virtual_socket_server.h" -#include "test/field_trial.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" using rtc::SocketAddress; @@ -293,9 +293,10 @@ class TurnPortTest : public ::testing::Test, const std::string& password, const ProtocolAddress& server_address) { RelayCredentials credentials(username, password); - turn_port_ = TurnPort::Create( - &main_, &socket_factory_, network, 0, 0, kIceUfrag1, kIcePwd1, - server_address, credentials, 0, {}, {}, turn_customizer_.get()); + turn_port_ = + TurnPort::Create(&main_, &socket_factory_, network, 0, 0, kIceUfrag1, + kIcePwd1, server_address, credentials, 0, {}, {}, + turn_customizer_.get(), nullptr, &field_trials_); if (!turn_port_) { return false; } @@ -327,9 +328,10 @@ class TurnPortTest : public ::testing::Test, } RelayCredentials credentials(username, password); - turn_port_ = TurnPort::Create( - &main_, &socket_factory_, MakeNetwork(kLocalAddr1), socket_.get(), - kIceUfrag1, kIcePwd1, server_address, credentials, 0, nullptr); + turn_port_ = + TurnPort::Create(&main_, &socket_factory_, MakeNetwork(kLocalAddr1), + socket_.get(), kIceUfrag1, kIcePwd1, server_address, + credentials, 0, nullptr, &field_trials_); // This TURN port will be the controlling. turn_port_->SetIceRole(ICEROLE_CONTROLLING); ConnectSignals(); @@ -760,6 +762,7 @@ class TurnPortTest : public ::testing::Test, } protected: + webrtc::test::ScopedKeyValueConfig field_trials_; rtc::ScopedFakeClock fake_clock_; // When a "create port" helper method is called with an IP, we create a // Network with that IP and add it to this list. Using a list instead of a @@ -1852,8 +1855,8 @@ TEST_F(TurnPortTest, TestTurnDangerousAlternateServer) { } TEST_F(TurnPortTest, TestTurnDangerousServerAllowedWithFieldTrial) { - webrtc::test::ScopedFieldTrials override_field_trials( - "WebRTC-Turn-AllowSystemPorts/Enabled/"); + webrtc::test::ScopedKeyValueConfig override_field_trials( + field_trials_, "WebRTC-Turn-AllowSystemPorts/Enabled/"); CreateTurnPort(kTurnUsername, kTurnPassword, kTurnDangerousProtoAddr); ASSERT_TRUE(turn_port_); } diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc index 06eabb83dd..20032a99d7 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -20,6 +20,7 @@ #include "absl/algorithm/container.h" #include "absl/memory/memory.h" +#include "api/transport/field_trial_based_config.h" #include "p2p/base/basic_packet_socket_factory.h" #include "p2p/base/port.h" #include "p2p/base/stun_port.h" @@ -31,7 +32,6 @@ #include "rtc_base/logging.h" #include "rtc_base/task_utils/to_queued_task.h" #include "rtc_base/trace_event.h" -#include "system_wrappers/include/field_trial.h" #include "system_wrappers/include/metrics.h" using rtc::CreateRandomId; @@ -152,7 +152,7 @@ BasicPortAllocator::BasicPortAllocator( webrtc::TurnCustomizer* customizer, RelayPortFactoryInterface* relay_port_factory) : network_manager_(network_manager), socket_factory_(socket_factory) { - InitRelayPortFactory(relay_port_factory); + Init(relay_port_factory, nullptr); RTC_DCHECK(relay_port_factory_ != nullptr); RTC_DCHECK(network_manager_ != nullptr); RTC_DCHECK(socket_factory_ != nullptr); @@ -162,7 +162,7 @@ BasicPortAllocator::BasicPortAllocator( BasicPortAllocator::BasicPortAllocator(rtc::NetworkManager* network_manager) : network_manager_(network_manager), socket_factory_(nullptr) { - InitRelayPortFactory(nullptr); + Init(nullptr, nullptr); RTC_DCHECK(relay_port_factory_ != nullptr); RTC_DCHECK(network_manager_ != nullptr); } @@ -177,7 +177,7 @@ BasicPortAllocator::BasicPortAllocator(rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, const ServerAddresses& stun_servers) : network_manager_(network_manager), socket_factory_(socket_factory) { - InitRelayPortFactory(nullptr); + Init(nullptr, nullptr); RTC_DCHECK(relay_port_factory_ != nullptr); RTC_DCHECK(network_manager_ != nullptr); SetConfiguration(stun_servers, std::vector(), 0, @@ -251,14 +251,22 @@ void BasicPortAllocator::AddTurnServer(const RelayServerConfig& turn_server) { turn_port_prune_policy(), turn_customizer()); } -void BasicPortAllocator::InitRelayPortFactory( - RelayPortFactoryInterface* relay_port_factory) { +void BasicPortAllocator::Init( + RelayPortFactoryInterface* relay_port_factory, + const webrtc::WebRtcKeyValueConfig* field_trials) { if (relay_port_factory != nullptr) { relay_port_factory_ = relay_port_factory; } else { default_relay_port_factory_.reset(new TurnPortFactory()); relay_port_factory_ = default_relay_port_factory_.get(); } + + if (field_trials != nullptr) { + field_trials_ = field_trials; + } else { + owned_field_trials_ = std::make_unique(); + field_trials_ = owned_field_trials_.get(); + } } // BasicPortAllocatorSession @@ -602,8 +610,9 @@ void BasicPortAllocatorSession::UpdateIceParametersInternal() { void BasicPortAllocatorSession::GetPortConfigurations() { RTC_DCHECK_RUN_ON(network_thread_); - auto config = std::make_unique(allocator_->stun_servers(), - username(), password()); + auto config = std::make_unique( + allocator_->stun_servers(), username(), password(), + allocator()->field_trials()); for (const RelayServerConfig& turn_server : allocator_->turn_servers()) { config->AddRelay(turn_server); @@ -1565,6 +1574,7 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config) { args.server_address = &(*relay_port); args.config = &config; args.turn_customizer = session_->allocator()->turn_customizer(); + args.field_trials = session_->allocator()->field_trials(); std::unique_ptr port; // Shared socket mode must be enabled only for UDP based ports. Hence @@ -1658,24 +1668,19 @@ void AllocationSequence::OnPortDestroyed(PortInterface* port) { } } -// PortConfiguration -PortConfiguration::PortConfiguration(const rtc::SocketAddress& stun_address, - const std::string& username, - const std::string& password) - : stun_address(stun_address), username(username), password(password) { - if (!stun_address.IsNil()) - stun_servers.insert(stun_address); -} - -PortConfiguration::PortConfiguration(const ServerAddresses& stun_servers, - const std::string& username, - const std::string& password) +PortConfiguration::PortConfiguration( + const ServerAddresses& stun_servers, + const std::string& username, + const std::string& password, + const webrtc::WebRtcKeyValueConfig* field_trials) : stun_servers(stun_servers), username(username), password(password) { if (!stun_servers.empty()) stun_address = *(stun_servers.begin()); // Note that this won't change once the config is initialized. - use_turn_server_as_stun_server_disabled = - webrtc::field_trial::IsDisabled("WebRTC-UseTurnServerAsStunServer"); + if (field_trials) { + use_turn_server_as_stun_server_disabled = + field_trials->IsDisabled("WebRTC-UseTurnServerAsStunServer"); + } } ServerAddresses PortConfiguration::StunServers() { diff --git a/p2p/client/basic_port_allocator.h b/p2p/client/basic_port_allocator.h index b1dc7b12a2..946aa6b17e 100644 --- a/p2p/client/basic_port_allocator.h +++ b/p2p/client/basic_port_allocator.h @@ -16,6 +16,7 @@ #include #include "api/turn_customizer.h" +#include "api/webrtc_key_value_config.h" #include "p2p/base/port_allocator.h" #include "p2p/client/relay_port_factory_interface.h" #include "p2p/client/turn_port_factory.h" @@ -80,15 +81,23 @@ class RTC_EXPORT BasicPortAllocator : public PortAllocator { void SetVpnList(const std::vector& vpn_list) override; + const webrtc::WebRtcKeyValueConfig* field_trials() const { + return field_trials_; + } + private: void OnIceRegathering(PortAllocatorSession* session, IceRegatheringReason reason); - // This function makes sure that relay_port_factory_ is set properly. - void InitRelayPortFactory(RelayPortFactoryInterface* relay_port_factory); + // This function makes sure that relay_port_factory_ and field_trials_ is set + // properly. + void Init(RelayPortFactoryInterface* relay_port_factory, + const webrtc::WebRtcKeyValueConfig* field_trials); bool MdnsObfuscationEnabled() const override; + const webrtc::WebRtcKeyValueConfig* field_trials_; + std::unique_ptr owned_field_trials_; rtc::NetworkManager* network_manager_; rtc::PacketSocketFactory* socket_factory_; int network_ignore_mask_ = rtc::kDefaultNetworkIgnoreMask; @@ -298,14 +307,10 @@ struct RTC_EXPORT PortConfiguration { typedef std::vector RelayList; RelayList relays; - // TODO(jiayl): remove this ctor when Chrome is updated. - PortConfiguration(const rtc::SocketAddress& stun_address, - const std::string& username, - const std::string& password); - PortConfiguration(const ServerAddresses& stun_servers, const std::string& username, - const std::string& password); + const std::string& password, + const webrtc::WebRtcKeyValueConfig* field_trials = nullptr); // Returns addresses of both the explicitly configured STUN servers, // and TURN servers that should be used as STUN servers. diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc index 6db82d2e30..aa04b78855 100644 --- a/p2p/client/basic_port_allocator_unittest.cc +++ b/p2p/client/basic_port_allocator_unittest.cc @@ -42,9 +42,9 @@ #include "rtc_base/thread.h" #include "rtc_base/virtual_socket_server.h" #include "system_wrappers/include/metrics.h" -#include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" using rtc::IPAddress; using rtc::SocketAddress; @@ -2444,12 +2444,12 @@ TEST_F(BasicPortAllocatorTest, TestUseTurnServerAsStunSever) { } TEST_F(BasicPortAllocatorTest, TestDoNotUseTurnServerAsStunSever) { - webrtc::test::ScopedFieldTrials field_trials( + webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-UseTurnServerAsStunServer/Disabled/"); ServerAddresses stun_servers; stun_servers.insert(kStunAddr); PortConfiguration port_config(stun_servers, "" /* user_name */, - "" /* password */); + "" /* password */, &field_trials); RelayServerConfig turn_servers = CreateTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr); port_config.AddRelay(turn_servers); diff --git a/p2p/client/relay_port_factory_interface.h b/p2p/client/relay_port_factory_interface.h index d3884126a6..cb4eb97483 100644 --- a/p2p/client/relay_port_factory_interface.h +++ b/p2p/client/relay_port_factory_interface.h @@ -26,6 +26,7 @@ class Thread; namespace webrtc { class TurnCustomizer; +class WebRtcKeyValueConfig; } // namespace webrtc namespace cricket { @@ -44,6 +45,7 @@ struct CreateRelayPortArgs { std::string username; std::string password; webrtc::TurnCustomizer* turn_customizer; + const webrtc::WebRtcKeyValueConfig* field_trials = nullptr; }; inline CreateRelayPortArgs::CreateRelayPortArgs() {} diff --git a/p2p/client/turn_port_factory.cc b/p2p/client/turn_port_factory.cc index feaada3a1c..07321b85d6 100644 --- a/p2p/client/turn_port_factory.cc +++ b/p2p/client/turn_port_factory.cc @@ -26,7 +26,8 @@ std::unique_ptr TurnPortFactory::Create( auto port = TurnPort::CreateUnique( args.network_thread, args.socket_factory, args.network, udp_socket, args.username, args.password, *args.server_address, - args.config->credentials, args.config->priority, args.turn_customizer); + args.config->credentials, args.config->priority, args.turn_customizer, + args.field_trials); if (!port) return nullptr; port->SetTlsCertPolicy(args.config->tls_cert_policy); @@ -42,7 +43,7 @@ std::unique_ptr TurnPortFactory::Create(const CreateRelayPortArgs& args, max_port, args.username, args.password, *args.server_address, args.config->credentials, args.config->priority, args.config->tls_alpn_protocols, args.config->tls_elliptic_curves, - args.turn_customizer, args.config->tls_cert_verifier); + args.turn_customizer, args.config->tls_cert_verifier, args.field_trials); if (!port) return nullptr; port->SetTlsCertPolicy(args.config->tls_cert_policy); diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 6c69130a38..727a30a1ae 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -400,6 +400,7 @@ JsepTransportController::CreateIceTransport(const std::string& transport_name, init.set_port_allocator(port_allocator_); init.set_async_dns_resolver_factory(async_dns_resolver_factory_); init.set_event_log(config_.event_log); + init.set_field_trials(config_.field_trials); return config_.ice_transport_factory->CreateIceTransport( transport_name, component, std::move(init)); }