Removing webrtc::PortAllocatorFactoryInterface.

talk/app/webrtc/peerconnection.cc

Index: talk/app/webrtc/peerconnection.cc
diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc
index 83e29191c8a2ac1b5003e5f684d7755250f11dec..313193bbf915bd53beb30156d3a3d0b94b8aacbd 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -64,12 +64,6 @@ using webrtc::MediaStreamInterface;
 using webrtc::PeerConnectionInterface;
 using webrtc::RtpSenderInterface;
 using webrtc::StreamCollection;
-using webrtc::StunConfigurations;
-using webrtc::TurnConfigurations;
-typedef webrtc::PortAllocatorFactoryInterface::StunConfiguration
-    StunConfiguration;
-typedef webrtc::PortAllocatorFactoryInterface::TurnConfiguration
-    TurnConfiguration;
 
 static const char kDefaultStreamLabel[] = "default";
 static const char kDefaultAudioTrackLabel[] = "defaulta0";
@@ -84,8 +78,6 @@ static const size_t kTurnTransportTokensNum = 2;
 static const int kDefaultStunPort = 3478;
 static const int kDefaultStunTlsPort = 5349;
 static const char kTransport[] = "transport";
-static const char kUdpTransportType[] = "udp";
-static const char kTcpTransportType[] = "tcp";
 
 // NOTE: Must be in the same order as the ServiceType enum.
 static const char* kValidIceServiceTypes[] = {"stun", "stuns", "turn", "turns"};
@@ -220,12 +212,12 @@ bool ParseHostnameAndPortFromString(const std::string& in_str,
   return !host->empty();
 }
 
-// Adds a StunConfiguration or TurnConfiguration to the appropriate list,
+// Adds a STUN or TURN server to the appropriate list,
 // by parsing |url| and using the username/password in |server|.
 bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server,
                        const std::string& url,
