Simplify PortAllocatorBase and make PortAllocator creation synchronous.

remoting/client/plugin/pepper_port_allocator.cc

Index: remoting/client/plugin/pepper_port_allocator.cc
diff --git a/remoting/client/plugin/pepper_port_allocator.cc b/remoting/client/plugin/pepper_port_allocator.cc
index f8554925d55905bd5511e23bb04cfc9527b6fe08..6a30e8694c20ed45973d6d03ea8d5e71847069ae 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 {
 
@@ -36,25 +37,22 @@ class PepperPortAllocatorSession : public protocol::PortAllocatorSessionBase {
       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:
+  PepperPortAllocator* allocator() override {
+    return static_cast<PepperPortAllocator*>(
+        BasicPortAllocatorSession::allocator());

[Jamie, 2016/01/09 00:12:07]

Optional: I'm not a big fan of this cast either, though at least there's an
inheritance relation that implies a somewhat tight coupling between the two
classes. Personally, I'd probably extract the pp::InstanceHandle explicitly in
the ctor and save it, but perhaps that's overkill.

I also question why allocator() needs to be virtual in the base class (it's the
only one of the inline getters that is) but that's out of scope for this CL.

[Sergey Ulanov, 2016/01/11 20:01:04]

Done.

+  }
+
   void OnUrlOpened(int32_t result);
   void ReadResponseBody();
   void OnResponseBodyRead(int32_t result);
 
-  pp::InstanceHandle instance_;
-
   cricket::ServerAddresses stun_hosts_;
 
   scoped_ptr<pp::URLLoader> relay_url_loader_;
@@ -71,54 +69,18 @@ PepperPortAllocatorSession::PepperPortAllocatorSession(
     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) {}
 
 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(allocator()->instance()));
+  pp::URLRequestInfo request_info(allocator()->instance());
   std::string url = "https://" + host + GetSessionRequestUrl() + "&sn=1";
   request_info.SetURL(url);
   request_info.SetMethod("GET");
@@ -200,22 +162,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 instance)
+    : PortAllocatorBase(
+          make_scoped_ptr(new PepperNetworkManager(instance)),
+          make_scoped_ptr(new PepperPacketSocketFactory(instance)),
+          transport_context),
+      instance_(instance) {}
 
 PepperPortAllocator::~PepperPortAllocator() {}
 
@@ -224,20 +178,20 @@ 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)
+    pp::InstanceHandle instance)
     : instance_(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, instance_));
 }
 
 }  // namespace remoting