diff --git a/p2p/base/fake_port_allocator.h b/p2p/base/fake_port_allocator.h index 706635778b..0bb7dd0139 100644 --- a/p2p/base/fake_port_allocator.h +++ b/p2p/base/fake_port_allocator.h @@ -92,7 +92,6 @@ class FakePortAllocatorSession : public PortAllocatorSession { component, ice_ufrag, ice_pwd, - /*kTiebreakerDefault = */ 44444, allocator->flags()), network_thread_(network_thread), factory_(factory), @@ -111,6 +110,7 @@ class FakePortAllocatorSession : public PortAllocatorSession { field_trials_(field_trials) { ipv4_network_.AddIP(rtc::IPAddress(INADDR_LOOPBACK)); ipv6_network_.AddIP(rtc::IPAddress(in6addr_loopback)); + set_ice_tiebreaker(/*kTiebreakerDefault = */ 44444); } void SetCandidateFilter(uint32_t filter) override { diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 2f18f1dbb9..0bccb67209 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -164,7 +164,7 @@ P2PTransportChannel::P2PTransportChannel( error_(0), remote_ice_mode_(ICEMODE_FULL), ice_role_(ICEROLE_UNKNOWN), - ice_tiebreaker_(0), + tiebreaker_(0), gathering_state_(kIceGatheringNew), weak_ping_interval_(GetWeakPingIntervalInFieldTrial(field_trials)), config_(RECEIVING_TIMEOUT, @@ -326,7 +326,7 @@ void P2PTransportChannel::SetIceTiebreaker(uint64_t tiebreaker) { return; } - ice_tiebreaker_ = tiebreaker; + tiebreaker_ = tiebreaker; } IceTransportState P2PTransportChannel::GetState() const { @@ -885,6 +885,7 @@ void P2PTransportChannel::MaybeStartGathering() { ice_parameters_.ufrag, ice_parameters_.pwd); if (pooled_session) { + pooled_session->set_ice_tiebreaker(tiebreaker_); AddAllocatorSession(std::move(pooled_session)); PortAllocatorSession* raw_pooled_session = allocator_sessions_.back().get(); @@ -901,6 +902,7 @@ void P2PTransportChannel::MaybeStartGathering() { AddAllocatorSession(allocator_->CreateSession( transport_name(), component(), ice_parameters_.ufrag, ice_parameters_.pwd)); + allocator_sessions_.back()->set_ice_tiebreaker(tiebreaker_); allocator_sessions_.back()->StartGettingPorts(); } } @@ -928,7 +930,7 @@ void P2PTransportChannel::OnPortReady(PortAllocatorSession* session, // if one is pending. port->SetIceRole(ice_role_); - port->SetIceTiebreaker(ice_tiebreaker_); + port->SetIceTiebreaker(tiebreaker_); ports_.push_back(port); port->SignalUnknownAddress.connect(this, &P2PTransportChannel::OnUnknownAddress); diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index f7472df38a..da7933f2e7 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -439,7 +439,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal, RTC_GUARDED_BY(network_thread_); IceMode remote_ice_mode_ RTC_GUARDED_BY(network_thread_); IceRole ice_role_ RTC_GUARDED_BY(network_thread_); - uint64_t ice_tiebreaker_ RTC_GUARDED_BY(network_thread_); + uint64_t tiebreaker_ RTC_GUARDED_BY(network_thread_); IceGatheringState gathering_state_ RTC_GUARDED_BY(network_thread_); std::unique_ptr regathering_controller_ RTC_GUARDED_BY(network_thread_); diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 79e984cfec..5bcfee0473 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -141,6 +141,7 @@ const cricket::IceParameters kIceParams[4] = { const uint64_t kLowTiebreaker = 11111; const uint64_t kHighTiebreaker = 22222; +const uint64_t kTiebreakerDefault = 44444; cricket::IceConfig CreateIceConfig( int receiving_timeout, @@ -298,6 +299,10 @@ class P2PTransportChannelTestBase : public ::testing::Test, &ep2_.network_manager_, socket_factory_.get(), stun_servers, kTurnUdpIntAddr, rtc::SocketAddress())); + ep1_.SetIceTiebreaker(kTiebreakerDefault); + ep1_.allocator_->SetIceTiebreaker(kTiebreakerDefault); + ep2_.SetIceTiebreaker(kTiebreakerDefault); + ep2_.allocator_->SetIceTiebreaker(kTiebreakerDefault); webrtc::metrics::Reset(); } @@ -3342,6 +3347,7 @@ class P2PTransportChannelPingTest : public ::testing::Test, protected: void PrepareChannel(P2PTransportChannel* ch) { ch->SetIceRole(ICEROLE_CONTROLLING); + ch->SetIceTiebreaker(kTiebreakerDefault); ch->SetIceParameters(kIceParams[0]); ch->SetRemoteIceParameters(kIceParams[1]); ch->SignalNetworkRouteChanged.connect( @@ -3709,6 +3715,7 @@ TEST_F(P2PTransportChannelPingTest, PingingStartedAsSoonAsPossible) { &field_trials_); P2PTransportChannel ch("TestChannel", 1, &pa, &field_trials_); ch.SetIceRole(ICEROLE_CONTROLLING); + ch.SetIceTiebreaker(kTiebreakerDefault); ch.SetIceParameters(kIceParams[0]); ch.MaybeStartGathering(); EXPECT_EQ_WAIT(IceGatheringState::kIceGatheringComplete, ch.gathering_state(), diff --git a/p2p/base/port_allocator.cc b/p2p/base/port_allocator.cc index 30bcfb1732..3745717510 100644 --- a/p2p/base/port_allocator.cc +++ b/p2p/base/port_allocator.cc @@ -62,7 +62,6 @@ PortAllocatorSession::PortAllocatorSession(absl::string_view content_name, int component, absl::string_view ice_ufrag, absl::string_view ice_pwd, - uint64_t ice_tiebreaker, uint32_t flags) : flags_(flags), generation_(0), @@ -70,7 +69,7 @@ PortAllocatorSession::PortAllocatorSession(absl::string_view content_name, component_(component), ice_ufrag_(ice_ufrag), ice_pwd_(ice_pwd), - ice_tiebreaker_(ice_tiebreaker) { + tiebreaker_(0) { // Pooled sessions are allowed to be created with empty content name, // component, ufrag and password. RTC_DCHECK(ice_ufrag.empty() == ice_pwd.empty()); @@ -102,7 +101,7 @@ PortAllocator::PortAllocator() step_delay_(kDefaultStepDelay), allow_tcp_listen_(true), candidate_filter_(CF_ALL), - tiebreaker_(rtc::CreateRandomId64()) { + tiebreaker_(0) { // The allocator will be attached to a thread in Initialize. thread_checker_.Detach(); } @@ -198,6 +197,13 @@ bool PortAllocator::SetConfiguration( return true; } +void PortAllocator::SetIceTiebreaker(uint64_t tiebreaker) { + tiebreaker_ = tiebreaker; + for (auto& pooled_session : pooled_sessions_) { + pooled_session->set_ice_tiebreaker(tiebreaker_); + } +} + std::unique_ptr PortAllocator::CreateSession( absl::string_view content_name, int component, diff --git a/p2p/base/port_allocator.h b/p2p/base/port_allocator.h index e9630df143..b8cffca9c0 100644 --- a/p2p/base/port_allocator.h +++ b/p2p/base/port_allocator.h @@ -199,7 +199,6 @@ class RTC_EXPORT PortAllocatorSession : public sigslot::has_slots<> { int component, absl::string_view ice_ufrag, absl::string_view ice_pwd, - uint64_t ice_tiebreaker, uint32_t flags); // Subclasses should clean up any ports created. @@ -213,9 +212,9 @@ class RTC_EXPORT PortAllocatorSession : public sigslot::has_slots<> { const std::string& ice_pwd() const { return ice_pwd_; } bool pooled() const { return pooled_; } - // TODO(bugs.webrtc.org/14605): remove once downstream code has been updated. - void set_ice_tiebreaker(uint64_t tiebreaker) { ice_tiebreaker_ = tiebreaker; } - uint64_t ice_tiebreaker() const { return ice_tiebreaker_; } + // TODO(bugs.webrtc.org/14605): move this to the constructor + void set_ice_tiebreaker(uint64_t tiebreaker) { tiebreaker_ = tiebreaker; } + uint64_t ice_tiebreaker() const { return tiebreaker_; } // Setting this filter should affect not only candidates gathered in the // future, but candidates already gathered and ports already "ready", @@ -330,10 +329,12 @@ class RTC_EXPORT PortAllocatorSession : public sigslot::has_slots<> { int component_; std::string ice_ufrag_; std::string ice_pwd_; - uint64_t ice_tiebreaker_; bool pooled_ = false; + // TODO(bugs.webrtc.org/14605): move this to the constructor + uint64_t tiebreaker_; + // SetIceParameters is an implementation detail which only PortAllocator // should be able to call. friend class PortAllocator; @@ -386,6 +387,9 @@ class RTC_EXPORT PortAllocator : public sigslot::has_slots<> { const absl::optional& stun_candidate_keepalive_interval = absl::nullopt); + void SetIceTiebreaker(uint64_t tiebreaker); + uint64_t IceTiebreaker() const { return tiebreaker_; } + const ServerAddresses& stun_servers() const { CheckRunOnValidThreadIfInitialized(); return stun_servers_; @@ -457,8 +461,6 @@ class RTC_EXPORT PortAllocator : public sigslot::has_slots<> { // 3. mDNS concealment of private IPs is enabled. Candidate SanitizeCandidate(const Candidate& c) const; - uint64_t ice_tiebreaker() const { return tiebreaker_; } - uint32_t flags() const { CheckRunOnValidThreadIfInitialized(); return flags_; diff --git a/p2p/base/port_allocator_unittest.cc b/p2p/base/port_allocator_unittest.cc index 2df8662f62..836a2fa494 100644 --- a/p2p/base/port_allocator_unittest.cc +++ b/p2p/base/port_allocator_unittest.cc @@ -26,6 +26,7 @@ static const char kIceUfrag[] = "UF00"; static const char kIcePwd[] = "TESTICEPWD00000000000000"; static const char kTurnUsername[] = "test"; static const char kTurnPassword[] = "test"; +constexpr uint64_t kTiebreakerDefault = 44444; class PortAllocatorTest : public ::testing::Test, public sigslot::has_slots<> { public: @@ -37,7 +38,9 @@ class PortAllocatorTest : public ::testing::Test, public sigslot::has_slots<> { allocator_(std::make_unique( rtc::Thread::Current(), packet_socket_factory_.get(), - &field_trials_)) {} + &field_trials_)) { + allocator_->SetIceTiebreaker(kTiebreakerDefault); + } protected: void SetConfigurationWithPoolSize(int candidate_pool_size) { diff --git a/p2p/base/regathering_controller_unittest.cc b/p2p/base/regathering_controller_unittest.cc index 573c0fd23f..91b7270f77 100644 --- a/p2p/base/regathering_controller_unittest.cc +++ b/p2p/base/regathering_controller_unittest.cc @@ -40,6 +40,7 @@ const rtc::SocketAddress kTurnUdpIntAddr("99.99.99.3", const cricket::RelayCredentials kRelayCredentials("test", "test"); const char kIceUfrag[] = "UF00"; const char kIcePwd[] = "TESTICEPWD00000000000000"; +constexpr uint64_t kTiebreakerDefault = 44444; } // namespace @@ -58,6 +59,7 @@ class RegatheringControllerTest : public ::testing::Test, rtc::Thread::Current(), packet_socket_factory_.get(), &field_trials_)) { + allocator_->SetIceTiebreaker(kTiebreakerDefault); BasicRegatheringController::Config regathering_config; regathering_config.regather_on_failed_networks_interval = 0; regathering_controller_.reset(new BasicRegatheringController( diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc index 92c0ac37f6..e95033efeb 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -243,7 +243,7 @@ PortAllocatorSession* BasicPortAllocator::CreateSessionInternal( CheckRunOnValidThreadAndInitialized(); return new BasicPortAllocatorSession(this, std::string(content_name), component, std::string(ice_ufrag), - std::string(ice_pwd), ice_tiebreaker()); + std::string(ice_pwd)); } void BasicPortAllocator::AddTurnServerForTesting( @@ -261,13 +261,11 @@ BasicPortAllocatorSession::BasicPortAllocatorSession( absl::string_view content_name, int component, absl::string_view ice_ufrag, - absl::string_view ice_pwd, - uint64_t ice_tiebreaker) + absl::string_view ice_pwd) : PortAllocatorSession(content_name, component, ice_ufrag, ice_pwd, - ice_tiebreaker, allocator->flags()), allocator_(allocator), network_thread_(rtc::Thread::Current()), @@ -283,19 +281,6 @@ BasicPortAllocatorSession::BasicPortAllocatorSession( allocator_->network_manager()->StartUpdating(); } -BasicPortAllocatorSession::BasicPortAllocatorSession( - BasicPortAllocator* allocator, - absl::string_view content_name, - int component, - absl::string_view ice_ufrag, - absl::string_view ice_pwd) - : BasicPortAllocatorSession(allocator, - content_name, - component, - ice_ufrag, - ice_pwd, - 0) {} - BasicPortAllocatorSession::~BasicPortAllocatorSession() { TRACE_EVENT0("webrtc", "BasicPortAllocatorSession::~BasicPortAllocatorSession"); diff --git a/p2p/client/basic_port_allocator.h b/p2p/client/basic_port_allocator.h index 1612f77d35..25201fd016 100644 --- a/p2p/client/basic_port_allocator.h +++ b/p2p/client/basic_port_allocator.h @@ -116,13 +116,6 @@ enum class SessionState { // destroyed on the network thread. class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession { public: - BasicPortAllocatorSession(BasicPortAllocator* allocator, - absl::string_view content_name, - int component, - absl::string_view ice_ufrag, - absl::string_view ice_pwd, - uint64_t ice_tiebreaker); - // TODO(bugs.webrtc.org/14605): remove this constructor BasicPortAllocatorSession(BasicPortAllocator* allocator, absl::string_view content_name, int component, diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc index f77040d128..77443eedbb 100644 --- a/p2p/client/basic_port_allocator_unittest.cc +++ b/p2p/client/basic_port_allocator_unittest.cc @@ -112,6 +112,8 @@ static const char kTurnPassword[] = "test"; // Add some margin of error for slow bots. static const int kStunTimeoutMs = cricket::STUN_TOTAL_TIMEOUT; +constexpr uint64_t kTiebreakerDefault = 44444; + namespace { void CheckStunKeepaliveIntervalOfAllReadyPorts( @@ -174,6 +176,7 @@ class BasicPortAllocatorTestBase : public ::testing::Test, &network_manager_, &socket_factory_, stun_servers, &field_trials_); allocator_->Initialize(); allocator_->set_step_delay(kMinimumStepDelay); + allocator_->SetIceTiebreaker(kTiebreakerDefault); webrtc::metrics::Reset(); } @@ -211,6 +214,7 @@ class BasicPortAllocatorTestBase : public ::testing::Test, allocator_.reset( new BasicPortAllocator(&network_manager_, &socket_factory_)); allocator_->Initialize(); + allocator_->SetIceTiebreaker(kTiebreakerDefault); allocator_->set_step_delay(kMinimumStepDelay); } // Endpoint is behind a NAT, with STUN specified. @@ -295,6 +299,7 @@ class BasicPortAllocatorTestBase : public ::testing::Test, this, &BasicPortAllocatorTestBase::OnCandidatesRemoved); session->SignalCandidatesAllocationDone.connect( this, &BasicPortAllocatorTestBase::OnCandidatesAllocationDone); + session->set_ice_tiebreaker(kTiebreakerDefault); return session; } diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index d5eb0c633d..9473d962ef 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -55,7 +55,6 @@ JsepTransportController::JsepTransportController( }), config_(std::move(config)), active_reset_srtp_params_(config.active_reset_srtp_params), - ice_tiebreaker_(port_allocator ? port_allocator->ice_tiebreaker() : 0), bundles_(config.bundle_policy) { // The `transport_observer` is assumed to be non-null. RTC_DCHECK(config_.transport_observer); @@ -63,6 +62,9 @@ JsepTransportController::JsepTransportController( RTC_DCHECK(config_.ice_transport_factory); RTC_DCHECK(config_.on_dtls_handshake_error_); RTC_DCHECK(config_.field_trials); + if (port_allocator_) { + port_allocator_->SetIceTiebreaker(ice_tiebreaker_); + } } JsepTransportController::~JsepTransportController() { diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index 7f06c22969..448844ac79 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -507,7 +507,7 @@ class JsepTransportController : public sigslot::has_slots<> { cricket::IceConfig ice_config_; cricket::IceRole ice_role_ = cricket::ICEROLE_CONTROLLING; - uint64_t ice_tiebreaker_; + uint64_t ice_tiebreaker_ = rtc::CreateRandomId64(); rtc::scoped_refptr certificate_; BundleManager bundles_; diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc index bd4848cf8b..492e108cbc 100644 --- a/pc/peer_connection_ice_unittest.cc +++ b/pc/peer_connection_ice_unittest.cc @@ -94,6 +94,7 @@ using ::testing::Values; constexpr int kIceCandidatesTimeout = 10000; constexpr int64_t kWaitTimeout = 10000; +constexpr uint64_t kTiebreakerDefault = 44444; class PeerConnectionWrapperForIceTest : public PeerConnectionWrapper { public: @@ -1447,6 +1448,7 @@ class PeerConnectionIceConfigTest : public ::testing::Test { packet_socket_factory_.get(), &field_trials_)); port_allocator_ = port_allocator.get(); + port_allocator_->SetIceTiebreaker(kTiebreakerDefault); PeerConnectionDependencies pc_dependencies(&observer_); pc_dependencies.allocator = std::move(port_allocator); auto result = pc_factory_->CreatePeerConnectionOrError( diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc index 71c85d4c6f..08fb1632d6 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -114,6 +114,7 @@ static const char kVideoTracks[][32] = {"videotrack0", "videotrack1"}; static const char kRecvonly[] = "recvonly"; static const char kSendrecv[] = "sendrecv"; +constexpr uint64_t kTiebreakerDefault = 44444; // Reference SDP with a MediaStream with label "stream1" and audio track with // id "audio_1" and a video track with id "video_1; @@ -710,6 +711,7 @@ class PeerConnectionInterfaceBaseTest : public ::testing::Test { std::make_unique(vss_.get()), &field_trials_)); port_allocator_ = port_allocator.get(); + port_allocator_->SetIceTiebreaker(kTiebreakerDefault); // Create certificate generator unless DTLS constraint is explicitly set to // false.