diff --git a/p2p/base/fake_port_allocator.h b/p2p/base/fake_port_allocator.h index d45d1e4998..09f0e34e51 100644 --- a/p2p/base/fake_port_allocator.h +++ b/p2p/base/fake_port_allocator.h @@ -107,15 +107,16 @@ class FakePortAllocatorSession : public PortAllocatorSession { (rtc::HasIPv6Enabled() && (flags() & PORTALLOCATOR_ENABLE_IPV6)) ? ipv6_network_ : ipv4_network_; - port_.reset(TestUDPPort::Create({.network_thread = network_thread_, - .socket_factory = factory_, - .network = &network, - .ice_username_fragment = username(), - .ice_password = password(), - .field_trials = field_trials_}, - 0, 0, false)); + port_.reset( + TestUDPPort::Create({.network_thread = network_thread_, + .socket_factory = factory_, + .network = &network, + .ice_username_fragment = username(), + .ice_password = password(), + .field_trials = field_trials_, + .ice_tiebreaker = allocator_->ice_tiebreaker()}, + 0, 0, false)); RTC_DCHECK(port_); - port_->SetIceTiebreaker(allocator_->ice_tiebreaker()); port_->SubscribePortDestroyed( [this](PortInterface* port) { OnPortDestroyed(port); }); AddPort(port_.get()); diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index dcea5b0e47..1c44a2f0eb 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -914,7 +914,6 @@ void P2PTransportChannel::OnPortReady(PortAllocatorSession* session, // if one is pending. port->SetIceRole(ice_role_); - port->SetIceTiebreaker(allocator_->ice_tiebreaker()); ports_.push_back(port); port->SignalUnknownAddress.connect(this, &P2PTransportChannel::OnUnknownAddress); diff --git a/p2p/base/port.cc b/p2p/base/port.cc index d9bacf3a5a..45b43b3ac6 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc @@ -118,7 +118,7 @@ Port::Port(const PortParametersRef& args, timeout_delay_(kPortTimeoutDelay), enable_port_packets_(false), ice_role_(ICEROLE_UNKNOWN), - tiebreaker_(0), + ice_tiebreaker_(args.ice_tiebreaker), shared_socket_(shared_socket), network_cost_(args.network->GetCost(*field_trials_)), weak_factory_(this) { @@ -160,12 +160,8 @@ void Port::SetIceRole(IceRole role) { ice_role_ = role; } -void Port::SetIceTiebreaker(uint64_t tiebreaker) { - tiebreaker_ = tiebreaker; -} - uint64_t Port::IceTiebreaker() const { - return tiebreaker_; + return ice_tiebreaker_; } bool Port::SharedSocket() const { @@ -224,8 +220,7 @@ void Port::AddAddress(const rtc::SocketAddress& address, type, generation_, "", network_->id(), network_cost_); // Set the relay protocol before computing the foundation field. c.set_relay_protocol(relay_protocol); - // TODO(bugs.webrtc.org/14605): ensure IceTiebreaker() is set. - c.ComputeFoundation(base_address, tiebreaker_); + c.ComputeFoundation(base_address, ice_tiebreaker_); c.set_priority( c.GetPriority(type_preference, network_->preference(), relay_preference, @@ -650,7 +645,7 @@ bool Port::MaybeIceRoleConflict(const rtc::SocketAddress& addr, switch (ice_role_) { case ICEROLE_CONTROLLING: if (ICEROLE_CONTROLLING == remote_ice_role) { - if (remote_tiebreaker >= tiebreaker_) { + if (remote_tiebreaker >= ice_tiebreaker_) { SignalRoleConflict(this); } else { // Send Role Conflict (487) error response. @@ -662,7 +657,7 @@ bool Port::MaybeIceRoleConflict(const rtc::SocketAddress& addr, break; case ICEROLE_CONTROLLED: if (ICEROLE_CONTROLLED == remote_ice_role) { - if (remote_tiebreaker < tiebreaker_) { + if (remote_tiebreaker < ice_tiebreaker_) { SignalRoleConflict(this); } else { // Send Role Conflict (487) error response. diff --git a/p2p/base/port.h b/p2p/base/port.h index 2df1d35b8e..b9eb343dba 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -180,6 +180,7 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { absl::string_view ice_username_fragment; absl::string_view ice_password; const webrtc::FieldTrialsView* field_trials; + uint64_t ice_tiebreaker; }; protected: @@ -206,7 +207,6 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { IceRole GetIceRole() const override; void SetIceRole(IceRole role) override; - void SetIceTiebreaker(uint64_t tiebreaker) override; uint64_t IceTiebreaker() const override; bool SharedSocket() const override; @@ -490,7 +490,7 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { int timeout_delay_; bool enable_port_packets_; IceRole ice_role_; - uint64_t tiebreaker_; + const uint64_t ice_tiebreaker_; bool shared_socket_; // A virtual cost perceived by the user, usually based on the network type diff --git a/p2p/base/port_interface.h b/p2p/base/port_interface.h index 34f835d138..6847727b10 100644 --- a/p2p/base/port_interface.h +++ b/p2p/base/port_interface.h @@ -60,7 +60,6 @@ class PortInterface { virtual void SetIceRole(IceRole role) = 0; virtual IceRole GetIceRole() const = 0; - virtual void SetIceTiebreaker(uint64_t tiebreaker) = 0; virtual uint64_t IceTiebreaker() const = 0; virtual bool SharedSocket() const = 0; diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc index be3b365427..35c79542df 100644 --- a/p2p/base/port_unittest.cc +++ b/p2p/base/port_unittest.cc @@ -105,10 +105,7 @@ const RelayCredentials kRelayCredentials("test", "test"); const uint32_t kDefaultPrflxPriority = ICE_TYPE_PREFERENCE_PRFLX << 24 | 30 << 8 | (256 - ICE_CANDIDATE_COMPONENT_DEFAULT); - -constexpr int kTiebreaker1 = 11111; -constexpr int kTiebreaker2 = 22222; -constexpr int kTiebreakerDefault = 44444; +constexpr uint64_t kTiebreakerDefault = 44444; const char* data = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"; @@ -535,61 +532,57 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { } std::unique_ptr CreateUdpPort(const SocketAddress& addr, PacketSocketFactory* socket_factory) { - auto port = UDPPort::Create({.network_thread = &main_, - .socket_factory = socket_factory, - .network = MakeNetwork(addr), - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_}, - 0, 0, true, absl::nullopt); - port->SetIceTiebreaker(kTiebreakerDefault); - return port; + return UDPPort::Create({.network_thread = &main_, + .socket_factory = socket_factory, + .network = MakeNetwork(addr), + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_, + .ice_tiebreaker = kTiebreakerDefault}, + 0, 0, true, absl::nullopt); } std::unique_ptr CreateUdpPortMultipleAddrs( const SocketAddress& global_addr, const SocketAddress& link_local_addr, PacketSocketFactory* socket_factory) { - auto port = UDPPort::Create( + return UDPPort::Create( {.network_thread = &main_, .socket_factory = socket_factory, .network = MakeNetworkMultipleAddrs(global_addr, link_local_addr), .ice_username_fragment = username_, .ice_password = password_, - .field_trials = &field_trials_}, + .field_trials = &field_trials_, + .ice_tiebreaker = kTiebreakerDefault}, 0, 0, true, absl::nullopt); - port->SetIceTiebreaker(kTiebreakerDefault); - return port; } std::unique_ptr CreateTcpPort(const SocketAddress& addr) { return CreateTcpPort(addr, &socket_factory_); } std::unique_ptr CreateTcpPort(const SocketAddress& addr, PacketSocketFactory* socket_factory) { - auto port = TCPPort::Create({.network_thread = &main_, - .socket_factory = socket_factory, - .network = MakeNetwork(addr), - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_}, - 0, 0, true); - port->SetIceTiebreaker(kTiebreakerDefault); - return port; + return TCPPort::Create({.network_thread = &main_, + .socket_factory = socket_factory, + .network = MakeNetwork(addr), + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_, + .ice_tiebreaker = kTiebreakerDefault}, + 0, 0, true); } std::unique_ptr CreateStunPort( const SocketAddress& addr, rtc::PacketSocketFactory* socket_factory) { ServerAddresses stun_servers; stun_servers.insert(kStunAddr); - auto port = StunPort::Create({.network_thread = &main_, - .socket_factory = socket_factory, - .network = MakeNetwork(addr), - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_}, - 0, 0, stun_servers, absl::nullopt); - port->SetIceTiebreaker(kTiebreakerDefault); - return port; + return StunPort::Create({.network_thread = &main_, + .socket_factory = socket_factory, + .network = MakeNetwork(addr), + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_, + .ice_tiebreaker = kTiebreakerDefault}, + 0, 0, stun_servers, absl::nullopt); } std::unique_ptr CreateRelayPort(const SocketAddress& addr, ProtocolType int_proto, @@ -623,10 +616,9 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { args.server_address = &server_address; args.config = &config; args.field_trials = &field_trials_; + args.ice_tiebreaker = kTiebreakerDefault; - auto port = TurnPort::Create(args, 0, 0); - port->SetIceTiebreaker(kTiebreakerDefault); - return port; + return TurnPort::Create(args, 0, 0); } std::unique_ptr CreateNatServer(const SocketAddress& addr, @@ -832,7 +824,8 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { .network = MakeNetwork(addr), .ice_username_fragment = username, .ice_password = password, - .field_trials = field_trials}; + .field_trials = field_trials, + .ice_tiebreaker = kTiebreakerDefault}; auto port = std::make_unique(args, 0, 0); port->SignalRoleConflict.connect(this, &PortTest::OnRoleConflict); return port; @@ -840,11 +833,9 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { std::unique_ptr CreateTestPort(const rtc::SocketAddress& addr, absl::string_view username, absl::string_view password, - cricket::IceRole role, - int tiebreaker) { + cricket::IceRole role) { auto port = CreateTestPort(addr, username, password); port->SetIceRole(role); - port->SetIceTiebreaker(tiebreaker); return port; } // Overload to create a test port given an rtc::Network directly. @@ -856,7 +847,8 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { .network = network, .ice_username_fragment = username, .ice_password = password, - .field_trials = nullptr}; + .field_trials = nullptr, + .ice_tiebreaker = kTiebreakerDefault}; auto port = std::make_unique(args, 0, 0); port->SignalRoleConflict.connect(this, &PortTest::OnRoleConflict); return port; @@ -1443,10 +1435,8 @@ TEST_F(PortTest, TestConnectionDeadWithDeadConnectionTimeout) { TEST_F(PortTest, TestConnectionDeadOutstandingPing) { auto port1 = CreateUdpPort(kLocalAddr1); port1->SetIceRole(cricket::ICEROLE_CONTROLLING); - port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); port2->SetIceRole(cricket::ICEROLE_CONTROLLED); - port2->SetIceTiebreaker(kTiebreaker2); TestChannel ch1(std::move(port1)); TestChannel ch2(std::move(port2)); @@ -1486,10 +1476,8 @@ TEST_F(PortTest, TestConnectionDeadOutstandingPing) { TEST_F(PortTest, TestLocalToLocalStandard) { auto port1 = CreateUdpPort(kLocalAddr1); port1->SetIceRole(cricket::ICEROLE_CONTROLLING); - port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); port2->SetIceRole(cricket::ICEROLE_CONTROLLED); - port2->SetIceTiebreaker(kTiebreaker2); // Same parameters as TestLocalToLocal above. TestConnectivity("udp", std::move(port1), "udp", std::move(port2), true, true, true, true); @@ -1502,7 +1490,6 @@ TEST_F(PortTest, TestLocalToLocalStandard) { TEST_F(PortTest, TestLoopbackCall) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport->SetIceTiebreaker(kTiebreaker1); lport->PrepareAddress(); ASSERT_FALSE(lport->Candidates().empty()); Connection* conn = @@ -1539,7 +1526,7 @@ TEST_F(PortTest, TestLoopbackCall) { // To make sure we receive error response, adding tiebreaker less than // what's present in request. modified_req->AddAttribute(std::make_unique( - STUN_ATTR_ICE_CONTROLLING, kTiebreaker1 - 1)); + STUN_ATTR_ICE_CONTROLLING, lport->IceTiebreaker() - 1)); modified_req->AddMessageIntegrity("lpass"); modified_req->AddFingerprint(); @@ -1562,10 +1549,8 @@ TEST_F(PortTest, TestLoopbackCall) { TEST_F(PortTest, TestIceRoleConflict) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport->SetIceTiebreaker(kTiebreaker1); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); rport->SetIceRole(cricket::ICEROLE_CONTROLLING); - rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -1668,9 +1653,7 @@ TEST_F(PortTest, TestDisableInterfaceOfTcpPort) { rsocket->Bind(kLocalAddr2); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); - rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -1809,7 +1792,6 @@ TEST_F(PortTest, TestUdpMultipleAddressesV6CrossTypePorts) { factory.set_next_udp_socket(socket); ports[i] = CreateUdpPortMultipleAddrs(addresses[i], kLinkLocalIPv6Addr, &factory); - ports[i]->SetIceTiebreaker(kTiebreakerDefault); socket->set_state(AsyncPacketSocket::STATE_BINDING); socket->SignalAddressReady(socket, addresses[i]); ports[i]->PrepareAddress(); @@ -1867,9 +1849,7 @@ TEST_F(PortTest, TestSendStunMessage) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); - rport->SetIceTiebreaker(kTiebreaker2); // Send a fake ping from lport to rport. lport->PrepareAddress(); @@ -2030,9 +2010,7 @@ TEST_F(PortTest, TestNomination) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); - rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -2086,9 +2064,7 @@ TEST_F(PortTest, TestRoundTripTime) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); - rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -2125,9 +2101,7 @@ TEST_F(PortTest, TestUseCandidateAttribute) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); - rport->SetIceTiebreaker(kTiebreaker2); // Send a fake ping from lport to rport. lport->PrepareAddress(); @@ -2154,9 +2128,7 @@ TEST_F(PortTest, TestNetworkCostChange) { auto lport = CreateTestPort(test_network, "lfrag", "lpass"); auto rport = CreateTestPort(test_network, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); - rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -2211,9 +2183,7 @@ TEST_F(PortTest, TestNetworkInfoAttribute) { auto lport = CreateTestPort(test_network, "lfrag", "lpass"); auto rport = CreateTestPort(test_network, "rfrag", "rpass"); lport->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport->SetIceTiebreaker(kTiebreaker1); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); - rport->SetIceTiebreaker(kTiebreaker2); uint16_t lnetwork_id = 9; test_network->set_id(lnetwork_id); @@ -2514,9 +2484,9 @@ TEST_F(PortTest, TestHandleStunResponseWithUnknownComprehensionRequiredAttribute) { // Generic setup. auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING, kTiebreakerDefault); + cricket::ICEROLE_CONTROLLING); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED, kTiebreakerDefault); + cricket::ICEROLE_CONTROLLED); lport->PrepareAddress(); rport->PrepareAddress(); ASSERT_FALSE(lport->Candidates().empty()); @@ -2553,9 +2523,9 @@ TEST_F(PortTest, TestHandleStunIndicationWithUnknownComprehensionRequiredAttribute) { // Generic set up. auto lport = CreateTestPort(kLocalAddr2, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING, kTiebreakerDefault); + cricket::ICEROLE_CONTROLLING); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED, kTiebreakerDefault); + cricket::ICEROLE_CONTROLLED); lport->PrepareAddress(); rport->PrepareAddress(); ASSERT_FALSE(lport->Candidates().empty()); @@ -2579,7 +2549,7 @@ TEST_F(PortTest, // indications are allowed only to the connection which is in read mode. TEST_F(PortTest, TestHandleStunBindingIndication) { auto lport = CreateTestPort(kLocalAddr2, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING, kTiebreaker1); + cricket::ICEROLE_CONTROLLING); // Verifying encoding and decoding STUN indication message. std::unique_ptr in_msg, out_msg; @@ -2600,7 +2570,6 @@ TEST_F(PortTest, TestHandleStunBindingIndication) { // last_ping_received. auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); rport->SetIceRole(cricket::ICEROLE_CONTROLLED); - rport->SetIceTiebreaker(kTiebreaker2); lport->PrepareAddress(); rport->PrepareAddress(); @@ -2635,7 +2604,6 @@ TEST_F(PortTest, TestHandleStunBindingIndication) { TEST_F(PortTest, TestComputeCandidatePriority) { auto port = CreateTestPort(kLocalAddr1, "name", "pass"); - port->SetIceTiebreaker(kTiebreakerDefault); port->set_type_preference(90); port->set_component(177); port->AddCandidateAddress(SocketAddress("192.168.1.4", 1234)); @@ -2673,7 +2641,6 @@ TEST_F(PortTest, TestComputeCandidatePriorityWithPriorityAdjustment) { webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IncreaseIceCandidatePriorityHostSrflx/Enabled/"); auto port = CreateTestPort(kLocalAddr1, "name", "pass", &field_trials); - port->SetIceTiebreaker(kTiebreakerDefault); port->set_type_preference(90); port->set_component(177); port->AddCandidateAddress(SocketAddress("192.168.1.4", 1234)); @@ -2711,7 +2678,6 @@ TEST_F(PortTest, TestComputeCandidatePriorityWithPriorityAdjustment) { // Test that candidates with different types will have different foundation. TEST_F(PortTest, TestFoundation) { auto testport = CreateTestPort(kLocalAddr1, "name", "pass"); - testport->SetIceTiebreaker(kTiebreakerDefault); testport->AddCandidateAddress(kLocalAddr1, kLocalAddr1, IceCandidateType::kHost, cricket::ICE_TYPE_PREFERENCE_HOST, false); @@ -2834,11 +2800,9 @@ TEST_F(PortTest, TestCandidatePriority) { // Test the Connection priority is calculated correctly. TEST_F(PortTest, TestConnectionPriority) { auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); - lport->SetIceTiebreaker(kTiebreakerDefault); lport->set_type_preference(cricket::ICE_TYPE_PREFERENCE_HOST); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); - rport->SetIceTiebreaker(kTiebreakerDefault); rport->set_type_preference(cricket::ICE_TYPE_PREFERENCE_RELAY_UDP); lport->set_component(123); lport->AddCandidateAddress(SocketAddress("192.168.1.4", 1234)); @@ -2876,11 +2840,9 @@ TEST_F(PortTest, TestConnectionPriorityWithPriorityAdjustment) { webrtc::test::ScopedKeyValueConfig field_trials( "WebRTC-IncreaseIceCandidatePriorityHostSrflx/Enabled/"); auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass", &field_trials); - lport->SetIceTiebreaker(kTiebreakerDefault); lport->set_type_preference(cricket::ICE_TYPE_PREFERENCE_HOST); auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass", &field_trials); - rport->SetIceTiebreaker(kTiebreakerDefault); rport->set_type_preference(cricket::ICE_TYPE_PREFERENCE_RELAY_UDP); lport->set_component(123); lport->AddCandidateAddress(SocketAddress("192.168.1.4", 1234)); @@ -3085,13 +3047,12 @@ TEST_F(PortTest, TestTimeoutForNeverWritable) { // In this test `ch1` behaves like FULL mode client and we have created // port which responds to the ping message just like LITE client. TEST_F(PortTest, TestIceLiteConnectivity) { - auto ice_full_port = - CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING, kTiebreaker1); + auto ice_full_port = CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING); auto* ice_full_port_ptr = ice_full_port.get(); - auto ice_lite_port = CreateTestPort( - kLocalAddr2, "rfrag", "rpass", cricket::ICEROLE_CONTROLLED, kTiebreaker2); + auto ice_lite_port = CreateTestPort(kLocalAddr2, "rfrag", "rpass", + cricket::ICEROLE_CONTROLLED); // Setup TestChannel. This behaves like FULL mode client. TestChannel ch1(std::move(ice_full_port)); ch1.SetIceMode(ICEMODE_FULL); @@ -3197,12 +3158,11 @@ TEST_P(GoogPingTest, TestGoogPingAnnounceEnable) { << trials.announce_goog_ping << " enable:" << trials.enable_goog_ping; - auto port1_unique = - CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING, kTiebreaker1); + auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED, kTiebreaker2); + cricket::ICEROLE_CONTROLLED); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3288,12 +3248,11 @@ TEST_F(PortTest, TestGoogPingUnsupportedVersionInStunBinding) { trials.announce_goog_ping = true; trials.enable_goog_ping = true; - auto port1_unique = - CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING, kTiebreaker1); + auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED, kTiebreaker2); + cricket::ICEROLE_CONTROLLED); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3359,12 +3318,11 @@ TEST_F(PortTest, TestGoogPingUnsupportedVersionInStunBindingResponse) { trials.announce_goog_ping = true; trials.enable_goog_ping = true; - auto port1_unique = - CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING, kTiebreaker1); + auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED, kTiebreaker2); + cricket::ICEROLE_CONTROLLED); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3460,12 +3418,11 @@ TEST_F(PortTest, TestChangeInAttributeMakesGoogPingFallsbackToStunBinding) { trials.announce_goog_ping = true; trials.enable_goog_ping = true; - auto port1_unique = - CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING, kTiebreaker1); + auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED, kTiebreaker2); + cricket::ICEROLE_CONTROLLED); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3545,12 +3502,11 @@ TEST_F(PortTest, TestErrorResponseMakesGoogPingFallBackToStunBinding) { trials.announce_goog_ping = true; trials.enable_goog_ping = true; - auto port1_unique = - CreateTestPort(kLocalAddr1, "lfrag", "lpass", - cricket::ICEROLE_CONTROLLING, kTiebreaker1); + auto port1_unique = CreateTestPort(kLocalAddr1, "lfrag", "lpass", + cricket::ICEROLE_CONTROLLING); auto* port1 = port1_unique.get(); auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass", - cricket::ICEROLE_CONTROLLED, kTiebreaker2); + cricket::ICEROLE_CONTROLLED); TestChannel ch1(std::move(port1_unique)); // Block usage of STUN_ATTR_USE_CANDIDATE so that @@ -3641,13 +3597,11 @@ TEST_F(PortTest, TestPortTimeoutIfNotKeptAlive) { ConnectToSignalDestroyed(port1.get()); port1->set_timeout_delay(timeout_delay); // milliseconds port1->SetIceRole(cricket::ICEROLE_CONTROLLING); - port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); ConnectToSignalDestroyed(port2.get()); port2->set_timeout_delay(timeout_delay); // milliseconds port2->SetIceRole(cricket::ICEROLE_CONTROLLED); - port2->SetIceTiebreaker(kTiebreaker2); // Set up channels and ensure both ports will be deleted. TestChannel ch1(std::move(port1)); @@ -3670,14 +3624,12 @@ TEST_F(PortTest, TestPortTimeoutAfterNewConnectionCreatedAndDestroyed) { ConnectToSignalDestroyed(port1.get()); port1->set_timeout_delay(timeout_delay); // milliseconds port1->SetIceRole(cricket::ICEROLE_CONTROLLING); - port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); ConnectToSignalDestroyed(port2.get()); port2->set_timeout_delay(timeout_delay); // milliseconds port2->SetIceRole(cricket::ICEROLE_CONTROLLED); - port2->SetIceTiebreaker(kTiebreaker2); // Set up channels and ensure both ports will be deleted. TestChannel ch1(std::move(port1)); @@ -3711,13 +3663,11 @@ TEST_F(PortTest, TestPortNotTimeoutUntilPruned) { ConnectToSignalDestroyed(port1.get()); port1->set_timeout_delay(timeout_delay); // milliseconds port1->SetIceRole(cricket::ICEROLE_CONTROLLING); - port1->SetIceTiebreaker(kTiebreaker1); auto port2 = CreateUdpPort(kLocalAddr2); ConnectToSignalDestroyed(port2.get()); port2->set_timeout_delay(timeout_delay); // milliseconds port2->SetIceRole(cricket::ICEROLE_CONTROLLED); - port2->SetIceTiebreaker(kTiebreaker2); // The connection must not be destroyed before a connection is attempted. EXPECT_EQ(0, ports_destroyed()); @@ -3766,7 +3716,6 @@ TEST_F(PortTest, TestSupportsProtocol) { // on both the port itself and its candidates. TEST_F(PortTest, TestSetIceParameters) { auto port = CreateTestPort(kLocalAddr1, "ufrag1", "password1"); - port->SetIceTiebreaker(kTiebreakerDefault); port->PrepareAddress(); EXPECT_EQ(1UL, port->Candidates().size()); port->SetIceParameters(1, "ufrag2", "password2"); @@ -3781,7 +3730,6 @@ TEST_F(PortTest, TestSetIceParameters) { TEST_F(PortTest, TestAddConnectionWithSameAddress) { auto port = CreateTestPort(kLocalAddr1, "ufrag1", "password1"); - port->SetIceTiebreaker(kTiebreakerDefault); port->PrepareAddress(); EXPECT_EQ(1u, port->Candidates().size()); rtc::SocketAddress address("1.1.1.1", 5000); @@ -3817,9 +3765,7 @@ class ConnectionTest : public PortTest { lport_ = CreateTestPort(kLocalAddr1, "lfrag", "lpass"); rport_ = CreateTestPort(kLocalAddr2, "rfrag", "rpass"); lport_->SetIceRole(cricket::ICEROLE_CONTROLLING); - lport_->SetIceTiebreaker(kTiebreaker1); rport_->SetIceRole(cricket::ICEROLE_CONTROLLED); - rport_->SetIceTiebreaker(kTiebreaker2); lport_->PrepareAddress(); rport_->PrepareAddress(); diff --git a/p2p/base/stun_port_unittest.cc b/p2p/base/stun_port_unittest.cc index 423d16b11b..1277d58e38 100644 --- a/p2p/base/stun_port_unittest.cc +++ b/p2p/base/stun_port_unittest.cc @@ -142,9 +142,9 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { .network = &network_, .ice_username_fragment = rtc::CreateRandomString(16), .ice_password = rtc::CreateRandomString(22), - .field_trials = field_trials}, + .field_trials = field_trials, + .ice_tiebreaker = kTiebreakerDefault}, 0, 0, stun_servers, absl::nullopt); - stun_port_->SetIceTiebreaker(kTiebreakerDefault); stun_port_->set_stun_keepalive_delay(stun_keepalive_delay_); // If `stun_keepalive_lifetime_` is negative, let the stun port // choose its lifetime from the network type. @@ -179,10 +179,10 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { .network = &network_, .ice_username_fragment = rtc::CreateRandomString(16), .ice_password = rtc::CreateRandomString(22), - .field_trials = field_trials}, + .field_trials = field_trials, + .ice_tiebreaker = kTiebreakerDefault}, socket_.get(), false, absl::nullopt); ASSERT_TRUE(stun_port_ != NULL); - stun_port_->SetIceTiebreaker(kTiebreakerDefault); ServerAddresses stun_servers; stun_servers.insert(server_addr); stun_port_->set_server_addresses(stun_servers); diff --git a/p2p/base/tcp_port_unittest.cc b/p2p/base/tcp_port_unittest.cc index be246a8576..fa3f7f329b 100644 --- a/p2p/base/tcp_port_unittest.cc +++ b/p2p/base/tcp_port_unittest.cc @@ -83,29 +83,28 @@ class TCPPortTest : public ::testing::Test, public sigslot::has_slots<> { } std::unique_ptr CreateTCPPort(const SocketAddress& addr) { - auto port = std::unique_ptr( - TCPPort::Create({.network_thread = &main_, - .socket_factory = &socket_factory_, - .network = MakeNetwork(addr), - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_}, - 0, 0, true)); - port->SetIceTiebreaker(kTiebreakerDefault); - return port; + return TCPPort::Create( + { + .network_thread = &main_, + .socket_factory = &socket_factory_, + .network = MakeNetwork(addr), + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_, + .ice_tiebreaker = kTiebreakerDefault, + }, + 0, 0, true); } std::unique_ptr CreateTCPPort(const rtc::Network* network) { - auto port = std::unique_ptr( - TCPPort::Create({.network_thread = &main_, - .socket_factory = &socket_factory_, - .network = network, - .ice_username_fragment = username_, - .ice_password = password_, - .field_trials = &field_trials_}, - 0, 0, true)); - port->SetIceTiebreaker(kTiebreakerDefault); - return port; + return TCPPort::Create({.network_thread = &main_, + .socket_factory = &socket_factory_, + .network = network, + .ice_username_fragment = username_, + .ice_password = password_, + .field_trials = &field_trials_, + .ice_tiebreaker = kTiebreakerDefault}, + 0, 0, true); } protected: diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index f8783d5daf..299a8980db 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -80,17 +80,20 @@ class TurnPort : public Port { return nullptr; } // Using `new` to access a non-public constructor. - return absl::WrapUnique( - new TurnPort({.network_thread = args.network_thread, - .socket_factory = args.socket_factory, - .network = args.network, - .ice_username_fragment = args.username, - .ice_password = args.password, - .field_trials = args.field_trials}, - socket, *args.server_address, args.config->credentials, - args.relative_priority, args.config->tls_alpn_protocols, - args.config->tls_elliptic_curves, args.turn_customizer, - args.config->tls_cert_verifier)); + return absl::WrapUnique(new TurnPort( + { + .network_thread = args.network_thread, + .socket_factory = args.socket_factory, + .network = args.network, + .ice_username_fragment = args.username, + .ice_password = args.password, + .field_trials = args.field_trials, + .ice_tiebreaker = args.ice_tiebreaker, + }, + socket, *args.server_address, args.config->credentials, + args.relative_priority, args.config->tls_alpn_protocols, + args.config->tls_elliptic_curves, args.turn_customizer, + args.config->tls_cert_verifier)); } // Create a TURN port that will use a new socket, bound to `network` and diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index 66a154048b..1a62cf3751 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -295,6 +295,7 @@ class TurnPortTest : public ::testing::Test, args.config = &config; args.turn_customizer = turn_customizer_.get(); args.field_trials = &field_trials_; + args.ice_tiebreaker = kTiebreakerDefault; turn_port_ = TurnPort::Create(args, 0, 0); if (!turn_port_) { @@ -302,7 +303,6 @@ class TurnPortTest : public ::testing::Test, } // This TURN port will be the controlling. turn_port_->SetIceRole(ICEROLE_CONTROLLING); - turn_port_->SetIceTiebreaker(kTiebreakerDefault); ConnectSignals(); if (server_address.proto == cricket::PROTO_TLS) { @@ -343,10 +343,10 @@ class TurnPortTest : public ::testing::Test, args.config = &config; args.turn_customizer = turn_customizer_.get(); args.field_trials = &field_trials_; + args.ice_tiebreaker = kTiebreakerDefault; turn_port_ = TurnPort::Create(args, socket_.get()); // This TURN port will be the controlling. turn_port_->SetIceRole(ICEROLE_CONTROLLING); - turn_port_->SetIceTiebreaker(kTiebreakerDefault); ConnectSignals(); } @@ -371,11 +371,11 @@ class TurnPortTest : public ::testing::Test, .network = MakeNetwork(address), .ice_username_fragment = kIceUfrag2, .ice_password = kIcePwd2, - .field_trials = &field_trials_}, + .field_trials = &field_trials_, + .ice_tiebreaker = kTiebreakerDefault}, 0, 0, false, absl::nullopt); // UDP port will be controlled. udp_port_->SetIceRole(ICEROLE_CONTROLLED); - udp_port_->SetIceTiebreaker(kTiebreakerDefault); udp_port_->SignalPortComplete.connect(this, &TurnPortTest::OnUdpPortComplete); } diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc index f1525b0589..ae136fe35f 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -1475,7 +1475,8 @@ void AllocationSequence::CreateUDPPorts() { .network = network_, .ice_username_fragment = session_->username(), .ice_password = session_->password(), - .field_trials = session_->allocator()->field_trials()}, + .field_trials = session_->allocator()->field_trials(), + .ice_tiebreaker = session_->allocator()->ice_tiebreaker()}, udp_socket_.get(), emit_local_candidate_for_anyaddress, session_->allocator()->stun_candidate_keepalive_interval()); } else { @@ -1485,14 +1486,14 @@ void AllocationSequence::CreateUDPPorts() { .network = network_, .ice_username_fragment = session_->username(), .ice_password = session_->password(), - .field_trials = session_->allocator()->field_trials()}, + .field_trials = session_->allocator()->field_trials(), + .ice_tiebreaker = session_->allocator()->ice_tiebreaker()}, session_->allocator()->min_port(), session_->allocator()->max_port(), emit_local_candidate_for_anyaddress, session_->allocator()->stun_candidate_keepalive_interval()); } if (port) { - 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)) { @@ -1527,12 +1528,12 @@ void AllocationSequence::CreateTCPPorts() { .network = network_, .ice_username_fragment = session_->username(), .ice_password = session_->password(), - .field_trials = session_->allocator()->field_trials()}, + .field_trials = session_->allocator()->field_trials(), + .ice_tiebreaker = session_->allocator()->ice_tiebreaker()}, session_->allocator()->min_port(), session_->allocator()->max_port(), session_->allocator()->allow_tcp_listen()); if (port) { - 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. @@ -1561,12 +1562,12 @@ void AllocationSequence::CreateStunPorts() { .network = network_, .ice_username_fragment = session_->username(), .ice_password = session_->password(), - .field_trials = session_->allocator()->field_trials()}, + .field_trials = session_->allocator()->field_trials(), + .ice_tiebreaker = session_->allocator()->ice_tiebreaker()}, session_->allocator()->min_port(), session_->allocator()->max_port(), config_->StunServers(), session_->allocator()->stun_candidate_keepalive_interval()); if (port) { - 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. @@ -1634,6 +1635,7 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config, args.turn_customizer = session_->allocator()->turn_customizer(); args.field_trials = session_->allocator()->field_trials(); args.relative_priority = relative_priority; + args.ice_tiebreaker = session_->allocator()->ice_tiebreaker(); std::unique_ptr port; // Shared socket mode must be enabled only for UDP based ports. Hence @@ -1669,7 +1671,6 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config, } } RTC_DCHECK(port != NULL); - port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); session_->AddAllocatedPort(port.release(), this); } } diff --git a/p2p/client/relay_port_factory_interface.h b/p2p/client/relay_port_factory_interface.h index edfca3697b..bdc8c3d448 100644 --- a/p2p/client/relay_port_factory_interface.h +++ b/p2p/client/relay_port_factory_interface.h @@ -49,6 +49,7 @@ struct CreateRelayPortArgs { // to the candidates from other servers. Required because ICE priorities // need to be unique. int relative_priority = 0; + uint64_t ice_tiebreaker = 0; }; // A factory for creating RelayPort's.