diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc index 7e8b4d3478..dc78c58c56 100644 --- a/p2p/base/port_unittest.cc +++ b/p2p/base/port_unittest.cc @@ -485,26 +485,25 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { } std::unique_ptr CreateUdpPort(const SocketAddress& addr, PacketSocketFactory* socket_factory) { - return absl::WrapUnique(UDPPort::Create( - &main_, socket_factory, MakeNetwork(addr), 0, 0, username_, password_, - std::string(), true, absl::nullopt)); + return UDPPort::Create(&main_, socket_factory, MakeNetwork(addr), 0, 0, + username_, password_, std::string(), true, + absl::nullopt); } std::unique_ptr CreateTcpPort(const SocketAddress& addr) { return CreateTcpPort(addr, &socket_factory_); } std::unique_ptr CreateTcpPort(const SocketAddress& addr, PacketSocketFactory* socket_factory) { - return absl::WrapUnique(TCPPort::Create(&main_, socket_factory, - MakeNetwork(addr), 0, 0, username_, - password_, true)); + return TCPPort::Create(&main_, socket_factory, MakeNetwork(addr), 0, 0, + username_, password_, true); } std::unique_ptr CreateStunPort(const SocketAddress& addr, rtc::PacketSocketFactory* factory) { ServerAddresses stun_servers; stun_servers.insert(kStunAddr); - return absl::WrapUnique(StunPort::Create( - &main_, factory, MakeNetwork(addr), 0, 0, username_, password_, - stun_servers, std::string(), absl::nullopt)); + return StunPort::Create(&main_, factory, MakeNetwork(addr), 0, 0, username_, + password_, stun_servers, std::string(), + absl::nullopt); } std::unique_ptr CreateRelayPort(const SocketAddress& addr, RelayType rtype, @@ -531,10 +530,10 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { ProtocolType int_proto, ProtocolType ext_proto, const rtc::SocketAddress& server_addr) { - return absl::WrapUnique(TurnPort::Create( + return TurnPort::CreateUnique( &main_, socket_factory, MakeNetwork(addr), 0, 0, username_, password_, ProtocolAddress(server_addr, int_proto), kRelayCredentials, 0, "", {}, - {}, nullptr, nullptr)); + {}, nullptr, nullptr); } std::unique_ptr CreateGturnPort(const SocketAddress& addr, ProtocolType int_proto, @@ -549,9 +548,8 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { // TODO(pthatcher): Remove GTURN. // Generate a username with length of 16 for Gturn only. std::string username = rtc::CreateRandomString(kGturnUserNameLength); - return absl::WrapUnique(RelayPort::Create(&main_, &socket_factory_, - MakeNetwork(addr), 0, 0, username, - password_)); + return RelayPort::Create(&main_, &socket_factory_, MakeNetwork(addr), 0, 0, + username, password_); // TODO(?): Add an external address for ext_proto, so that the // other side can connect to this port using a non-UDP protocol. } diff --git a/p2p/base/relayport.h b/p2p/base/relayport.h index 48a939e0cf..7c67915dc0 100644 --- a/p2p/base/relayport.h +++ b/p2p/base/relayport.h @@ -12,6 +12,7 @@ #define P2P_BASE_RELAYPORT_H_ #include +#include #include #include #include @@ -35,15 +36,16 @@ class RelayPort : public Port { typedef std::pair OptionValue; // RelayPort doesn't yet do anything fancy in the ctor. - static RelayPort* Create(rtc::Thread* thread, - rtc::PacketSocketFactory* factory, - rtc::Network* network, - uint16_t min_port, - uint16_t max_port, - const std::string& username, - const std::string& password) { - return new RelayPort(thread, factory, network, min_port, max_port, username, - password); + static std::unique_ptr Create(rtc::Thread* thread, + rtc::PacketSocketFactory* factory, + rtc::Network* network, + uint16_t min_port, + uint16_t max_port, + const std::string& username, + const std::string& password) { + // Using `new` to access a non-public constructor. + return absl::WrapUnique(new RelayPort(thread, factory, network, min_port, + max_port, username, password)); } ~RelayPort() override; diff --git a/p2p/base/stunport.cc b/p2p/base/stunport.cc index c60bf92eb4..e09351e83b 100644 --- a/p2p/base/stunport.cc +++ b/p2p/base/stunport.cc @@ -563,22 +563,24 @@ bool UDPPort::HasCandidateWithAddress(const rtc::SocketAddress& addr) const { return false; } -StunPort* StunPort::Create(rtc::Thread* thread, - rtc::PacketSocketFactory* factory, - rtc::Network* network, - uint16_t min_port, - uint16_t max_port, - const std::string& username, - const std::string& password, - const ServerAddresses& servers, - const std::string& origin, - absl::optional stun_keepalive_interval) { - StunPort* port = new StunPort(thread, factory, network, min_port, max_port, - username, password, servers, origin); +std::unique_ptr StunPort::Create( + rtc::Thread* thread, + rtc::PacketSocketFactory* factory, + rtc::Network* network, + uint16_t min_port, + uint16_t max_port, + const std::string& username, + const std::string& password, + const ServerAddresses& servers, + const std::string& origin, + absl::optional stun_keepalive_interval) { + // Using `new` to access a non-public constructor. + auto port = absl::WrapUnique(new StunPort(thread, factory, network, min_port, + max_port, username, password, + servers, origin)); port->set_stun_keepalive_delay(stun_keepalive_interval); if (!port->Init()) { - delete port; - port = NULL; + return nullptr; } return port; } diff --git a/p2p/base/stunport.h b/p2p/base/stunport.h index 5e397835db..e719de4a6c 100644 --- a/p2p/base/stunport.h +++ b/p2p/base/stunport.h @@ -35,42 +35,45 @@ static const int HIGH_COST_PORT_KEEPALIVE_LIFETIME = 2 * 60 * 1000; // Communicates using the address on the outside of a NAT. class UDPPort : public Port { public: - static UDPPort* Create(rtc::Thread* thread, - rtc::PacketSocketFactory* factory, - rtc::Network* network, - rtc::AsyncPacketSocket* socket, - const std::string& username, - const std::string& password, - const std::string& origin, - bool emit_local_for_anyaddress, - absl::optional stun_keepalive_interval) { - UDPPort* port = new UDPPort(thread, factory, network, socket, username, - password, origin, emit_local_for_anyaddress); + static std::unique_ptr Create( + rtc::Thread* thread, + rtc::PacketSocketFactory* factory, + rtc::Network* network, + rtc::AsyncPacketSocket* socket, + const std::string& username, + const std::string& password, + const std::string& origin, + bool emit_local_for_anyaddress, + absl::optional stun_keepalive_interval) { + // Using `new` to access a non-public constructor. + auto port = absl::WrapUnique(new UDPPort(thread, factory, network, socket, + username, password, origin, + emit_local_for_anyaddress)); port->set_stun_keepalive_delay(stun_keepalive_interval); if (!port->Init()) { - delete port; - port = NULL; + return nullptr; } return port; } - static UDPPort* Create(rtc::Thread* thread, - rtc::PacketSocketFactory* factory, - rtc::Network* network, - uint16_t min_port, - uint16_t max_port, - const std::string& username, - const std::string& password, - const std::string& origin, - bool emit_local_for_anyaddress, - absl::optional stun_keepalive_interval) { - UDPPort* port = + static std::unique_ptr Create( + rtc::Thread* thread, + rtc::PacketSocketFactory* factory, + rtc::Network* network, + uint16_t min_port, + uint16_t max_port, + const std::string& username, + const std::string& password, + const std::string& origin, + bool emit_local_for_anyaddress, + absl::optional stun_keepalive_interval) { + // Using `new` to access a non-public constructor. + auto port = absl::WrapUnique( new UDPPort(thread, factory, network, min_port, max_port, username, - password, origin, emit_local_for_anyaddress); + password, origin, emit_local_for_anyaddress)); port->set_stun_keepalive_delay(stun_keepalive_interval); if (!port->Init()) { - delete port; - port = NULL; + return nullptr; } return port; } @@ -262,16 +265,17 @@ class UDPPort : public Port { class StunPort : public UDPPort { public: - static StunPort* Create(rtc::Thread* thread, - rtc::PacketSocketFactory* factory, - rtc::Network* network, - uint16_t min_port, - uint16_t max_port, - const std::string& username, - const std::string& password, - const ServerAddresses& servers, - const std::string& origin, - absl::optional stun_keepalive_interval); + static std::unique_ptr Create( + rtc::Thread* thread, + rtc::PacketSocketFactory* factory, + rtc::Network* network, + uint16_t min_port, + uint16_t max_port, + const std::string& username, + const std::string& password, + const ServerAddresses& servers, + const std::string& origin, + absl::optional stun_keepalive_interval); void PrepareAddress() override; diff --git a/p2p/base/stunport_unittest.cc b/p2p/base/stunport_unittest.cc index 29754e861c..8cf10d571a 100644 --- a/p2p/base/stunport_unittest.cc +++ b/p2p/base/stunport_unittest.cc @@ -75,10 +75,10 @@ class StunPortTestBase : public testing::Test, public sigslot::has_slots<> { } void CreateStunPort(const ServerAddresses& stun_servers) { - stun_port_.reset(cricket::StunPort::Create( + stun_port_ = cricket::StunPort::Create( rtc::Thread::Current(), &socket_factory_, &network_, 0, 0, rtc::CreateRandomString(16), rtc::CreateRandomString(22), stun_servers, - std::string(), absl::nullopt)); + std::string(), absl::nullopt); 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. @@ -100,10 +100,10 @@ class StunPortTestBase : public testing::Test, public sigslot::has_slots<> { } ASSERT_TRUE(socket_ != NULL); socket_->SignalReadPacket.connect(this, &StunPortTestBase::OnReadPacket); - stun_port_.reset(cricket::UDPPort::Create( + stun_port_ = cricket::UDPPort::Create( rtc::Thread::Current(), &socket_factory_, &network_, socket_.get(), rtc::CreateRandomString(16), rtc::CreateRandomString(22), std::string(), - false, absl::nullopt)); + false, absl::nullopt); ASSERT_TRUE(stun_port_ != NULL); ServerAddresses stun_servers; stun_servers.insert(server_addr); diff --git a/p2p/base/tcpport.h b/p2p/base/tcpport.h index fa5c958c21..6a71d083ab 100644 --- a/p2p/base/tcpport.h +++ b/p2p/base/tcpport.h @@ -30,16 +30,18 @@ class TCPConnection; // call this TCPPort::OnReadPacket (3 arg) to dispatch to a connection. class TCPPort : public Port { public: - static TCPPort* Create(rtc::Thread* thread, - rtc::PacketSocketFactory* factory, - rtc::Network* network, - uint16_t min_port, - uint16_t max_port, - const std::string& username, - const std::string& password, - bool allow_listen) { - return new TCPPort(thread, factory, network, min_port, max_port, username, - password, allow_listen); + static std::unique_ptr Create(rtc::Thread* thread, + rtc::PacketSocketFactory* factory, + rtc::Network* network, + uint16_t min_port, + uint16_t max_port, + const std::string& username, + const std::string& password, + bool allow_listen) { + // Using `new` to access a non-public constructor. + return absl::WrapUnique(new TCPPort(thread, factory, network, min_port, + max_port, username, password, + allow_listen)); } ~TCPPort() override; diff --git a/p2p/base/turnport.h b/p2p/base/turnport.h index 11b58d4f6c..fe5929866e 100644 --- a/p2p/base/turnport.h +++ b/p2p/base/turnport.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -50,6 +51,8 @@ class TurnPort : public Port { // packets. }; // Create a TURN port using the shared UDP socket, |socket|. + // TODO(steveanton): Change to unique_ptr once downstream clients have + // converted. static TurnPort* Create(rtc::Thread* thread, rtc::PacketSocketFactory* factory, rtc::Network* network, @@ -61,13 +64,33 @@ class TurnPort : public Port { int server_priority, const std::string& origin, webrtc::TurnCustomizer* customizer) { - return new TurnPort(thread, factory, network, socket, username, password, + return CreateUnique(thread, factory, network, socket, username, password, server_address, credentials, server_priority, origin, - customizer); + customizer) + .release(); + } + static std::unique_ptr CreateUnique( + rtc::Thread* thread, + rtc::PacketSocketFactory* factory, + rtc::Network* network, + rtc::AsyncPacketSocket* socket, + const std::string& username, // ice username. + const std::string& password, // ice password. + const ProtocolAddress& server_address, + const RelayCredentials& credentials, + int server_priority, + const std::string& origin, + webrtc::TurnCustomizer* customizer) { + // Using `new` to access a non-public constructor. + return absl::WrapUnique(new TurnPort( + thread, factory, network, socket, username, password, server_address, + credentials, server_priority, origin, customizer)); } // Create a TURN port that will use a new socket, bound to |network| and // using a port in the range between |min_port| and |max_port|. + // TODO(steveanton): Change to unique_ptr once downstream clients have + // converted. static TurnPort* Create( rtc::Thread* thread, rtc::PacketSocketFactory* factory, @@ -84,10 +107,35 @@ class TurnPort : public Port { const std::vector& tls_elliptic_curves, webrtc::TurnCustomizer* customizer, rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr) { - return new TurnPort(thread, factory, network, min_port, max_port, username, + // Using `new` to access a non-public constructor. + return CreateUnique(thread, factory, network, min_port, max_port, username, password, server_address, credentials, server_priority, origin, tls_alpn_protocols, tls_elliptic_curves, - customizer, tls_cert_verifier); + customizer, tls_cert_verifier) + .release(); + } + static std::unique_ptr CreateUnique( + rtc::Thread* thread, + rtc::PacketSocketFactory* factory, + rtc::Network* network, + uint16_t min_port, + uint16_t max_port, + const std::string& username, // ice username. + const std::string& password, // ice password. + const ProtocolAddress& server_address, + const RelayCredentials& credentials, + int server_priority, + const std::string& origin, + const std::vector& tls_alpn_protocols, + const std::vector& tls_elliptic_curves, + webrtc::TurnCustomizer* customizer, + rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr) { + // Using `new` to access a non-public constructor. + return absl::WrapUnique( + new TurnPort(thread, factory, network, min_port, max_port, username, + password, server_address, credentials, server_priority, + origin, tls_alpn_protocols, tls_elliptic_curves, + customizer, tls_cert_verifier)); } ~TurnPort() override; diff --git a/p2p/base/turnport_unittest.cc b/p2p/base/turnport_unittest.cc index bac35e891c..27657a382e 100644 --- a/p2p/base/turnport_unittest.cc +++ b/p2p/base/turnport_unittest.cc @@ -270,10 +270,9 @@ class TurnPortTest : public testing::Test, const ProtocolAddress& server_address, const std::string& origin) { RelayCredentials credentials(username, password); - turn_port_.reset(TurnPort::Create( + turn_port_ = TurnPort::CreateUnique( &main_, &socket_factory_, network, 0, 0, kIceUfrag1, kIcePwd1, - server_address, credentials, 0, origin, std::vector(), - std::vector(), turn_customizer_.get())); + server_address, credentials, 0, origin, {}, {}, turn_customizer_.get()); // This TURN port will be the controlling. turn_port_->SetIceRole(ICEROLE_CONTROLLING); ConnectSignals(); @@ -301,10 +300,10 @@ class TurnPortTest : public testing::Test, } RelayCredentials credentials(username, password); - turn_port_.reset(TurnPort::Create(&main_, &socket_factory_, - MakeNetwork(kLocalAddr1), socket_.get(), - kIceUfrag1, kIcePwd1, server_address, - credentials, 0, std::string(), nullptr)); + turn_port_ = TurnPort::CreateUnique(&main_, &socket_factory_, + MakeNetwork(kLocalAddr1), socket_.get(), + kIceUfrag1, kIcePwd1, server_address, + credentials, 0, std::string(), nullptr); // This TURN port will be the controlling. turn_port_->SetIceRole(ICEROLE_CONTROLLING); ConnectSignals(); @@ -329,9 +328,9 @@ class TurnPortTest : public testing::Test, void CreateUdpPort() { CreateUdpPort(kLocalAddr2); } void CreateUdpPort(const SocketAddress& address) { - udp_port_.reset(UDPPort::Create( - &main_, &socket_factory_, MakeNetwork(address), 0, 0, kIceUfrag2, - kIcePwd2, std::string(), false, absl::nullopt)); + udp_port_ = UDPPort::Create(&main_, &socket_factory_, MakeNetwork(address), + 0, 0, kIceUfrag2, kIcePwd2, std::string(), + false, absl::nullopt); // UDP port will be controlled. udp_port_->SetIceRole(ICEROLE_CONTROLLED); udp_port_->SignalPortComplete.connect(this, diff --git a/p2p/client/basicportallocator.cc b/p2p/client/basicportallocator.cc index c1b1de58dd..a83df39a74 100644 --- a/p2p/client/basicportallocator.cc +++ b/p2p/client/basicportallocator.cc @@ -1362,7 +1362,7 @@ void AllocationSequence::CreateUDPPorts() { // TODO(mallinath) - Remove UDPPort creating socket after shared socket // is enabled completely. - UDPPort* port = NULL; + std::unique_ptr port; bool emit_local_candidate_for_anyaddress = !IsFlagSet(PORTALLOCATOR_DISABLE_DEFAULT_LOCAL_CANDIDATE); if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET) && udp_socket_) { @@ -1384,7 +1384,7 @@ void AllocationSequence::CreateUDPPorts() { // If shared socket is enabled, STUN candidate will be allocated by the // UDPPort. if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET)) { - udp_port_ = port; + udp_port_ = port.get(); port->SignalDestroyed.connect(this, &AllocationSequence::OnPortDestroyed); // If STUN is not disabled, setting stun server address to port. @@ -1398,7 +1398,7 @@ void AllocationSequence::CreateUDPPorts() { } } - session_->AddAllocatedPort(port, this, true); + session_->AddAllocatedPort(port.release(), this, true); } } @@ -1408,13 +1408,13 @@ void AllocationSequence::CreateTCPPorts() { return; } - Port* port = TCPPort::Create( + std::unique_ptr port = TCPPort::Create( session_->network_thread(), session_->socket_factory(), network_, session_->allocator()->min_port(), session_->allocator()->max_port(), session_->username(), session_->password(), session_->allocator()->allow_tcp_listen()); if (port) { - session_->AddAllocatedPort(port, this, true); + session_->AddAllocatedPort(port.release(), this, true); // Since TCPPort is not created using shared socket, |port| will not be // added to the dequeue. } @@ -1436,14 +1436,14 @@ void AllocationSequence::CreateStunPorts() { return; } - StunPort* port = StunPort::Create( + std::unique_ptr port = StunPort::Create( session_->network_thread(), session_->socket_factory(), network_, session_->allocator()->min_port(), session_->allocator()->max_port(), session_->username(), session_->password(), config_->StunServers(), session_->allocator()->origin(), session_->allocator()->stun_candidate_keepalive_interval()); if (port) { - session_->AddAllocatedPort(port, this, true); + session_->AddAllocatedPort(port.release(), this, true); // Since StunPort is not created using shared socket, |port| will not be // added to the dequeue. } @@ -1479,11 +1479,12 @@ void AllocationSequence::CreateRelayPorts() { void AllocationSequence::CreateGturnPort(const RelayServerConfig& config) { // TODO(mallinath) - Rename RelayPort to GTurnPort. - RelayPort* port = RelayPort::Create( + std::unique_ptr port = RelayPort::Create( session_->network_thread(), session_->socket_factory(), network_, session_->allocator()->min_port(), session_->allocator()->max_port(), config_->username, config_->password); if (port) { + RelayPort* port_ptr = port.release(); // Since RelayPort is not created using shared socket, |port| will not be // added to the dequeue. // Note: We must add the allocated port before we add addresses because @@ -1491,17 +1492,17 @@ void AllocationSequence::CreateGturnPort(const RelayServerConfig& config) { // settings. However, we also can't prepare the address (normally // done by AddAllocatedPort) until we have these addresses. So we // wait to do that until below. - session_->AddAllocatedPort(port, this, false); + session_->AddAllocatedPort(port_ptr, this, false); // Add the addresses of this protocol. PortList::const_iterator relay_port; for (relay_port = config.ports.begin(); relay_port != config.ports.end(); ++relay_port) { - port->AddServerAddress(*relay_port); - port->AddExternalAddress(*relay_port); + port_ptr->AddServerAddress(*relay_port); + port_ptr->AddExternalAddress(*relay_port); } // Start fetching an address for this port. - port->PrepareAddress(); + port_ptr->PrepareAddress(); } } diff --git a/p2p/client/turnportfactory.cc b/p2p/client/turnportfactory.cc index 6404134c4a..c0c172076b 100644 --- a/p2p/client/turnportfactory.cc +++ b/p2p/client/turnportfactory.cc @@ -11,6 +11,7 @@ #include "p2p/client/turnportfactory.h" #include +#include #include "p2p/base/turnport.h" @@ -21,26 +22,26 @@ TurnPortFactory::~TurnPortFactory() {} std::unique_ptr TurnPortFactory::Create( const CreateRelayPortArgs& args, rtc::AsyncPacketSocket* udp_socket) { - TurnPort* port = TurnPort::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.origin, args.turn_customizer); port->SetTlsCertPolicy(args.config->tls_cert_policy); - return std::unique_ptr(port); + return std::move(port); } std::unique_ptr TurnPortFactory::Create(const CreateRelayPortArgs& args, int min_port, int max_port) { - TurnPort* port = TurnPort::Create( + auto port = TurnPort::CreateUnique( args.network_thread, args.socket_factory, args.network, min_port, max_port, args.username, args.password, *args.server_address, args.config->credentials, args.config->priority, args.origin, args.config->tls_alpn_protocols, args.config->tls_elliptic_curves, args.turn_customizer, args.config->tls_cert_verifier); port->SetTlsCertPolicy(args.config->tls_cert_policy); - return std::unique_ptr(port); + return std::move(port); } } // namespace cricket