-                       StunConfigurations* stun_config,
-                       TurnConfigurations* turn_config) {
+                       cricket::ServerAddresses* stun_servers,
+                       std::vector<cricket::RelayServerConfig>* turn_servers) {
   // draft-nandakumar-rtcweb-stun-uri-01
   // stunURI       = scheme ":" stun-host [ ":" stun-port ]
   // scheme        = "stun" / "stuns"
@@ -240,10 +232,10 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server,
   // transport-ext = 1*unreserved
   // turn-host     = IP-literal / IPv4address / reg-name
   // turn-port     = *DIGIT
-  RTC_DCHECK(stun_config != nullptr);
-  RTC_DCHECK(turn_config != nullptr);
+  RTC_DCHECK(stun_servers != nullptr);
+  RTC_DCHECK(turn_servers != nullptr);
   std::vector<std::string> tokens;
-  std::string turn_transport_type = kUdpTransportType;
+  cricket::ProtocolType turn_transport_type = cricket::PROTO_UDP;
   RTC_DCHECK(!url.empty());
   rtc::tokenize(url, '?', &tokens);
   std::string uri_without_transport = tokens[0];
@@ -254,11 +246,12 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server,
     if (tokens[0] == kTransport) {
       // As per above grammar transport param will be consist of lower case
       // letters.
-      if (tokens[1] != kUdpTransportType && tokens[1] != kTcpTransportType) {
+      if (!cricket::StringToProto(tokens[1].c_str(), &turn_transport_type) ||
+          (turn_transport_type != cricket::PROTO_UDP &&
+           turn_transport_type != cricket::PROTO_TCP)) {
         LOG(LS_WARNING) << "Transport param should always be udp or tcp.";
         return false;
       }
-      turn_transport_type = tokens[1];
     }
   }
 
@@ -297,7 +290,7 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server,
   int port = kDefaultStunPort;
   if (service_type == TURNS) {
     port = kDefaultStunTlsPort;
-    turn_transport_type = kTcpTransportType;
+    turn_transport_type = cricket::PROTO_TCP;
   }
 
   std::string address;
@@ -314,16 +307,14 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server,
   switch (service_type) {
     case STUN:
     case STUNS:
-      stun_config->push_back(StunConfiguration(address, port));
+      stun_servers->insert(rtc::SocketAddress(address, port));
       break;
     case TURN:
     case TURNS: {
       bool secure = (service_type == TURNS);
-      turn_config->push_back(TurnConfiguration(address, port,
-                                               username,
-                                               server.password,
-                                               turn_transport_type,
-                                               secure));
+      turn_servers->push_back(
+          cricket::RelayServerConfig(address, port, username, server.password,
+                                     turn_transport_type, secure));
       break;
     }
     case INVALID:
@@ -334,40 +325,6 @@ bool ParseIceServerUrl(const PeerConnectionInterface::IceServer& server,
   return true;
 }
 
-void ConvertToCricketIceServers(
-    const std::vector<StunConfiguration>& stuns,
-    const std::vector<TurnConfiguration>& turns,
-    cricket::ServerAddresses* cricket_stuns,
-    std::vector<cricket::RelayServerConfig>* cricket_turns) {
-  RTC_DCHECK(cricket_stuns && cricket_turns);
-  for (const StunConfiguration& stun : stuns) {
-    cricket_stuns->insert(stun.server);
-  }
-
-  int priority = static_cast<int>(turns.size() - 1);
-  for (const TurnConfiguration& turn : turns) {
-    cricket::RelayCredentials credentials(turn.username, turn.password);
-    cricket::RelayServerConfig relay_server(cricket::RELAY_TURN);
-    cricket::ProtocolType protocol;
-    // Using VERIFY because ParseIceServers should have already caught an
-    // invalid transport type.
-    if (!VERIFY(
-            cricket::StringToProto(turn.transport_type.c_str(), &protocol))) {
-      LOG(LS_WARNING) << "Ignoring TURN server " << turn.server << ". "
-                      << "Reason= Incorrect " << turn.transport_type
-                      << " transport parameter.";
-    } else {
-      relay_server.ports.push_back(
-          cricket::ProtocolAddress(turn.server, protocol, turn.secure));
-      relay_server.credentials = credentials;
-      relay_server.priority = priority;
-      cricket_turns->push_back(relay_server);
-    }
-    // First in the list gets highest priority.
-    --priority;
-  }
-}
-
 // Check if we can send |new_stream| on a PeerConnection.
 bool CanAddLocalMediaStream(webrtc::StreamCollectionInterface* current_streams,
                             webrtc::MediaStreamInterface* new_stream) {
@@ -560,8 +517,8 @@ bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints,
 }
 
 bool ParseIceServers(const PeerConnectionInterface::IceServers& servers,
-                     StunConfigurations* stun_config,
-                     TurnConfigurations* turn_config) {
+                     cricket::ServerAddresses* stun_servers,
+                     std::vector<cricket::RelayServerConfig>* turn_servers) {
   for (const webrtc::PeerConnectionInterface::IceServer& server : servers) {
     if (!server.urls.empty()) {
       for (const std::string& url : server.urls) {
@@ -569,13 +526,13 @@ bool ParseIceServers(const PeerConnectionInterface::IceServers& servers,
           LOG(LS_ERROR) << "Empty uri.";
           return false;
         }
-        if (!ParseIceServerUrl(server, url, stun_config, turn_config)) {
+        if (!ParseIceServerUrl(server, url, stun_servers, turn_servers)) {
           return false;
         }
       }
     } else if (!server.uri.empty()) {
       // Fallback to old .uri if new .urls isn't present.
-      if (!ParseIceServerUrl(server, server.uri, stun_config, turn_config)) {
+      if (!ParseIceServerUrl(server, server.uri, stun_servers, turn_servers)) {
         return false;
       }
     } else {
@@ -583,6 +540,11 @@ bool ParseIceServers(const PeerConnectionInterface::IceServers& servers,
       return false;
     }
   }

[pthatcher1, 2015/12/15 07:58:44]

Can you leave a comment explaining why this is important (that candidates for
different turn servers need different priorities)?

[Taylor Brandstetter, 2015/12/15 21:31:54]

Done.

+  int priority = static_cast<int>(turn_servers->size() - 1);
+  for (cricket::RelayServerConfig& turn_server : *turn_servers) {
+    // First in the list gets highest priority.
+    turn_server.priority = priority--;
+  }
   return true;
 }
 
@@ -613,30 +575,6 @@ PeerConnection::~PeerConnection() {
 bool PeerConnection::Initialize(
     const PeerConnectionInterface::RTCConfiguration& configuration,
     const MediaConstraintsInterface* constraints,
-    PortAllocatorFactoryInterface* allocator_factory,
-    rtc::scoped_ptr<DtlsIdentityStoreInterface> dtls_identity_store,
-    PeerConnectionObserver* observer) {
-  RTC_DCHECK(observer != nullptr);
-  if (!observer) {
-    return false;
-  }
-
-  // This Initialize function parses ICE servers an extra time, but it will
-  // be removed once all PortAllocaotrs support SetIceServers.
-  std::vector<PortAllocatorFactoryInterface::StunConfiguration> stun_config;
-  std::vector<PortAllocatorFactoryInterface::TurnConfiguration> turn_config;
-  if (!ParseIceServers(configuration.servers, &stun_config, &turn_config)) {
-    return false;
-  }
-  rtc::scoped_ptr<cricket::PortAllocator> allocator(
-      allocator_factory->CreatePortAllocator(stun_config, turn_config));
-  return Initialize(configuration, constraints, allocator.Pass(),
-                    dtls_identity_store.Pass(), observer);
-}
-
-bool PeerConnection::Initialize(
-    const PeerConnectionInterface::RTCConfiguration& configuration,
-    const MediaConstraintsInterface* constraints,
     rtc::scoped_ptr<cricket::PortAllocator> allocator,
     rtc::scoped_ptr<DtlsIdentityStoreInterface> dtls_identity_store,
     PeerConnectionObserver* observer) {
@@ -649,17 +587,12 @@ bool PeerConnection::Initialize(
 
   port_allocator_ = allocator.Pass();
 
-  std::vector<PortAllocatorFactoryInterface::StunConfiguration> stun_config;
-  std::vector<PortAllocatorFactoryInterface::TurnConfiguration> turn_config;
-  if (!ParseIceServers(configuration.servers, &stun_config, &turn_config)) {
+  cricket::ServerAddresses stun_servers;
+  std::vector<cricket::RelayServerConfig> turn_servers;
+  if (!ParseIceServers(configuration.servers, &stun_servers, &turn_servers)) {
     return false;
   }
-
-  cricket::ServerAddresses cricket_stuns;
-  std::vector<cricket::RelayServerConfig> cricket_turns;
-  ConvertToCricketIceServers(stun_config, turn_config, &cricket_stuns,
-                             &cricket_turns);
-  port_allocator_->SetIceServers(cricket_stuns, cricket_turns);
+  port_allocator_->SetIceServers(stun_servers, turn_servers);
 
   // To handle both internal and externally created port allocator, we will
   // enable BUNDLE here.
@@ -1212,16 +1145,12 @@ void PeerConnection::SetRemoteDescription(
 bool PeerConnection::SetConfiguration(const RTCConfiguration& config) {
   TRACE_EVENT0("webrtc", "PeerConnection::SetConfiguration");
   if (port_allocator_) {
-    std::vector<PortAllocatorFactoryInterface::StunConfiguration> stuns;
-    std::vector<PortAllocatorFactoryInterface::TurnConfiguration> turns;
-    if (!ParseIceServers(config.servers, &stuns, &turns)) {
+    cricket::ServerAddresses stun_servers;
+    std::vector<cricket::RelayServerConfig> turn_servers;
+    if (!ParseIceServers(config.servers, &stun_servers, &turn_servers)) {
       return false;
     }
-
-    cricket::ServerAddresses cricket_stuns;
-    std::vector<cricket::RelayServerConfig> cricket_turns;
-    ConvertToCricketIceServers(stuns, turns, &cricket_stuns, &cricket_turns);
-    port_allocator_->SetIceServers(cricket_stuns, cricket_turns);
+    port_allocator_->SetIceServers(stun_servers, turn_servers);
   }
   session_->SetIceConfig(session_->ParseIceConfig(config));
   return session_->SetIceTransports(config.type);