Add support of multiple STUN servers in UDPPort.

Now UDPPort signals PortComplete or PortError when the Bind requests for all STUN servers are responded or failed. If any STUN bind is successful, PortComplete is signaled; otherwise, PortError is signaled.

I discovered a bug in SocketAddress while working on this. It didn't consider two addresses unequal if they have unresolved IP and different hosts. It's fixed now.

BUG=3310
R=mallinath@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/13879004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6707 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/app/webrtc/portallocatorfactory.cc

@@ -55,19 +55,15 @@ PortAllocatorFactory::~PortAllocatorFactory() {}
 cricket::PortAllocator* PortAllocatorFactory::CreatePortAllocator(
     const std::vector<StunConfiguration>& stun,
     const std::vector<TurnConfiguration>& turn) {
-  std::vector<talk_base::SocketAddress> stun_hosts;
+  cricket::ServerAddresses stun_hosts;
   typedef std::vector<StunConfiguration>::const_iterator StunIt;
   for (StunIt stun_it = stun.begin(); stun_it != stun.end(); ++stun_it) {
-    stun_hosts.push_back(stun_it->server);
+    stun_hosts.insert(stun_it->server);
   }
 
-  talk_base::SocketAddress stun_addr;
-  if (!stun_hosts.empty()) {
-    stun_addr = stun_hosts.front();
-  }
   scoped_ptr<cricket::BasicPortAllocator> allocator(
       new cricket::BasicPortAllocator(
-          network_manager_.get(), socket_factory_.get(), stun_addr));
+          network_manager_.get(), socket_factory_.get(), stun_hosts));
 
   for (size_t i = 0; i < turn.size(); ++i) {
     cricket::RelayCredentials credentials(turn[i].username, turn[i].password);

talk/app/webrtc/webrtcsession_unittest.cc

@@ -306,12 +306,16 @@ 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);

talk/p2p/base/p2ptransportchannel_unittest.cc

@@ -50,6 +50,7 @@ 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;
@@ -152,12 +153,14 @@ 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_, kStunAddr, kRelayUdpIntAddr,
+        &ep1_.network_manager_,
-        kRelayTcpIntAddr, kRelaySslTcpIntAddr));
+        stun_servers, kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr));
     ep2_.allocator_.reset(new cricket::BasicPortAllocator(
-        &ep2_.network_manager_, kStunAddr, kRelayUdpIntAddr,
+        &ep2_.network_manager_,
-        kRelayTcpIntAddr, kRelaySslTcpIntAddr));
+        stun_servers, kRelayUdpIntAddr, kRelayTcpIntAddr, kRelaySslTcpIntAddr));
   }
 
  protected:
@@ -806,13 +809,17 @@ 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_),
-        kStunAddr, talk_base::SocketAddress(), talk_base::SocketAddress(),
+        stun_servers,
+        talk_base::SocketAddress(), talk_base::SocketAddress(),
         talk_base::SocketAddress()));
     GetEndpoint(1)->allocator_.reset(
         new cricket::BasicPortAllocator(&(GetEndpoint(1)->network_manager_),
-        kStunAddr, talk_base::SocketAddress(), talk_base::SocketAddress(),
+        stun_servers,
+        talk_base::SocketAddress(), talk_base::SocketAddress(),
         talk_base::SocketAddress()));
 
     cricket::RelayServerConfig relay_server(cricket::RELAY_GTURN);

talk/p2p/base/port.h

@@ -31,6 +31,7 @@
 #include <string>
 #include <vector>
 #include <map>
+#include <set>
 
 #include "talk/base/asyncpacketsocket.h"
 #include "talk/base/network.h"
@@ -109,6 +110,8 @@ struct ProtocolAddress {
       : address(a), proto(p), secure(sec) { }
 };
 
+typedef std::set<talk_base::SocketAddress> 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.

talk/p2p/base/port_unittest.cc

