diff --git a/talk/app/webrtc/portallocatorfactory.cc b/talk/app/webrtc/portallocatorfactory.cc index c907e9c61..decd33c7b 100644 --- a/talk/app/webrtc/portallocatorfactory.cc +++ b/talk/app/webrtc/portallocatorfactory.cc @@ -55,15 +55,19 @@ PortAllocatorFactory::~PortAllocatorFactory() {} cricket::PortAllocator* PortAllocatorFactory::CreatePortAllocator( const std::vector& stun, const std::vector& turn) { - cricket::ServerAddresses stun_hosts; + std::vector stun_hosts; typedef std::vector::const_iterator StunIt; for (StunIt stun_it = stun.begin(); stun_it != stun.end(); ++stun_it) { - stun_hosts.insert(stun_it->server); + stun_hosts.push_back(stun_it->server); } + talk_base::SocketAddress stun_addr; + if (!stun_hosts.empty()) { + stun_addr = stun_hosts.front(); + } scoped_ptr allocator( new cricket::BasicPortAllocator( - network_manager_.get(), socket_factory_.get(), stun_hosts)); + network_manager_.get(), socket_factory_.get(), stun_addr)); for (size_t i = 0; i < turn.size(); ++i) { cricket::RelayCredentials credentials(turn[i].username, turn[i].password); diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index c86da8e39..956f8a66f 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -306,16 +306,12 @@ class WebRtcSessionTest : public testing::Test { cricket::STUN_SERVER_PORT)), stun_server_(Thread::Current(), stun_socket_addr_), turn_server_(Thread::Current(), kTurnUdpIntAddr, kTurnUdpExtAddr), + allocator_(new cricket::BasicPortAllocator( + &network_manager_, stun_socket_addr_, + SocketAddress(), SocketAddress(), SocketAddress())), mediastream_signaling_(channel_manager_.get()), ice_type_(PeerConnectionInterface::kAll) { tdesc_factory_->set_protocol(cricket::ICEPROTO_HYBRID); - - cricket::ServerAddresses stun_servers; - stun_servers.insert(stun_socket_addr_); - allocator_.reset(new cricket::BasicPortAllocator( - &network_manager_, - stun_servers, - SocketAddress(), SocketAddress(), SocketAddress())); allocator_->set_flags(cricket::PORTALLOCATOR_DISABLE_TCP | cricket::PORTALLOCATOR_DISABLE_RELAY | cricket::PORTALLOCATOR_ENABLE_BUNDLE); diff --git a/talk/p2p/base/p2ptransportchannel_unittest.cc b/talk/p2p/base/p2ptransportchannel_unittest.cc index f65f502dc..498fde9dc 100644 --- a/talk/p2p/base/p2ptransportchannel_unittest.cc +++ b/talk/p2p/base/p2ptransportchannel_unittest.cc @@ -50,7 +50,6 @@ using cricket::kMinimumStepDelay; using cricket::kDefaultStepDelay; using cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG; using cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET; -using cricket::ServerAddresses; using talk_base::SocketAddress; static const int kDefaultTimeout = 1000; @@ -153,14 +152,12 @@ class P2PTransportChannelTestBase : public testing::Test, ep1_.role_ = cricket::ICEROLE_CONTROLLING; ep2_.role_ = cricket::ICEROLE_CONTROLLED; - ServerAddresses stun_servers; - stun_servers.insert(kStunAddr); ep1_.allocator_.reset(new cricket::BasicPortAllocator( - &ep1_.network_manager_, - stun_servers, kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)); + &ep1_.network_manager_, kStunAddr, kRelayUdpIntAddr, + kRelayTcpIntAddr, kRelaySslTcpIntAddr)); ep2_.allocator_.reset(new cricket::BasicPortAllocator( - &ep2_.network_manager_, - stun_servers, kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)); + &ep2_.network_manager_, kStunAddr, kRelayUdpIntAddr, + kRelayTcpIntAddr, kRelaySslTcpIntAddr)); } protected: @@ -809,17 +806,13 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase { // Ideally we want to use TURN server for both GICE and ICE, but in case // of GICE, TURN server usage is not producing results reliabally. // TODO(mallinath): Remove Relay and use TURN server for all tests. - ServerAddresses stun_servers; - stun_servers.insert(kStunAddr); GetEndpoint(0)->allocator_.reset( new cricket::BasicPortAllocator(&(GetEndpoint(0)->network_manager_), - stun_servers, - talk_base::SocketAddress(), talk_base::SocketAddress(), + kStunAddr, talk_base::SocketAddress(), talk_base::SocketAddress(), talk_base::SocketAddress())); GetEndpoint(1)->allocator_.reset( new cricket::BasicPortAllocator(&(GetEndpoint(1)->network_manager_), - stun_servers, - talk_base::SocketAddress(), talk_base::SocketAddress(), + kStunAddr, talk_base::SocketAddress(), talk_base::SocketAddress(), talk_base::SocketAddress())); cricket::RelayServerConfig relay_server(cricket::RELAY_GTURN); diff --git a/talk/p2p/base/port.h b/talk/p2p/base/port.h index 62ec0b796..6e5c383b7 100644 --- a/talk/p2p/base/port.h +++ b/talk/p2p/base/port.h @@ -31,7 +31,6 @@ #include #include #include -#include #include "talk/base/asyncpacketsocket.h" #include "talk/base/network.h" @@ -110,8 +109,6 @@ struct ProtocolAddress { : address(a), proto(p), secure(sec) { } }; -typedef std::set ServerAddresses; - // Represents a local communication mechanism that can be used to create // connections to similar mechanisms of the other client. Subclasses of this // one add support for specific mechanisms like local UDP ports. diff --git a/talk/p2p/base/port_unittest.cc b/talk/p2p/base/port_unittest.cc index 70c8ccc58..fc6d48c9a 100644 --- a/talk/p2p/base/port_unittest.cc +++ b/talk/p2p/base/port_unittest.cc @@ -453,11 +453,9 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { } StunPort* CreateStunPort(const SocketAddress& addr, talk_base::PacketSocketFactory* factory) { - ServerAddresses stun_servers; - stun_servers.insert(kStunAddr); StunPort* port = StunPort::Create(main_, factory, &network_, addr.ipaddr(), 0, 0, - username_, password_, stun_servers); + username_, password_, kStunAddr); port->SetIceProtocolType(ice_protocol_); return port; } diff --git a/talk/p2p/base/stunport.cc b/talk/p2p/base/stunport.cc index 0a051cbba..6e18fc505 100644 --- a/talk/p2p/base/stunport.cc +++ b/talk/p2p/base/stunport.cc @@ -69,10 +69,10 @@ class StunBindingRequest : public StunRequest { LOG(LS_ERROR) << "Binding address has bad family"; } else { talk_base::SocketAddress addr(addr_attr->ipaddr(), addr_attr->port()); - port_->OnStunBindingRequestSucceeded(server_addr_, addr); + port_->OnStunBindingRequestSucceeded(addr); } - // We will do a keep-alive regardless of whether this request succeeds. + // We will do a keep-alive regardless of whether this request suceeds. // This should have almost no impact on network usage. if (keep_alive_) { port_->requests_.SendDelayed( @@ -92,7 +92,7 @@ class StunBindingRequest : public StunRequest { << " reason='" << attr->reason() << "'"; } - port_->OnStunBindingOrResolveRequestFailed(server_addr_); + port_->OnStunBindingOrResolveRequestFailed(); if (keep_alive_ && (talk_base::TimeSince(start_time_) <= RETRY_TIMEOUT)) { @@ -107,7 +107,7 @@ class StunBindingRequest : public StunRequest { << port_->GetLocalAddress().ToSensitiveString() << " (" << port_->Network()->name() << ")"; - port_->OnStunBindingOrResolveRequestFailed(server_addr_); + port_->OnStunBindingOrResolveRequestFailed(); if (keep_alive_ && (talk_base::TimeSince(start_time_) <= RETRY_TIMEOUT)) { @@ -120,60 +120,10 @@ class StunBindingRequest : public StunRequest { private: UDPPort* port_; bool keep_alive_; - const talk_base::SocketAddress server_addr_; + talk_base::SocketAddress server_addr_; uint32 start_time_; }; -UDPPort::AddressResolver::AddressResolver( - talk_base::PacketSocketFactory* factory) - : socket_factory_(factory) {} - -UDPPort::AddressResolver::~AddressResolver() { - for (ResolverMap::iterator it = resolvers_.begin(); - it != resolvers_.end(); ++it) { - it->second->Destroy(true); - } -} - -void UDPPort::AddressResolver::Resolve( - const talk_base::SocketAddress& address) { - if (resolvers_.find(address) != resolvers_.end()) - return; - - talk_base::AsyncResolverInterface* resolver = - socket_factory_->CreateAsyncResolver(); - resolvers_.insert( - std::pair( - address, resolver)); - - resolver->SignalDone.connect(this, - &UDPPort::AddressResolver::OnResolveResult); - - resolver->Start(address); -} - -bool UDPPort::AddressResolver::GetResolvedAddress( - const talk_base::SocketAddress& input, - int family, - talk_base::SocketAddress* output) const { - ResolverMap::const_iterator it = resolvers_.find(input); - if (it == resolvers_.end()) - return false; - - return it->second->GetResolvedAddress(family, output); -} - -void UDPPort::AddressResolver::OnResolveResult( - talk_base::AsyncResolverInterface* resolver) { - for (ResolverMap::iterator it = resolvers_.begin(); - it != resolvers_.end(); ++it) { - if (it->second == resolver) { - SignalDone(it->first, resolver->GetError()); - return; - } - } -} - UDPPort::UDPPort(talk_base::Thread* thread, talk_base::PacketSocketFactory* factory, talk_base::Network* network, @@ -184,6 +134,7 @@ UDPPort::UDPPort(talk_base::Thread* thread, requests_(thread), socket_(socket), error_(0), + resolver_(NULL), ready_(false), stun_keepalive_delay_(KEEPALIVE_DELAY) { } @@ -198,6 +149,7 @@ UDPPort::UDPPort(talk_base::Thread* thread, requests_(thread), socket_(NULL), error_(0), + resolver_(NULL), ready_(false), stun_keepalive_delay_(KEEPALIVE_DELAY) { } @@ -220,6 +172,9 @@ bool UDPPort::Init() { } UDPPort::~UDPPort() { + if (resolver_) { + resolver_->Destroy(true); + } if (!SharedSocket()) delete socket_; } @@ -234,11 +189,11 @@ void UDPPort::PrepareAddress() { void UDPPort::MaybePrepareStunCandidate() { // Sending binding request to the STUN server if address is available to // prepare STUN candidate. - if (!server_addresses_.empty()) { - SendStunBindingRequests(); + if (!server_addr_.IsNil()) { + SendStunBindingRequest(); } else { // Port is done allocating candidates. - MaybeSetPortCompleteOrError(); + SetResult(true); } } @@ -299,13 +254,12 @@ void UDPPort::OnReadPacket( const talk_base::SocketAddress& remote_addr, const talk_base::PacketTime& packet_time) { ASSERT(socket == socket_); - ASSERT(!remote_addr.IsUnresolved()); // Look for a response from the STUN server. // Even if the response doesn't match one of our outstanding requests, we // will eat it because it might be a response to a retransmitted packet, and // we already cleared the request when we got the first response. - if (server_addresses_.find(remote_addr) != server_addresses_.end()) { + if (!server_addr_.IsUnresolved() && remote_addr == server_addr_) { requests_.CheckResponse(data, size); return; } @@ -321,114 +275,76 @@ void UDPPort::OnReadyToSend(talk_base::AsyncPacketSocket* socket) { Port::OnReadyToSend(); } -void UDPPort::SendStunBindingRequests() { +void UDPPort::SendStunBindingRequest() { // We will keep pinging the stun server to make sure our NAT pin-hole stays // open during the call. + // TODO: Support multiple stun servers, or make ResolveStunAddress find a + // server with the correct family, or something similar. ASSERT(requests_.empty()); - - for (ServerAddresses::const_iterator it = server_addresses_.begin(); - it != server_addresses_.end(); ++it) { - SendStunBindingRequest(*it); - } -} - -void UDPPort::ResolveStunAddress(const talk_base::SocketAddress& stun_addr) { - if (!resolver_) { - resolver_.reset(new AddressResolver(socket_factory())); - resolver_->SignalDone.connect(this, &UDPPort::OnResolveResult); - } - - resolver_->Resolve(stun_addr); -} - -void UDPPort::OnResolveResult(const talk_base::SocketAddress& input, - int error) { - ASSERT(resolver_.get()); - - talk_base::SocketAddress resolved; - if (error != 0 || - !resolver_->GetResolvedAddress(input, ip().family(), &resolved)) { - LOG_J(LS_WARNING, this) << "StunPort: stun host lookup received error " - << error; - OnStunBindingOrResolveRequestFailed(input); - return; - } - - server_addresses_.erase(input); - - if (server_addresses_.find(resolved) == server_addresses_.end()) { - server_addresses_.insert(resolved); - SendStunBindingRequest(resolved); - } -} - -void UDPPort::SendStunBindingRequest( - const talk_base::SocketAddress& stun_addr) { - if (stun_addr.IsUnresolved()) { - ResolveStunAddress(stun_addr); - + if (server_addr_.IsUnresolved()) { + ResolveStunAddress(); } else if (socket_->GetState() == talk_base::AsyncPacketSocket::STATE_BOUND) { // Check if |server_addr_| is compatible with the port's ip. - if (IsCompatibleAddress(stun_addr)) { - requests_.Send(new StunBindingRequest(this, true, stun_addr)); + if (IsCompatibleAddress(server_addr_)) { + requests_.Send(new StunBindingRequest(this, true, server_addr_)); } else { // Since we can't send stun messages to the server, we should mark this // port ready. - LOG(LS_WARNING) << "STUN server address is incompatible."; - OnStunBindingOrResolveRequestFailed(stun_addr); + OnStunBindingOrResolveRequestFailed(); } } } -void UDPPort::OnStunBindingRequestSucceeded( - const talk_base::SocketAddress& stun_server_addr, - const talk_base::SocketAddress& stun_reflected_addr) { - if (bind_request_succeeded_servers_.find(stun_server_addr) != - bind_request_succeeded_servers_.end()) { +void UDPPort::ResolveStunAddress() { + if (resolver_) + return; + + resolver_ = socket_factory()->CreateAsyncResolver(); + resolver_->SignalDone.connect(this, &UDPPort::OnResolveResult); + resolver_->Start(server_addr_); +} + +void UDPPort::OnResolveResult(talk_base::AsyncResolverInterface* resolver) { + ASSERT(resolver == resolver_); + if (resolver_->GetError() != 0 || + !resolver_->GetResolvedAddress(ip().family(), &server_addr_)) { + LOG_J(LS_WARNING, this) << "StunPort: stun host lookup received error " + << resolver_->GetError(); + OnStunBindingOrResolveRequestFailed(); return; } - bind_request_succeeded_servers_.insert(stun_server_addr); - if (!SharedSocket() || stun_reflected_addr != socket_->GetLocalAddress()) { - // If socket is shared and |stun_reflected_addr| is equal to local socket + SendStunBindingRequest(); +} + +void UDPPort::OnStunBindingRequestSucceeded( + const talk_base::SocketAddress& stun_addr) { + if (ready_) // Discarding the binding response if port is already enabled. + return; + + if (!SharedSocket() || stun_addr != socket_->GetLocalAddress()) { + // If socket is shared and |stun_addr| is equal to local socket // address then discarding the stun address. // For STUN related address is local socket address. - AddAddress(stun_reflected_addr, socket_->GetLocalAddress(), + AddAddress(stun_addr, socket_->GetLocalAddress(), socket_->GetLocalAddress(), UDP_PROTOCOL_NAME, STUN_PORT_TYPE, ICE_TYPE_PREFERENCE_SRFLX, false); } - MaybeSetPortCompleteOrError(); + SetResult(true); } -void UDPPort::OnStunBindingOrResolveRequestFailed( - const talk_base::SocketAddress& stun_server_addr) { - if (bind_request_failed_servers_.find(stun_server_addr) != - bind_request_failed_servers_.end()) { +void UDPPort::OnStunBindingOrResolveRequestFailed() { + if (ready_) // Discarding failure response if port is already enabled. return; - } - bind_request_failed_servers_.insert(stun_server_addr); - MaybeSetPortCompleteOrError(); + + // If socket is shared, we should process local udp candidate. + SetResult(SharedSocket()); } -void UDPPort::MaybeSetPortCompleteOrError() { - if (ready_) - return; - - // Do not set port ready if we are still waiting for bind responses. - const size_t servers_done_bind_request = bind_request_failed_servers_.size() + - bind_request_succeeded_servers_.size(); - if (server_addresses_.size() != servers_done_bind_request) { - return; - } - +void UDPPort::SetResult(bool success) { // Setting ready status. ready_ = true; - - // The port is "completed" if there is no stun server provided, or the bind - // request succeeded for any stun server, or the socket is shared. - if (server_addresses_.empty() || - bind_request_succeeded_servers_.size() > 0 || - SharedSocket()) { + if (success) { SignalPortComplete(this); } else { SignalPortError(this); diff --git a/talk/p2p/base/stunport.h b/talk/p2p/base/stunport.h index 367db22cf..c45d6af33 100644 --- a/talk/p2p/base/stunport.h +++ b/talk/p2p/base/stunport.h @@ -82,12 +82,9 @@ class UDPPort : public Port { return socket_->GetLocalAddress(); } - const ServerAddresses server_addresses() const { - return server_addresses_; - } - void - set_server_addresses(const ServerAddresses& addresses) { - server_addresses_ = addresses; + const talk_base::SocketAddress& server_addr() const { return server_addr_; } + void set_server_addr(const talk_base::SocketAddress& addr) { + server_addr_ = addr; } virtual void PrepareAddress(); @@ -143,64 +140,29 @@ class UDPPort : public Port { // This method will send STUN binding request if STUN server address is set. void MaybePrepareStunCandidate(); - void SendStunBindingRequests(); + void SendStunBindingRequest(); private: - // A helper class which can be called repeatedly to resolve multiple - // addresses, as opposed to talk_base::AsyncResolverInterface, which can only - // resolve one address per instance. - class AddressResolver : public sigslot::has_slots<> { - public: - explicit AddressResolver(talk_base::PacketSocketFactory* factory); - ~AddressResolver(); - - void Resolve(const talk_base::SocketAddress& address); - bool GetResolvedAddress(const talk_base::SocketAddress& input, - int family, - talk_base::SocketAddress* output) const; - - // The signal is sent when resolving the specified address is finished. The - // first argument is the input address, the second argument is the error - // or 0 if it succeeded. - sigslot::signal2 SignalDone; - - private: - typedef std::map ResolverMap; - - void OnResolveResult(talk_base::AsyncResolverInterface* resolver); - - talk_base::PacketSocketFactory* socket_factory_; - ResolverMap resolvers_; - }; - // DNS resolution of the STUN server. - void ResolveStunAddress(const talk_base::SocketAddress& stun_addr); - void OnResolveResult(const talk_base::SocketAddress& input, int error); - - void SendStunBindingRequest(const talk_base::SocketAddress& stun_addr); + void ResolveStunAddress(); + void OnResolveResult(talk_base::AsyncResolverInterface* resolver); // Below methods handles binding request responses. - void OnStunBindingRequestSucceeded( - const talk_base::SocketAddress& stun_server_addr, - const talk_base::SocketAddress& stun_reflected_addr); - void OnStunBindingOrResolveRequestFailed( - const talk_base::SocketAddress& stun_server_addr); + void OnStunBindingRequestSucceeded(const talk_base::SocketAddress& stun_addr); + void OnStunBindingOrResolveRequestFailed(); // Sends STUN requests to the server. void OnSendPacket(const void* data, size_t size, StunRequest* req); // TODO(mallinaht) - Move this up to cricket::Port when SignalAddressReady is // changed to SignalPortReady. - void MaybeSetPortCompleteOrError(); + void SetResult(bool success); - ServerAddresses server_addresses_; - ServerAddresses bind_request_succeeded_servers_; - ServerAddresses bind_request_failed_servers_; + talk_base::SocketAddress server_addr_; StunRequestManager requests_; talk_base::AsyncPacketSocket* socket_; int error_; - talk_base::scoped_ptr resolver_; + talk_base::AsyncResolverInterface* resolver_; bool ready_; int stun_keepalive_delay_; @@ -209,18 +171,17 @@ class UDPPort : public Port { class StunPort : public UDPPort { public: - static StunPort* Create( - talk_base::Thread* thread, - talk_base::PacketSocketFactory* factory, - talk_base::Network* network, - const talk_base::IPAddress& ip, - int min_port, int max_port, - const std::string& username, - const std::string& password, - const ServerAddresses& servers) { + static StunPort* Create(talk_base::Thread* thread, + talk_base::PacketSocketFactory* factory, + talk_base::Network* network, + const talk_base::IPAddress& ip, + int min_port, int max_port, + const std::string& username, + const std::string& password, + const talk_base::SocketAddress& server_addr) { StunPort* port = new StunPort(thread, factory, network, ip, min_port, max_port, - username, password, servers); + username, password, server_addr); if (!port->Init()) { delete port; port = NULL; @@ -231,7 +192,7 @@ class StunPort : public UDPPort { virtual ~StunPort() {} virtual void PrepareAddress() { - SendStunBindingRequests(); + SendStunBindingRequest(); } protected: @@ -239,12 +200,12 @@ class StunPort : public UDPPort { talk_base::Network* network, const talk_base::IPAddress& ip, int min_port, int max_port, const std::string& username, const std::string& password, - const ServerAddresses& servers) + const talk_base::SocketAddress& server_address) : UDPPort(thread, factory, network, ip, min_port, max_port, username, password) { // UDPPort will set these to local udp, updating these to STUN. set_type(STUN_PORT_TYPE); - set_server_addresses(servers); + set_server_addr(server_address); } }; diff --git a/talk/p2p/base/stunport_unittest.cc b/talk/p2p/base/stunport_unittest.cc index 0965712f0..5850027ec 100644 --- a/talk/p2p/base/stunport_unittest.cc +++ b/talk/p2p/base/stunport_unittest.cc @@ -30,18 +30,15 @@ #include "talk/base/physicalsocketserver.h" #include "talk/base/scoped_ptr.h" #include "talk/base/socketaddress.h" -#include "talk/base/ssladapter.h" #include "talk/base/virtualsocketserver.h" #include "talk/p2p/base/basicpacketsocketfactory.h" #include "talk/p2p/base/stunport.h" #include "talk/p2p/base/teststunserver.h" -using cricket::ServerAddresses; using talk_base::SocketAddress; static const SocketAddress kLocalAddr("127.0.0.1", 0); -static const SocketAddress kStunAddr1("127.0.0.1", 5000); -static const SocketAddress kStunAddr2("127.0.0.1", 4000); +static const SocketAddress kStunAddr("127.0.0.1", 5000); static const SocketAddress kBadAddr("0.0.0.1", 5000); static const SocketAddress kStunHostnameAddr("localhost", 5000); static const SocketAddress kBadHostnameAddr("not-a-real-hostname", 5000); @@ -61,10 +58,8 @@ class StunPortTest : public testing::Test, ss_scope_(ss_.get()), network_("unittest", "unittest", talk_base::IPAddress(INADDR_ANY), 32), socket_factory_(talk_base::Thread::Current()), - stun_server_1_(new cricket::TestStunServer( - talk_base::Thread::Current(), kStunAddr1)), - stun_server_2_(new cricket::TestStunServer( - talk_base::Thread::Current(), kStunAddr2)), + stun_server_(new cricket::TestStunServer( + talk_base::Thread::Current(), kStunAddr)), done_(false), error_(false), stun_keepalive_delay_(0) { } @@ -73,16 +68,10 @@ class StunPortTest : public testing::Test, bool error() const { return error_; } void CreateStunPort(const talk_base::SocketAddress& server_addr) { - ServerAddresses stun_servers; - stun_servers.insert(server_addr); - CreateStunPort(stun_servers); - } - - void CreateStunPort(const ServerAddresses& stun_servers) { stun_port_.reset(cricket::StunPort::Create( talk_base::Thread::Current(), &socket_factory_, &network_, kLocalAddr.ipaddr(), 0, 0, talk_base::CreateRandomString(16), - talk_base::CreateRandomString(22), stun_servers)); + talk_base::CreateRandomString(22), server_addr)); stun_port_->set_stun_keepalive_delay(stun_keepalive_delay_); stun_port_->SignalPortComplete.connect(this, &StunPortTest::OnPortComplete); @@ -100,9 +89,7 @@ class StunPortTest : public testing::Test, &network_, socket_.get(), talk_base::CreateRandomString(16), talk_base::CreateRandomString(22))); ASSERT_TRUE(stun_port_ != NULL); - ServerAddresses stun_servers; - stun_servers.insert(server_addr); - stun_port_->set_server_addresses(stun_servers); + stun_port_->set_server_addr(server_addr); stun_port_->SignalPortComplete.connect(this, &StunPortTest::OnPortComplete); stun_port_->SignalPortError.connect(this, @@ -128,17 +115,11 @@ class StunPortTest : public testing::Test, protected: static void SetUpTestCase() { - talk_base::InitializeSSL(); // Ensure the RNG is inited. talk_base::InitRandom(NULL, 0); - - } - static void TearDownTestCase() { - talk_base::CleanupSSL(); } void OnPortComplete(cricket::Port* port) { - ASSERT_FALSE(done_); done_ = true; error_ = false; } @@ -157,8 +138,7 @@ class StunPortTest : public testing::Test, talk_base::Network network_; talk_base::BasicPacketSocketFactory socket_factory_; talk_base::scoped_ptr stun_port_; - talk_base::scoped_ptr stun_server_1_; - talk_base::scoped_ptr stun_server_2_; + talk_base::scoped_ptr stun_server_; talk_base::scoped_ptr socket_; bool done_; bool error_; @@ -167,14 +147,14 @@ class StunPortTest : public testing::Test, // Test that we can create a STUN port TEST_F(StunPortTest, TestBasic) { - CreateStunPort(kStunAddr1); + CreateStunPort(kStunAddr); EXPECT_EQ("stun", port()->Type()); EXPECT_EQ(0U, port()->Candidates().size()); } // Test that we can get an address from a STUN server. TEST_F(StunPortTest, TestPrepareAddress) { - CreateStunPort(kStunAddr1); + CreateStunPort(kStunAddr); PrepareAddress(); EXPECT_TRUE_WAIT(done(), kTimeoutMs); ASSERT_EQ(1U, port()->Candidates().size()); @@ -229,7 +209,7 @@ TEST_F(StunPortTest, TestKeepAliveResponse) { // Test that a local candidate can be generated using a shared socket. TEST_F(StunPortTest, TestSharedSocketPrepareAddress) { - CreateSharedStunPort(kStunAddr1); + CreateSharedStunPort(kStunAddr); PrepareAddress(); EXPECT_TRUE_WAIT(done(), kTimeoutMs); ASSERT_EQ(1U, port()->Candidates().size()); @@ -252,28 +232,3 @@ TEST_F(StunPortTest, TestSharedSocketPrepareAddressInvalidHostname) { SendData(data.c_str(), data.length()); // No crash is success. } - -// Test that candidates can be allocated for multiple STUN servers. -TEST_F(StunPortTest, TestMultipleGoodStunServers) { - ServerAddresses stun_servers; - stun_servers.insert(kStunAddr1); - stun_servers.insert(kStunAddr2); - CreateStunPort(stun_servers); - EXPECT_EQ("stun", port()->Type()); - PrepareAddress(); - EXPECT_TRUE_WAIT(done(), kTimeoutMs); - EXPECT_EQ(2U, port()->Candidates().size()); -} - -// Test that candidates can be allocated for multiple STUN servers, one of which -// is not reachable. -TEST_F(StunPortTest, TestMultipleStunServersWithBadServer) { - ServerAddresses stun_servers; - stun_servers.insert(kStunAddr1); - stun_servers.insert(kBadAddr); - CreateStunPort(stun_servers); - EXPECT_EQ("stun", port()->Type()); - PrepareAddress(); - EXPECT_TRUE_WAIT(done(), kTimeoutMs); - EXPECT_EQ(1U, port()->Candidates().size()); -} diff --git a/talk/p2p/client/basicportallocator.cc b/talk/p2p/client/basicportallocator.cc index 22cd9b566..696588a51 100644 --- a/talk/p2p/client/basicportallocator.cc +++ b/talk/p2p/client/basicportallocator.cc @@ -186,23 +186,23 @@ BasicPortAllocator::BasicPortAllocator( BasicPortAllocator::BasicPortAllocator( talk_base::NetworkManager* network_manager, talk_base::PacketSocketFactory* socket_factory, - const ServerAddresses& stun_servers) + const talk_base::SocketAddress& stun_address) : network_manager_(network_manager), socket_factory_(socket_factory), - stun_servers_(stun_servers) { + stun_address_(stun_address) { ASSERT(socket_factory_ != NULL); Construct(); } BasicPortAllocator::BasicPortAllocator( talk_base::NetworkManager* network_manager, - const ServerAddresses& stun_servers, + const talk_base::SocketAddress& stun_address, const talk_base::SocketAddress& relay_address_udp, const talk_base::SocketAddress& relay_address_tcp, const talk_base::SocketAddress& relay_address_ssl) : network_manager_(network_manager), socket_factory_(NULL), - stun_servers_(stun_servers) { + stun_address_(stun_address) { RelayServerConfig config(RELAY_GTURN); if (!relay_address_udp.IsNil()) @@ -333,7 +333,7 @@ void BasicPortAllocatorSession::OnMessage(talk_base::Message *message) { } void BasicPortAllocatorSession::GetPortConfigurations() { - PortConfiguration* config = new PortConfiguration(allocator_->stun_servers(), + PortConfiguration* config = new PortConfiguration(allocator_->stun_address(), username(), password()); @@ -422,7 +422,7 @@ void BasicPortAllocatorSession::DoAllocate() { } // Disables phases that are not specified in this config. - if (!config || config->StunServers().empty()) { + if (!config || config->stun_address.IsNil()) { // No STUN ports specified in this config. sequence_flags |= PORTALLOCATOR_DISABLE_STUN; } @@ -753,8 +753,8 @@ void AllocationSequence::DisableEquivalentPhases(talk_base::Network* network, *flags |= PORTALLOCATOR_DISABLE_TCP; if (config_ && config) { - if (config_->StunServers() == config->StunServers()) { - // Already got this STUN servers covered. + if (config_->stun_address == config->stun_address) { + // Already got this STUN server covered. *flags |= PORTALLOCATOR_DISABLE_STUN; } if (!config_->relays.empty()) { @@ -878,15 +878,15 @@ void AllocationSequence::CreateUDPPorts() { // If STUN is not disabled, setting stun server address to port. if (!IsFlagSet(PORTALLOCATOR_DISABLE_STUN)) { - // If config has stun_servers, use it to get server reflexive candidate + // If config has stun_address, use it to get server reflexive candidate // otherwise use first TURN server which supports UDP. - if (config_ && !config_->StunServers().empty()) { + if (config_ && !config_->stun_address.IsNil()) { LOG(LS_INFO) << "AllocationSequence: UDPPort will be handling the " << "STUN candidate generation."; - port->set_server_addresses(config_->StunServers()); + port->set_server_addr(config_->stun_address); } else if (config_ && config_->SupportsProtocol(RELAY_TURN, PROTO_UDP)) { - port->set_server_addresses(config_->GetRelayServerAddresses( + port->set_server_addr(config_->GetFirstRelayServerAddress( RELAY_TURN, PROTO_UDP)); LOG(LS_INFO) << "AllocationSequence: TURN Server address will be " << " used for generating STUN candidate."; @@ -931,8 +931,8 @@ void AllocationSequence::CreateStunPorts() { // If BasicPortAllocatorSession::OnAllocate left STUN ports enabled then we // ought to have an address for them here. - ASSERT(config_ && !config_->StunServers().empty()); - if (!(config_ && !config_->StunServers().empty())) { + ASSERT(config_ && !config_->stun_address.IsNil()); + if (!(config_ && !config_->stun_address.IsNil())) { LOG(LS_WARNING) << "AllocationSequence: No STUN server configured, skipping."; return; @@ -944,7 +944,7 @@ void AllocationSequence::CreateStunPorts() { session_->allocator()->min_port(), session_->allocator()->max_port(), session_->username(), session_->password(), - config_->StunServers()); + config_->stun_address); if (port) { session_->AddAllocatedPort(port, this, true); // Since StunPort is not created using shared socket, |port| will not be @@ -1115,27 +1115,9 @@ PortConfiguration::PortConfiguration( const talk_base::SocketAddress& stun_address, const std::string& username, const std::string& password) - : stun_address(stun_address), username(username), password(password) { - if (!stun_address.IsNil()) - stun_servers.insert(stun_address); -} - -PortConfiguration::PortConfiguration(const ServerAddresses& stun_servers, - const std::string& username, - const std::string& password) - : stun_servers(stun_servers), + : stun_address(stun_address), username(username), password(password) { - if (!stun_servers.empty()) - stun_address = *(stun_servers.begin()); -} - -ServerAddresses PortConfiguration::StunServers() { - if (!stun_address.IsNil() && - stun_servers.find(stun_address) == stun_servers.end()) { - stun_servers.insert(stun_address); - } - return stun_servers; } void PortConfiguration::AddRelay(const RelayServerConfig& config) { @@ -1164,15 +1146,14 @@ bool PortConfiguration::SupportsProtocol(RelayType turn_type, return false; } -ServerAddresses PortConfiguration::GetRelayServerAddresses( +talk_base::SocketAddress PortConfiguration::GetFirstRelayServerAddress( RelayType turn_type, ProtocolType type) const { - ServerAddresses servers; for (size_t i = 0; i < relays.size(); ++i) { if (relays[i].type == turn_type && SupportsProtocol(relays[i], type)) { - servers.insert(relays[i].ports.front().address); + return relays[i].ports.front().address; } } - return servers; + return talk_base::SocketAddress(); } } // namespace cricket diff --git a/talk/p2p/client/basicportallocator.h b/talk/p2p/client/basicportallocator.h index 7ca22b3ae..8a60c425c 100644 --- a/talk/p2p/client/basicportallocator.h +++ b/talk/p2p/client/basicportallocator.h @@ -68,9 +68,9 @@ class BasicPortAllocator : public PortAllocator { explicit BasicPortAllocator(talk_base::NetworkManager* network_manager); BasicPortAllocator(talk_base::NetworkManager* network_manager, talk_base::PacketSocketFactory* socket_factory, - const ServerAddresses& stun_servers); + const talk_base::SocketAddress& stun_server); BasicPortAllocator(talk_base::NetworkManager* network_manager, - const ServerAddresses& stun_servers, + const talk_base::SocketAddress& stun_server, const talk_base::SocketAddress& relay_server_udp, const talk_base::SocketAddress& relay_server_tcp, const talk_base::SocketAddress& relay_server_ssl); @@ -82,8 +82,8 @@ class BasicPortAllocator : public PortAllocator { // creates its own socket factory. talk_base::PacketSocketFactory* socket_factory() { return socket_factory_; } - const ServerAddresses& stun_servers() const { - return stun_servers_; + const talk_base::SocketAddress& stun_address() const { + return stun_address_; } const std::vector& relays() const { @@ -104,7 +104,7 @@ class BasicPortAllocator : public PortAllocator { talk_base::NetworkManager* network_manager_; talk_base::PacketSocketFactory* socket_factory_; - const ServerAddresses stun_servers_; + const talk_base::SocketAddress stun_address_; std::vector relays_; bool allow_tcp_listen_; }; @@ -217,27 +217,17 @@ class BasicPortAllocatorSession : public PortAllocatorSession, // Records configuration information useful in creating ports. struct PortConfiguration : public talk_base::MessageData { - // TODO(jiayl): remove |stun_address| when Chrome is updated. talk_base::SocketAddress stun_address; - ServerAddresses stun_servers; std::string username; std::string password; typedef std::vector RelayList; RelayList relays; - // TODO(jiayl): remove this ctor when Chrome is updated. PortConfiguration(const talk_base::SocketAddress& stun_address, const std::string& username, const std::string& password); - PortConfiguration(const ServerAddresses& stun_servers, - const std::string& username, - const std::string& password); - - // TODO(jiayl): remove when |stun_address| is removed. - ServerAddresses StunServers(); - // Adds another relay server, with the given ports and modifier, to the list. void AddRelay(const RelayServerConfig& config); @@ -245,9 +235,9 @@ struct PortConfiguration : public talk_base::MessageData { bool SupportsProtocol(const RelayServerConfig& relay, ProtocolType type) const; bool SupportsProtocol(RelayType turn_type, ProtocolType type) const; - // Helper method returns the server addresses for the matching RelayType and - // Protocol type. - ServerAddresses GetRelayServerAddresses( + // Helper method returns the first server address for the matching + // RelayType and Protocol type. + talk_base::SocketAddress GetFirstRelayServerAddress( RelayType turn_type, ProtocolType type) const; }; diff --git a/talk/p2p/client/connectivitychecker.cc b/talk/p2p/client/connectivitychecker.cc index facb01e03..1b599430e 100644 --- a/talk/p2p/client/connectivitychecker.cc +++ b/talk/p2p/client/connectivitychecker.cc @@ -288,9 +288,7 @@ void ConnectivityChecker::OnStunPortComplete(Port* port) { uint32 now = talk_base::Time(); NicInfo* nic_info = &i->second; nic_info->external_address = c.address(); - - nic_info->stun_server_addresses = - static_cast(port)->server_addresses(); + nic_info->stun_server_address = static_cast(port)->server_addr(); nic_info->stun.rtt = now - nic_info->stun.start_time_ms; } else { LOG(LS_ERROR) << "Got stun address for non-existing nic"; @@ -305,9 +303,7 @@ void ConnectivityChecker::OnStunPortError(Port* port) { if (i != nics_.end()) { // We have it already, add the new information. NicInfo* nic_info = &i->second; - - nic_info->stun_server_addresses = - static_cast(port)->server_addresses(); + nic_info->stun_server_address = static_cast(port)->server_addr(); } } @@ -339,7 +335,7 @@ StunPort* ConnectivityChecker::CreateStunPort( const PortConfiguration* config, talk_base::Network* network) { return StunPort::Create(worker_, socket_factory_.get(), network, network->ip(), 0, 0, - username, password, config->stun_servers); + username, password, config->stun_address); } RelayPort* ConnectivityChecker::CreateRelayPort( @@ -411,9 +407,7 @@ void ConnectivityChecker::CreateRelayPorts( void ConnectivityChecker::AllocatePorts() { const std::string username = talk_base::CreateRandomString(ICE_UFRAG_LENGTH); const std::string password = talk_base::CreateRandomString(ICE_PWD_LENGTH); - ServerAddresses stun_servers; - stun_servers.insert(stun_address_); - PortConfiguration config(stun_servers, username, password); + PortConfiguration config(stun_address_, username, password); std::vector networks; network_manager_->GetNetworks(&networks); if (networks.empty()) { diff --git a/talk/p2p/client/connectivitychecker.h b/talk/p2p/client/connectivitychecker.h index 3f10c5762..95b736d09 100644 --- a/talk/p2p/client/connectivitychecker.h +++ b/talk/p2p/client/connectivitychecker.h @@ -96,7 +96,7 @@ struct NicInfo { talk_base::IPAddress ip; talk_base::ProxyInfo proxy_info; talk_base::SocketAddress external_address; - ServerAddresses stun_server_addresses; + talk_base::SocketAddress stun_server_address; talk_base::SocketAddress media_server_address; ConnectInfo stun; ConnectInfo http; diff --git a/talk/p2p/client/connectivitychecker_unittest.cc b/talk/p2p/client/connectivitychecker_unittest.cc index 8d6fa9df7..c62120bee 100644 --- a/talk/p2p/client/connectivitychecker_unittest.cc +++ b/talk/p2p/client/connectivitychecker_unittest.cc @@ -66,7 +66,7 @@ class FakeStunPort : public StunPort { const talk_base::IPAddress& ip, int min_port, int max_port, const std::string& username, const std::string& password, - const ServerAddresses& server_addr) + const talk_base::SocketAddress& server_addr) : StunPort(thread, factory, network, ip, min_port, max_port, username, password, server_addr) { } @@ -215,7 +215,7 @@ class ConnectivityCheckerForTest : public ConnectivityChecker { network, network->ip(), kMinPort, kMaxPort, username, password, - config->stun_servers); + config->stun_address); } virtual RelayPort* CreateRelayPort( const std::string& username, const std::string& password, @@ -254,8 +254,7 @@ class ConnectivityCheckerTest : public testing::Test { EXPECT_EQ(kExternalAddr, info.external_address); // Verify that the stun server address has been set. - EXPECT_EQ(1U, info.stun_server_addresses.size()); - EXPECT_EQ(kStunAddr, *(info.stun_server_addresses.begin())); + EXPECT_EQ(kStunAddr, info.stun_server_address); // Verify that the media server address has been set. Don't care // about port since it is different for different protocols. diff --git a/talk/p2p/client/httpportallocator.cc b/talk/p2p/client/httpportallocator.cc index 1529770c7..b881d439f 100644 --- a/talk/p2p/client/httpportallocator.cc +++ b/talk/p2p/client/httpportallocator.cc @@ -142,13 +142,7 @@ void HttpPortAllocatorSessionBase::GetPortConfigurations() { // but for now is done here and added to the initial config. Note any later // configs will have unresolved stun ips and will be discarded by the // AllocationSequence. - ServerAddresses hosts; - for (std::vector::iterator it = stun_hosts_.begin(); - it != stun_hosts_.end(); ++it) { - hosts.insert(*it); - } - - PortConfiguration* config = new PortConfiguration(hosts, + PortConfiguration* config = new PortConfiguration(stun_hosts_[0], username(), password()); ConfigReady(config); @@ -212,13 +206,7 @@ void HttpPortAllocatorSessionBase::ReceiveSessionResponse( std::string relay_tcp_port = map["relay.tcp_port"]; std::string relay_ssltcp_port = map["relay.ssltcp_port"]; - ServerAddresses hosts; - for (std::vector::iterator it = stun_hosts_.begin(); - it != stun_hosts_.end(); ++it) { - hosts.insert(*it); - } - - PortConfiguration* config = new PortConfiguration(hosts, + PortConfiguration* config = new PortConfiguration(stun_hosts_[0], map["username"], map["password"]); diff --git a/talk/p2p/client/portallocator_unittest.cc b/talk/p2p/client/portallocator_unittest.cc index 760d16820..b9def3041 100644 --- a/talk/p2p/client/portallocator_unittest.cc +++ b/talk/p2p/client/portallocator_unittest.cc @@ -48,7 +48,6 @@ #include "talk/p2p/client/basicportallocator.h" #include "talk/p2p/client/httpportallocator.h" -using cricket::ServerAddresses; using talk_base::SocketAddress; using talk_base::Thread; @@ -116,13 +115,10 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { kRelayTcpIntAddr, kRelayTcpExtAddr, kRelaySslTcpIntAddr, kRelaySslTcpExtAddr), turn_server_(Thread::Current(), kTurnUdpIntAddr, kTurnUdpExtAddr), + allocator_(new cricket::BasicPortAllocator( + &network_manager_, kStunAddr, + kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)), candidate_allocation_done_(false) { - cricket::ServerAddresses stun_servers; - stun_servers.insert(kStunAddr); - allocator_.reset(new cricket::BasicPortAllocator( - &network_manager_, - stun_servers, - kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr)); allocator_->set_step_delay(cricket::kMinimumStepDelay); } @@ -269,7 +265,7 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { // Tests that we can init the port allocator and create a session. TEST_F(PortAllocatorTest, TestBasic) { EXPECT_EQ(&network_manager_, allocator().network_manager()); - EXPECT_EQ(kStunAddr, *allocator().stun_servers().begin()); + EXPECT_EQ(kStunAddr, allocator().stun_address()); ASSERT_EQ(1u, allocator().relays().size()); EXPECT_EQ(cricket::RELAY_GTURN, allocator().relays()[0].type); // Empty relay credentials are used for GTURN. @@ -700,10 +696,8 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNat) { AddInterface(kClientAddr); talk_base::scoped_ptr nat_server( CreateNatServer(kNatAddr, talk_base::NAT_OPEN_CONE)); - ServerAddresses stun_servers; - stun_servers.insert(kStunAddr); allocator_.reset(new cricket::BasicPortAllocator( - &network_manager_, &nat_socket_factory_, stun_servers)); + &network_manager_, &nat_socket_factory_, kStunAddr)); allocator_->set_step_delay(cricket::kMinimumStepDelay); allocator_->set_flags(allocator().flags() | cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | @@ -792,10 +786,8 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurn) { AddInterface(kClientAddr); talk_base::scoped_ptr nat_server( CreateNatServer(kNatAddr, talk_base::NAT_OPEN_CONE)); - ServerAddresses stun_servers; - stun_servers.insert(kStunAddr); allocator_.reset(new cricket::BasicPortAllocator( - &network_manager_, &nat_socket_factory_, stun_servers)); + &network_manager_, &nat_socket_factory_, kStunAddr)); cricket::RelayServerConfig relay_server(cricket::RELAY_TURN); cricket::RelayCredentials credentials(kTurnUsername, kTurnPassword); relay_server.credentials = credentials; @@ -903,7 +895,6 @@ TEST(HttpPortAllocatorTest, TestHttpPortAllocatorHostLists) { talk_base::SocketAddress("1.unittest.corp.google.com", 0)); stun_servers.push_back( talk_base::SocketAddress("2.unittest.corp.google.com", 0)); - alloc.SetRelayHosts(relay_servers); alloc.SetStunHosts(stun_servers); EXPECT_EQ(2U, alloc.relay_hosts().size());