diff --git a/p2p/base/fake_port_allocator.h b/p2p/base/fake_port_allocator.h index 0bb7dd0139..a51a7ca4ee 100644 --- a/p2p/base/fake_port_allocator.h +++ b/p2p/base/fake_port_allocator.h @@ -93,6 +93,7 @@ class FakePortAllocatorSession : public PortAllocatorSession { ice_ufrag, ice_pwd, allocator->flags()), + allocator_(allocator), network_thread_(network_thread), factory_(factory), ipv4_network_("network", @@ -110,7 +111,6 @@ 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 { @@ -127,7 +127,7 @@ class FakePortAllocatorSession : public PortAllocatorSession { username(), password(), false, field_trials_)); RTC_DCHECK(port_); - port_->SetIceTiebreaker(ice_tiebreaker()); + port_->SetIceTiebreaker(allocator_->ice_tiebreaker()); port_->SubscribePortDestroyed( [this](PortInterface* port) { OnPortDestroyed(port); }); AddPort(port_.get()); @@ -199,6 +199,7 @@ class FakePortAllocatorSession : public PortAllocatorSession { port_.release(); } + PortAllocator* allocator_; rtc::Thread* network_thread_; rtc::PacketSocketFactory* factory_; rtc::Network ipv4_network_; diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 0bccb67209..2f18f1dbb9 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), - tiebreaker_(0), + ice_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; } - tiebreaker_ = tiebreaker; + ice_tiebreaker_ = tiebreaker; } IceTransportState P2PTransportChannel::GetState() const { @@ -885,7 +885,6 @@ 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(); @@ -902,7 +901,6 @@ 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(); } } @@ -930,7 +928,7 @@ void P2PTransportChannel::OnPortReady(PortAllocatorSession* session, // if one is pending. port->SetIceRole(ice_role_); - port->SetIceTiebreaker(tiebreaker_); + port->SetIceTiebreaker(ice_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 da7933f2e7..f7472df38a 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 tiebreaker_ RTC_GUARDED_BY(network_thread_); + uint64_t ice_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 5bcfee0473..79e984cfec 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -141,7 +141,6 @@ 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, @@ -299,10 +298,6 @@ 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(); } @@ -3347,7 +3342,6 @@ 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( @@ -3715,7 +3709,6 @@ 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 3745717510..52fc8c1d39 100644 --- a/p2p/base/port_allocator.cc +++ b/p2p/base/port_allocator.cc @@ -68,8 +68,7 @@ PortAllocatorSession::PortAllocatorSession(absl::string_view content_name, content_name_(content_name), component_(component), ice_ufrag_(ice_ufrag), - ice_pwd_(ice_pwd), - tiebreaker_(0) { + ice_pwd_(ice_pwd) { // Pooled sessions are allowed to be created with empty content name, // component, ufrag and password. RTC_DCHECK(ice_ufrag.empty() == ice_pwd.empty()); @@ -101,7 +100,7 @@ PortAllocator::PortAllocator() step_delay_(kDefaultStepDelay), allow_tcp_listen_(true), candidate_filter_(CF_ALL), - tiebreaker_(0) { + tiebreaker_(rtc::CreateRandomId64()) { // The allocator will be attached to a thread in Initialize. thread_checker_.Detach(); } @@ -189,7 +188,6 @@ bool PortAllocator::SetConfiguration( PortAllocatorSession* pooled_session = CreateSessionInternal("", 0, iceCredentials.ufrag, iceCredentials.pwd); pooled_session->set_pooled(true); - pooled_session->set_ice_tiebreaker(tiebreaker_); pooled_session->StartGettingPorts(); pooled_sessions_.push_back( std::unique_ptr(pooled_session)); @@ -197,13 +195,6 @@ 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, @@ -213,7 +204,6 @@ std::unique_ptr PortAllocator::CreateSession( auto session = std::unique_ptr( CreateSessionInternal(content_name, component, ice_ufrag, ice_pwd)); session->SetCandidateFilter(candidate_filter()); - session->set_ice_tiebreaker(tiebreaker_); return session; } diff --git a/p2p/base/port_allocator.h b/p2p/base/port_allocator.h index b8cffca9c0..63ecfc6904 100644 --- a/p2p/base/port_allocator.h +++ b/p2p/base/port_allocator.h @@ -212,10 +212,6 @@ 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): 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", // which would be returned by ReadyCandidates() and ReadyPorts(). @@ -332,9 +328,6 @@ class RTC_EXPORT PortAllocatorSession : public sigslot::has_slots<> { 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; @@ -387,9 +380,6 @@ 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_; @@ -461,6 +451,8 @@ 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 836a2fa494..2df8662f62 100644 --- a/p2p/base/port_allocator_unittest.cc +++ b/p2p/base/port_allocator_unittest.cc @@ -26,7 +26,6 @@ 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: @@ -38,9 +37,7 @@ class PortAllocatorTest : public ::testing::Test, public sigslot::has_slots<> { allocator_(std::make_unique( rtc::Thread::Current(), packet_socket_factory_.get(), - &field_trials_)) { - allocator_->SetIceTiebreaker(kTiebreakerDefault); - } + &field_trials_)) {} protected: void SetConfigurationWithPoolSize(int candidate_pool_size) { diff --git a/p2p/base/regathering_controller_unittest.cc b/p2p/base/regathering_controller_unittest.cc index 91b7270f77..573c0fd23f 100644 --- a/p2p/base/regathering_controller_unittest.cc +++ b/p2p/base/regathering_controller_unittest.cc @@ -40,7 +40,6 @@ 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 @@ -59,7 +58,6 @@ 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 e95033efeb..cc38a66727 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -1483,7 +1483,7 @@ void AllocationSequence::CreateUDPPorts() { } if (port) { - port->SetIceTiebreaker(session_->ice_tiebreaker()); + port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); // If shared socket is enabled, STUN candidate will be allocated by the // UDPPort. if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET)) { @@ -1519,7 +1519,7 @@ void AllocationSequence::CreateTCPPorts() { session_->allocator()->allow_tcp_listen(), session_->allocator()->field_trials()); if (port) { - port->SetIceTiebreaker(session_->ice_tiebreaker()); + port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); session_->AddAllocatedPort(port.release(), this); // Since TCPPort is not created using shared socket, `port` will not be // added to the dequeue. @@ -1549,7 +1549,7 @@ void AllocationSequence::CreateStunPorts() { session_->allocator()->stun_candidate_keepalive_interval(), session_->allocator()->field_trials()); if (port) { - port->SetIceTiebreaker(session_->ice_tiebreaker()); + port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); session_->AddAllocatedPort(port.release(), this); // Since StunPort is not created using shared socket, `port` will not be // added to the dequeue. @@ -1652,7 +1652,7 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config, } } RTC_DCHECK(port != NULL); - port->SetIceTiebreaker(session_->ice_tiebreaker()); + port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); session_->AddAllocatedPort(port.release(), this); } } diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc index 77443eedbb..f77040d128 100644 --- a/p2p/client/basic_port_allocator_unittest.cc +++ b/p2p/client/basic_port_allocator_unittest.cc @@ -112,8 +112,6 @@ 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( @@ -176,7 +174,6 @@ 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(); } @@ -214,7 +211,6 @@ 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. @@ -299,7 +295,6 @@ 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 9473d962ef..d5eb0c633d 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -55,6 +55,7 @@ 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); @@ -62,9 +63,6 @@ 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 448844ac79..7f06c22969 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_ = rtc::CreateRandomId64(); + uint64_t ice_tiebreaker_; rtc::scoped_refptr certificate_; BundleManager bundles_; diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc index 492e108cbc..bd4848cf8b 100644 --- a/pc/peer_connection_ice_unittest.cc +++ b/pc/peer_connection_ice_unittest.cc @@ -94,7 +94,6 @@ using ::testing::Values; constexpr int kIceCandidatesTimeout = 10000; constexpr int64_t kWaitTimeout = 10000; -constexpr uint64_t kTiebreakerDefault = 44444; class PeerConnectionWrapperForIceTest : public PeerConnectionWrapper { public: @@ -1448,7 +1447,6 @@ 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 08fb1632d6..71c85d4c6f 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -114,7 +114,6 @@ 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; @@ -711,7 +710,6 @@ 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.