@@ -453,9 +453,11 @@ 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_, kStunAddr);
+                                      username_, password_, stun_servers);
     port->SetIceProtocolType(ice_protocol_);
     return port;
   }

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(addr);
+      port_->OnStunBindingRequestSucceeded(server_addr_, addr);
     }
 
-    // We will do a keep-alive regardless of whether this request suceeds.
+    // We will do a keep-alive regardless of whether this request succeeds.
     // 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();
+    port_->OnStunBindingOrResolveRequestFailed(server_addr_);
 
     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();
+    port_->OnStunBindingOrResolveRequestFailed(server_addr_);
 
     if (keep_alive_
         && (talk_base::TimeSince(start_time_) <= RETRY_TIMEOUT)) {
@@ -120,10 +120,60 @@ class StunBindingRequest : public StunRequest {
  private:
   UDPPort* port_;
   bool keep_alive_;
-  talk_base::SocketAddress server_addr_;
+  const 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<talk_base::SocketAddress, talk_base::AsyncResolverInterface*>(
+          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,
@@ -134,7 +184,6 @@ UDPPort::UDPPort(talk_base::Thread* thread,
       requests_(thread),
       socket_(socket),
       error_(0),
-      resolver_(NULL),
       ready_(false),
       stun_keepalive_delay_(KEEPALIVE_DELAY) {
 }
@@ -149,7 +198,6 @@ UDPPort::UDPPort(talk_base::Thread* thread,
       requests_(thread),
       socket_(NULL),
       error_(0),
-      resolver_(NULL),
       ready_(false),
       stun_keepalive_delay_(KEEPALIVE_DELAY) {
 }
@@ -172,9 +220,6 @@ bool UDPPort::Init() {
 }
 
 UDPPort::~UDPPort() {
-  if (resolver_) {
-    resolver_->Destroy(true);
-  }
   if (!SharedSocket())
     delete socket_;
 }
@@ -189,11 +234,11 @@ void UDPPort::PrepareAddress() {
 void UDPPort::MaybePrepareStunCandidate() {
   // Sending binding request to the STUN server if address is available to
   // prepare STUN candidate.
-  if (!server_addr_.IsNil()) {
+  if (!server_addresses_.empty()) {
-    SendStunBindingRequest();
+    SendStunBindingRequests();
   } else {
     // Port is done allocating candidates.
-    SetResult(true);
+    MaybeSetPortCompleteOrError();
   }
 }
 
@@ -254,12 +299,13 @@ 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_addr_.IsUnresolved() && remote_addr == server_addr_) {
+  if (server_addresses_.find(remote_addr) != server_addresses_.end()) {
     requests_.CheckResponse(data, size);
     return;
   }
@@ -275,76 +321,114 @@ void UDPPort::OnReadyToSend(talk_base::AsyncPacketSocket* socket) {
   Port::OnReadyToSend();
 }
 
-void UDPPort::SendStunBindingRequest() {
+void UDPPort::SendStunBindingRequests() {
   // 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());
-  if (server_addr_.IsUnresolved()) {
+
-    ResolveStunAddress();
+  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);
+
   } else if (socket_->GetState() == talk_base::AsyncPacketSocket::STATE_BOUND) {
     // Check if |server_addr_| is compatible with the port's ip.
-    if (IsCompatibleAddress(server_addr_)) {
+    if (IsCompatibleAddress(stun_addr)) {
-      requests_.Send(new StunBindingRequest(this, true, server_addr_));
+      requests_.Send(new StunBindingRequest(this, true, stun_addr));
     } else {
       // Since we can't send stun messages to the server, we should mark this
       // port ready.
-      OnStunBindingOrResolveRequestFailed();
+      LOG(LS_WARNING) << "STUN server address is incompatible.";
+      OnStunBindingOrResolveRequestFailed(stun_addr);
     }
   }
 }
 
-void UDPPort::ResolveStunAddress() {
+void UDPPort::OnStunBindingRequestSucceeded(
-  if (resolver_)
+    const talk_base::SocketAddress& stun_server_addr,
-    return;
+    const talk_base::SocketAddress& stun_reflected_addr) {
-
+  if (bind_request_succeeded_servers_.find(stun_server_addr) !=
-  resolver_ = socket_factory()->CreateAsyncResolver();
+          bind_request_succeeded_servers_.end()) {
-  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);
 
-  SendStunBindingRequest();
+  if (!SharedSocket() || stun_reflected_addr != socket_->GetLocalAddress()) {
-}
+    // If socket is shared and |stun_reflected_addr| is equal to local socket
-
-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_addr, socket_->GetLocalAddress(),
+    AddAddress(stun_reflected_addr, socket_->GetLocalAddress(),
                socket_->GetLocalAddress(), UDP_PROTOCOL_NAME,
                STUN_PORT_TYPE, ICE_TYPE_PREFERENCE_SRFLX, false);
   }
-  SetResult(true);
+  MaybeSetPortCompleteOrError();
 }
 
-void UDPPort::OnStunBindingOrResolveRequestFailed() {
+void UDPPort::OnStunBindingOrResolveRequestFailed(
-  if (ready_)  // Discarding failure response if port is already enabled.
+    const talk_base::SocketAddress& stun_server_addr) {
+  if (bind_request_failed_servers_.find(stun_server_addr) !=
+          bind_request_failed_servers_.end()) {
+    return;
+  }
+  bind_request_failed_servers_.insert(stun_server_addr);
+  MaybeSetPortCompleteOrError();
+}
+
+void UDPPort::MaybeSetPortCompleteOrError() {
+  if (ready_)
     return;
 
-  // If socket is shared, we should process local udp candidate.
+  // Do not set port ready if we are still waiting for bind responses.
-  SetResult(SharedSocket());
+  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;
-  if (success) {
+
+  // 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()) {
     SignalPortComplete(this);
   } else {
     SignalPortError(this);

talk/p2p/base/stunport.h

@@ -82,9 +82,12 @@ class UDPPort : public Port {
     return socket_->GetLocalAddress();
   }
 
-  const talk_base::SocketAddress& server_addr() const { return server_addr_; }
+  const ServerAddresses server_addresses() const {
-  void set_server_addr(const talk_base::SocketAddress& addr) {
+    return server_addresses_;
-    server_addr_ = addr;
+  }
+  void
+  set_server_addresses(const ServerAddresses& addresses) {
+    server_addresses_ = addresses;
   }
 
   virtual void PrepareAddress();
@@ -140,29 +143,64 @@ class UDPPort : public Port {
   // This method will send STUN binding request if STUN server address is set.
   void MaybePrepareStunCandidate();
 
-  void SendStunBindingRequest();
+  void SendStunBindingRequests();
 
  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<const talk_base::SocketAddress&, int> SignalDone;
+
+   private:
+    typedef std::map<talk_base::SocketAddress,
+                     talk_base::AsyncResolverInterface*> ResolverMap;
+
+    void OnResolveResult(talk_base::AsyncResolverInterface* resolver);
+
+    talk_base::PacketSocketFactory* socket_factory_;
+    ResolverMap resolvers_;
+  };
+
   // DNS resolution of the STUN server.
-  void ResolveStunAddress();
+  void ResolveStunAddress(const talk_base::SocketAddress& stun_addr);
-  void OnResolveResult(talk_base::AsyncResolverInterface* resolver);
+  void OnResolveResult(const talk_base::SocketAddress& input, int error);
+
+  void SendStunBindingRequest(const talk_base::SocketAddress& stun_addr);
 
   // Below methods handles binding request responses.
-  void OnStunBindingRequestSucceeded(const talk_base::SocketAddress& stun_addr);
+  void OnStunBindingRequestSucceeded(
-  void OnStunBindingOrResolveRequestFailed();
+      const talk_base::SocketAddress& stun_server_addr,
+      const talk_base::SocketAddress& stun_reflected_addr);
+  void OnStunBindingOrResolveRequestFailed(
+      const talk_base::SocketAddress& stun_server_addr);
 
   // 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 SetResult(bool success);
+  void MaybeSetPortCompleteOrError();
 
-  talk_base::SocketAddress server_addr_;
+  ServerAddresses server_addresses_;
+  ServerAddresses bind_request_succeeded_servers_;
+  ServerAddresses bind_request_failed_servers_;
   StunRequestManager requests_;
   talk_base::AsyncPacketSocket* socket_;
   int error_;
-  talk_base::AsyncResolverInterface* resolver_;
+  talk_base::scoped_ptr<AddressResolver> resolver_;
   bool ready_;
   int stun_keepalive_delay_;
 
@@ -171,17 +209,18 @@ class UDPPort : public Port {
 
 class StunPort : public UDPPort {
  public:
-  static StunPort* Create(talk_base::Thread* thread,
+  static StunPort* Create(
-                          talk_base::PacketSocketFactory* factory,
+      talk_base::Thread* thread,
-                          talk_base::Network* network,
+      talk_base::PacketSocketFactory* factory,
-                          const talk_base::IPAddress& ip,
+      talk_base::Network* network,
-                          int min_port, int max_port,
+      const talk_base::IPAddress& ip,
-                          const std::string& username,
+      int min_port, int max_port,
-                          const std::string& password,
+      const std::string& username,
-                          const talk_base::SocketAddress& server_addr) {
+      const std::string& password,
+      const ServerAddresses& servers) {
     StunPort* port = new StunPort(thread, factory, network,
                                   ip, min_port, max_port,
-                                  username, password, server_addr);
+                                  username, password, servers);
     if (!port->Init()) {
       delete port;
       port = NULL;
@@ -192,7 +231,7 @@ class StunPort : public UDPPort {
   virtual ~StunPort() {}
 
   virtual void PrepareAddress() {
-    SendStunBindingRequest();
+    SendStunBindingRequests();
   }
 
  protected:
@@ -200,12 +239,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 talk_base::SocketAddress& server_address)
+           const ServerAddresses& servers)
      : 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_addr(server_address);
+    set_server_addresses(servers);
   }
 };
 

talk/p2p/base/stunport_unittest.cc

@@ -30,15 +30,18 @@
 #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 kStunAddr("127.0.0.1", 5000);
+static const SocketAddress kStunAddr1("127.0.0.1", 5000);
+static const SocketAddress kStunAddr2("127.0.0.1", 4000);
 static const SocketAddress kBadAddr("0.0.0.1", 5000);
 static const SocketAddress kStunHostnameAddr("localhost", 5000);
 static const SocketAddress kBadHostnameAddr("not-a-real-hostname", 5000);
@@ -58,8 +61,10 @@ 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_(new cricket::TestStunServer(
+        stun_server_1_(new cricket::TestStunServer(
-          talk_base::Thread::Current(), kStunAddr)),
+          talk_base::Thread::Current(), kStunAddr1)),
+        stun_server_2_(new cricket::TestStunServer(
+          talk_base::Thread::Current(), kStunAddr2)),
         done_(false), error_(false), stun_keepalive_delay_(0) {
   }
 
@@ -68,10 +73,16 @@ 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), server_addr));
+        talk_base::CreateRandomString(22), stun_servers));
     stun_port_->set_stun_keepalive_delay(stun_keepalive_delay_);
     stun_port_->SignalPortComplete.connect(this,
         &StunPortTest::OnPortComplete);
@@ -89,7 +100,9 @@ class StunPortTest : public testing::Test,
         &network_, socket_.get(),
         talk_base::CreateRandomString(16), talk_base::CreateRandomString(22)));
     ASSERT_TRUE(stun_port_ != NULL);
-    stun_port_->set_server_addr(server_addr);
+    ServerAddresses stun_servers;
+    stun_servers.insert(server_addr);
+    stun_port_->set_server_addresses(stun_servers);
     stun_port_->SignalPortComplete.connect(this,
         &StunPortTest::OnPortComplete);
     stun_port_->SignalPortError.connect(this,
@@ -115,11 +128,17 @@ 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;
   }
@@ -138,7 +157,8 @@ class StunPortTest : public testing::Test,
   talk_base::Network network_;
   talk_base::BasicPacketSocketFactory socket_factory_;
   talk_base::scoped_ptr<cricket::UDPPort> stun_port_;
-  talk_base::scoped_ptr<cricket::TestStunServer> stun_server_;
+  talk_base::scoped_ptr<cricket::TestStunServer> stun_server_1_;
+  talk_base::scoped_ptr<cricket::TestStunServer> stun_server_2_;
   talk_base::scoped_ptr<talk_base::AsyncPacketSocket> socket_;
   bool done_;
   bool error_;
@@ -147,14 +167,14 @@ class StunPortTest : public testing::Test,
 
 // Test that we can create a STUN port
 TEST_F(StunPortTest, TestBasic) {
-  CreateStunPort(kStunAddr);
+  CreateStunPort(kStunAddr1);
   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(kStunAddr);
+  CreateStunPort(kStunAddr1);
   PrepareAddress();
   EXPECT_TRUE_WAIT(done(), kTimeoutMs);
   ASSERT_EQ(1U, port()->Candidates().size());
@@ -209,7 +229,7 @@ TEST_F(StunPortTest, TestKeepAliveResponse) {
 
 // Test that a local candidate can be generated using a shared socket.
 TEST_F(StunPortTest, TestSharedSocketPrepareAddress) {
-  CreateSharedStunPort(kStunAddr);
+  CreateSharedStunPort(kStunAddr1);
   PrepareAddress();
   EXPECT_TRUE_WAIT(done(), kTimeoutMs);
   ASSERT_EQ(1U, port()->Candidates().size());
@@ -232,3 +252,28 @@ 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());
+}

talk/p2p/client/basicportallocator.cc

@@ -186,23 +186,23 @@ BasicPortAllocator::BasicPortAllocator(
 BasicPortAllocator::BasicPortAllocator(
     talk_base::NetworkManager* network_manager,
     talk_base::PacketSocketFactory* socket_factory,
-    const talk_base::SocketAddress& stun_address)
+    const ServerAddresses& stun_servers)
     : network_manager_(network_manager),
       socket_factory_(socket_factory),
-      stun_address_(stun_address) {
+      stun_servers_(stun_servers) {
   ASSERT(socket_factory_ != NULL);
   Construct();
 }
 
 BasicPortAllocator::BasicPortAllocator(
     talk_base::NetworkManager* network_manager,
-    const talk_base::SocketAddress& stun_address,
+    const ServerAddresses& stun_servers,
     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_address_(stun_address) {
+      stun_servers_(stun_servers) {
 
   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_address(),
+  PortConfiguration* config = new PortConfiguration(allocator_->stun_servers(),
                                                     username(),
                                                     password());
 
@@ -422,7 +422,7 @@ void BasicPortAllocatorSession::DoAllocate() {
       }
 
       // Disables phases that are not specified in this config.
-      if (!config || config->stun_address.IsNil()) {
+      if (!config || config->StunServers().empty()) {
         // 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_->stun_address == config->stun_address) {
+    if (config_->StunServers() == config->StunServers()) {
-      // Already got this STUN server covered.
+      // Already got this STUN servers 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_address, use it to get server reflexive candidate
+        // If config has stun_servers, use it to get server reflexive candidate
         // otherwise use first TURN server which supports UDP.
-        if (config_ && !config_->stun_address.IsNil()) {
+        if (config_ && !config_->StunServers().empty()) {
           LOG(LS_INFO) << "AllocationSequence: UDPPort will be handling the "
                        <<  "STUN candidate generation.";
-          port->set_server_addr(config_->stun_address);
+          port->set_server_addresses(config_->StunServers());
         } else if (config_ &&
                    config_->SupportsProtocol(RELAY_TURN, PROTO_UDP)) {
-          port->set_server_addr(config_->GetFirstRelayServerAddress(
+          port->set_server_addresses(config_->GetRelayServerAddresses(
               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_->stun_address.IsNil());
+  ASSERT(config_ && !config_->StunServers().empty());
-  if (!(config_ && !config_->stun_address.IsNil())) {
+  if (!(config_ && !config_->StunServers().empty())) {
     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_->stun_address);
+                                config_->StunServers());
   if (port) {
     session_->AddAllocatedPort(port, this, true);
     // Since StunPort is not created using shared socket, |port| will not be
@@ -1115,9 +1115,27 @@ PortConfiguration::PortConfiguration(
     const talk_base::SocketAddress& stun_address,
     const std::string& username,
     const std::string& password)
-    : stun_address(stun_address),
+    : 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),
       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) {
@@ -1146,14 +1164,15 @@ bool PortConfiguration::SupportsProtocol(RelayType turn_type,
   return false;
 }
 
-talk_base::SocketAddress PortConfiguration::GetFirstRelayServerAddress(
+ServerAddresses PortConfiguration::GetRelayServerAddresses(
     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)) {
-      return relays[i].ports.front().address;
+      servers.insert(relays[i].ports.front().address);
     }
   }
-  return talk_base::SocketAddress();
+  return servers;
 }
 
 }  // namespace cricket

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 talk_base::SocketAddress& stun_server);
+                     const ServerAddresses& stun_servers);
   BasicPortAllocator(talk_base::NetworkManager* network_manager,
-                     const talk_base::SocketAddress& stun_server,
+                     const ServerAddresses& stun_servers,
                      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 talk_base::SocketAddress& stun_address() const {
+  const ServerAddresses& stun_servers() const {
-    return stun_address_;
+    return stun_servers_;
   }
 
   const std::vector<RelayServerConfig>& relays() const {
@@ -104,7 +104,7 @@ class BasicPortAllocator : public PortAllocator {
 
   talk_base::NetworkManager* network_manager_;
   talk_base::PacketSocketFactory* socket_factory_;
-  const talk_base::SocketAddress stun_address_;
+  const ServerAddresses stun_servers_;
   std::vector<RelayServerConfig> relays_;
   bool allow_tcp_listen_;
 };
@@ -217,17 +217,27 @@ 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<RelayServerConfig> 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);
 
@@ -235,9 +245,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 first server address for the matching
+  // Helper method returns the server addresses for the matching RelayType and
-  // RelayType and Protocol type.
+  // Protocol type.
-  talk_base::SocketAddress GetFirstRelayServerAddress(
+  ServerAddresses GetRelayServerAddresses(
       RelayType turn_type, ProtocolType type) const;
 };
 

talk/p2p/client/connectivitychecker.cc

@@ -288,7 +288,9 @@ 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_address = static_cast<StunPort*>(port)->server_addr();
+
+    nic_info->stun_server_addresses =
+        static_cast<StunPort*>(port)->server_addresses();
     nic_info->stun.rtt = now - nic_info->stun.start_time_ms;
   } else {
     LOG(LS_ERROR) << "Got stun address for non-existing nic";
@@ -303,7 +305,9 @@ 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_address = static_cast<StunPort*>(port)->server_addr();
+
+    nic_info->stun_server_addresses =
+        static_cast<StunPort*>(port)->server_addresses();
   }
 }
 
@@ -335,7 +339,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_address);
+                          username, password, config->stun_servers);
 }
 
 RelayPort* ConnectivityChecker::CreateRelayPort(
@@ -407,7 +411,9 @@ 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);
-  PortConfiguration config(stun_address_, username, password);
+  ServerAddresses stun_servers;
+  stun_servers.insert(stun_address_);
+  PortConfiguration config(stun_servers, username, password);
   std::vector<talk_base::Network*> networks;
   network_manager_->GetNetworks(&networks);
   if (networks.empty()) {

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;
-  talk_base::SocketAddress stun_server_address;
+  ServerAddresses stun_server_addresses;
   talk_base::SocketAddress media_server_address;
   ConnectInfo stun;
   ConnectInfo http;

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 talk_base::SocketAddress& server_addr)
+               const ServerAddresses& 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_address);
+                            config->stun_servers);
   }
   virtual RelayPort* CreateRelayPort(
       const std::string& username, const std::string& password,
@@ -254,7 +254,8 @@ class ConnectivityCheckerTest : public testing::Test {
     EXPECT_EQ(kExternalAddr, info.external_address);
 
     // Verify that the stun server address has been set.
-    EXPECT_EQ(kStunAddr, info.stun_server_address);
+    EXPECT_EQ(1U, info.stun_server_addresses.size());
+    EXPECT_EQ(kStunAddr, *(info.stun_server_addresses.begin()));
 
     // Verify that the media server address has been set. Don't care
     // about port since it is different for different protocols.

talk/p2p/client/httpportallocator.cc

@@ -142,7 +142,13 @@ 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.
-  PortConfiguration* config = new PortConfiguration(stun_hosts_[0],
+  ServerAddresses hosts;
+  for (std::vector<talk_base::SocketAddress>::iterator it = stun_hosts_.begin();
+      it != stun_hosts_.end(); ++it) {
+    hosts.insert(*it);
+  }
+
+  PortConfiguration* config = new PortConfiguration(hosts,
                                                     username(),
                                                     password());
   ConfigReady(config);
@@ -206,7 +212,13 @@ void HttpPortAllocatorSessionBase::ReceiveSessionResponse(
   std::string relay_tcp_port = map["relay.tcp_port"];
   std::string relay_ssltcp_port = map["relay.ssltcp_port"];
 
-  PortConfiguration* config = new PortConfiguration(stun_hosts_[0],
+  ServerAddresses hosts;
+  for (std::vector<talk_base::SocketAddress>::iterator it = stun_hosts_.begin();
+      it != stun_hosts_.end(); ++it) {
+    hosts.insert(*it);
+  }
+
+  PortConfiguration* config = new PortConfiguration(hosts,
                                                     map["username"],
                                                     map["password"]);
 

talk/p2p/client/portallocator_unittest.cc

@@ -48,6 +48,7 @@
 #include "talk/p2p/client/basicportallocator.h"
 #include "talk/p2p/client/httpportallocator.h"
 
+using cricket::ServerAddresses;
 using talk_base::SocketAddress;
 using talk_base::Thread;
 
@@ -115,10 +116,13 @@ 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);
   }
 
@@ -265,7 +269,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_address());
+  EXPECT_EQ(kStunAddr, *allocator().stun_servers().begin());
   ASSERT_EQ(1u, allocator().relays().size());
   EXPECT_EQ(cricket::RELAY_GTURN, allocator().relays()[0].type);
   // Empty relay credentials are used for GTURN.
@@ -696,8 +700,10 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNat) {
   AddInterface(kClientAddr);
   talk_base::scoped_ptr<talk_base::NATServer> 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_, kStunAddr));
+      &network_manager_, &nat_socket_factory_, stun_servers));
   allocator_->set_step_delay(cricket::kMinimumStepDelay);
   allocator_->set_flags(allocator().flags() |
                         cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
@@ -786,8 +792,10 @@ TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurn) {
   AddInterface(kClientAddr);
   talk_base::scoped_ptr<talk_base::NATServer> 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_, kStunAddr));
+      &network_manager_, &nat_socket_factory_, stun_servers));
   cricket::RelayServerConfig relay_server(cricket::RELAY_TURN);
   cricket::RelayCredentials credentials(kTurnUsername, kTurnPassword);
   relay_server.credentials = credentials;
@@ -895,6 +903,7 @@ 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());