diff --git a/p2p/base/fake_port_allocator.h b/p2p/base/fake_port_allocator.h index 0bb7dd0139..706635778b 100644 --- a/p2p/base/fake_port_allocator.h +++ b/p2p/base/fake_port_allocator.h @@ -92,6 +92,7 @@ class FakePortAllocatorSession : public PortAllocatorSession { component, ice_ufrag, ice_pwd, + /*kTiebreakerDefault = */ 44444, allocator->flags()), network_thread_(network_thread), factory_(factory), @@ -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 { 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..30bcfb1732 100644 --- a/p2p/base/port_allocator.cc +++ b/p2p/base/port_allocator.cc @@ -62,6 +62,7 @@ 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), @@ -69,7 +70,7 @@ PortAllocatorSession::PortAllocatorSession(absl::string_view content_name, component_(component), ice_ufrag_(ice_ufrag), ice_pwd_(ice_pwd), - tiebreaker_(0) { + ice_tiebreaker_(ice_tiebreaker) { // 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 +102,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(); } @@ -197,13 +198,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, diff --git a/p2p/base/port_allocator.h b/p2p/base/port_allocator.h index b8cffca9c0..e9630df143 100644 --- a/p2p/base/port_allocator.h +++ b/p2p/base/port_allocator.h @@ -199,6 +199,7 @@ 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. @@ -212,9 +213,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): move this to the constructor - void set_ice_tiebreaker(uint64_t tiebreaker) { tiebreaker_ = tiebreaker; } - uint64_t ice_tiebreaker() const { return tiebreaker_; } + // 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_; } // Setting this filter should affect not only candidates gathered in the // future, but candidates already gathered and ports already "ready", @@ -329,12 +330,10 @@ 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; @@ -387,9 +386,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 +457,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..92c0ac37f6 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)); + std::string(ice_pwd), ice_tiebreaker()); } void BasicPortAllocator::AddTurnServerForTesting( @@ -261,11 +261,13 @@ BasicPortAllocatorSession::BasicPortAllocatorSession( absl::string_view content_name, int component, absl::string_view ice_ufrag, - absl::string_view ice_pwd) + absl::string_view ice_pwd, + uint64_t ice_tiebreaker) : PortAllocatorSession(content_name, component, ice_ufrag, ice_pwd, + ice_tiebreaker, allocator->flags()), allocator_(allocator), network_thread_(rtc::Thread::Current()), @@ -281,6 +283,19 @@ 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 25201fd016..1612f77d35 100644 --- a/p2p/client/basic_port_allocator.h +++ b/p2p/client/basic_port_allocator.h @@ -116,6 +116,13 @@ 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 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.