Simplify PortAllocatorBase and make PortAllocator creation synchronous.

Previously PortAllocator instances were created asynchronously in
TransportContext. Making PortAllocator synchronous allows to simplify
logic in other places, particularly in WebrtcTransport. Also simplified
PortAllocatorBase - it no longer stores STUN and Relay information and
instead instances of PortAllocatorSessionBase call TransportContext
to request fresh information when allocating ports.

Also removed duplicate logic from PepperPortAllocator and
ChromiumPortAllocator. Specifically they were filtering TCP-based relay
configs, but that's not longer necessary because PortAllocatorBase
is responsible for that now.

BUG=547158

Review URL: https://codereview.chromium.org/1571943002

Cr-Commit-Position: refs/heads/master@{#368701}

2 files changed, 36 insertions, 84 deletions

diff --git a/remoting/client/plugin/pepper_port_allocator.cc b/remoting/client/plugin/pepper_port_allocator.cc
index f855492..6f2e27a 100644
--- a/remoting/client/plugin/pepper_port_allocator.cc
+++ b/remoting/client/plugin/pepper_port_allocator.cc
@@ -20,6 +20,7 @@
 #include "remoting/client/plugin/pepper_network_manager.h"
 #include "remoting/client/plugin/pepper_packet_socket_factory.h"
 #include "remoting/client/plugin/pepper_util.h"
+#include "remoting/protocol/transport_context.h"
 
 namespace remoting {
 
@@ -32,20 +33,14 @@ const int kReadSize = 1024;
 class PepperPortAllocatorSession : public protocol::PortAllocatorSessionBase {
  public:
   PepperPortAllocatorSession(
-      protocol::PortAllocatorBase* allocator,
+      PepperPortAllocator* allocator,
       const std::string& content_name,
       int component,
       const std::string& ice_username_fragment,
-      const std::string& ice_password,
-      const std::vector<rtc::SocketAddress>& stun_hosts,
-      const std::vector<std::string>& relay_hosts,
-      const std::string& relay_token,
-      const pp::InstanceHandle& instance);
+      const std::string& ice_password);
   ~PepperPortAllocatorSession() override;
 
   // PortAllocatorBase overrides.
-  void ConfigReady(cricket::PortConfiguration* config) override;
-  void GetPortConfigurations() override;
   void SendSessionRequest(const std::string& host) override;
 
  private:
@@ -53,7 +48,7 @@ class PepperPortAllocatorSession : public protocol::PortAllocatorSessionBase {
   void ReadResponseBody();
   void OnResponseBodyRead(int32_t result);
 
-  pp::InstanceHandle instance_;
+  pp::InstanceHandle pp_instance_;
 
   cricket::ServerAddresses stun_hosts_;
 
@@ -67,58 +62,23 @@ class PepperPortAllocatorSession : public protocol::PortAllocatorSessionBase {
 };
 
 PepperPortAllocatorSession::PepperPortAllocatorSession(
-    protocol::PortAllocatorBase* allocator,
+    PepperPortAllocator* allocator,
     const std::string& content_name,
     int component,
     const std::string& ice_username_fragment,
-    const std::string& ice_password,
-    const std::vector<rtc::SocketAddress>& stun_hosts,
-    const std::vector<std::string>& relay_hosts,
-    const std::string& relay_token,
-    const pp::InstanceHandle& instance)
+    const std::string& ice_password)
     : PortAllocatorSessionBase(allocator,
                                content_name,
                                component,
                                ice_username_fragment,
-                               ice_password,
-                               stun_hosts,
-                               relay_hosts,
-                               relay_token),
-      instance_(instance),
-      stun_hosts_(stun_hosts.begin(), stun_hosts.end()),
-      callback_factory_(this) {}
+                               ice_password),
+      pp_instance_(allocator->pp_instance()) {}
 
 PepperPortAllocatorSession::~PepperPortAllocatorSession() {}
 
-void PepperPortAllocatorSession::ConfigReady(
-    cricket::PortConfiguration* config) {
-  // Filter out non-UDP relay ports, so that we don't try using TCP.
-  for (cricket::PortConfiguration::RelayList::iterator relay =
-           config->relays.begin(); relay != config->relays.end(); ++relay) {
-    cricket::PortList filtered_ports;
-    for (cricket::PortList::iterator port =
-             relay->ports.begin(); port != relay->ports.end(); ++port) {
-      if (port->proto == cricket::PROTO_UDP) {
-        filtered_ports.push_back(*port);
-      }
-    }
-    relay->ports = filtered_ports;
-  }
-  cricket::BasicPortAllocatorSession::ConfigReady(config);
-}
-
-void PepperPortAllocatorSession::GetPortConfigurations() {
-  // Add a configuration without relay response first so local and STUN
-  // candidates can be allocated without waiting for the relay response.
-  ConfigReady(new cricket::PortConfiguration(
-      stun_hosts_, std::string(), std::string()));
-
-  TryCreateRelaySession();
-}
-
 void PepperPortAllocatorSession::SendSessionRequest(const std::string& host) {
-  relay_url_loader_.reset(new pp::URLLoader(instance_));
-  pp::URLRequestInfo request_info(instance_);
+  relay_url_loader_.reset(new pp::URLLoader(pp_instance_));
+  pp::URLRequestInfo request_info(pp_instance_);
   std::string url = "https://" + host + GetSessionRequestUrl() + "&sn=1";
   request_info.SetURL(url);
   request_info.SetMethod("GET");
@@ -200,22 +160,14 @@ void PepperPortAllocatorSession::OnResponseBodyRead(int32_t result) {
 
 }  // namespace
 
-// static
-scoped_ptr<PepperPortAllocator> PepperPortAllocator::Create(
-    const pp::InstanceHandle& instance) {
-  return make_scoped_ptr(new PepperPortAllocator(
-      instance, make_scoped_ptr(new PepperNetworkManager(instance)),
-      make_scoped_ptr(new PepperPacketSocketFactory(instance))));
-}
-
 PepperPortAllocator::PepperPortAllocator(
-    const pp::InstanceHandle& instance,
-    scoped_ptr<rtc::NetworkManager> network_manager,
-    scoped_ptr<rtc::PacketSocketFactory> socket_factory)
-    : PortAllocatorBase(network_manager.get(), socket_factory.get()),
-      instance_(instance),
-      network_manager_(std::move(network_manager)),
-      socket_factory_(std::move(socket_factory)) {}
+    scoped_refptr<protocol::TransportContext> transport_context,
+    pp::InstanceHandle pp_instance)
+    : PortAllocatorBase(
+          make_scoped_ptr(new PepperNetworkManager(pp_instance)),
+          make_scoped_ptr(new PepperPacketSocketFactory(pp_instance)),
+          transport_context),
+      pp_instance_(pp_instance) {}
 
 PepperPortAllocator::~PepperPortAllocator() {}
 
@@ -224,20 +176,21 @@ cricket::PortAllocatorSession* PepperPortAllocator::CreateSessionInternal(
     int component,
     const std::string& ice_username_fragment,
     const std::string& ice_password) {
-   return new PepperPortAllocatorSession(
-       this, content_name, component, ice_username_fragment, ice_password,
-       stun_hosts(), relay_hosts(), relay_token(), instance_);
+  return new PepperPortAllocatorSession(this, content_name, component,
+                                        ice_username_fragment, ice_password);
 }
 
 PepperPortAllocatorFactory::PepperPortAllocatorFactory(
-    const pp::InstanceHandle& instance)
-    : instance_(instance) {}
+    pp::InstanceHandle pp_instance)
+    : pp_instance_(pp_instance) {}
 
 PepperPortAllocatorFactory::~PepperPortAllocatorFactory() {}
 
-scoped_ptr<protocol::PortAllocatorBase>
-PepperPortAllocatorFactory::CreatePortAllocator() {
-  return PepperPortAllocator::Create(instance_);
+scoped_ptr<cricket::PortAllocator>
+PepperPortAllocatorFactory::CreatePortAllocator(
+    scoped_refptr<protocol::TransportContext> transport_context) {
+  return make_scoped_ptr(
+      new PepperPortAllocator(transport_context, pp_instance_));
 }
 
 }  // namespace remoting
diff --git a/remoting/client/plugin/pepper_port_allocator.h b/remoting/client/plugin/pepper_port_allocator.h
index 19fb03d..a254977 100644
--- a/remoting/client/plugin/pepper_port_allocator.h
+++ b/remoting/client/plugin/pepper_port_allocator.h
@@ -18,10 +18,13 @@ namespace remoting {
 // client plugin. It uses Pepper URLLoader API when creating relay sessions.
 class PepperPortAllocator : public protocol::PortAllocatorBase {
  public:
-  static scoped_ptr<PepperPortAllocator> Create(
-      const pp::InstanceHandle& instance);
+  PepperPortAllocator(
+      scoped_refptr<protocol::TransportContext> transport_context,
+      pp::InstanceHandle pp_instance);
   ~PepperPortAllocator() override;
 
+  pp::InstanceHandle pp_instance() { return pp_instance_; }
+
   // PortAllocatorBase overrides.
   cricket::PortAllocatorSession* CreateSessionInternal(
       const std::string& content_name,
@@ -30,12 +33,7 @@ class PepperPortAllocator : public protocol::PortAllocatorBase {
       const std::string& ice_password) override;
 
  private:
-  PepperPortAllocator(
-      const pp::InstanceHandle& instance,
-      scoped_ptr<rtc::NetworkManager> network_manager,
-      scoped_ptr<rtc::PacketSocketFactory> socket_factory);
-
-  pp::InstanceHandle instance_;
+  pp::InstanceHandle pp_instance_;
   scoped_ptr<rtc::NetworkManager> network_manager_;
   scoped_ptr<rtc::PacketSocketFactory> socket_factory_;
 
@@ -44,14 +42,15 @@ class PepperPortAllocator : public protocol::PortAllocatorBase {
 
 class PepperPortAllocatorFactory : public protocol::PortAllocatorFactory {
  public:
-  PepperPortAllocatorFactory(const pp::InstanceHandle& instance);
+  PepperPortAllocatorFactory(pp::InstanceHandle pp_instance);
   ~PepperPortAllocatorFactory() override;
 
    // PortAllocatorFactory interface.
-  scoped_ptr<protocol::PortAllocatorBase> CreatePortAllocator() override;
+  scoped_ptr<cricket::PortAllocator> CreatePortAllocator(
+      scoped_refptr<protocol::TransportContext> transport_context) override;
 
  private:
-  pp::InstanceHandle instance_;
+  pp::InstanceHandle pp_instance_;
 
   DISALLOW_COPY_AND_ASSIGN(PepperPortAllocatorFactory);
 };