Simplify PortAllocatorBase and make PortAllocator creation synchronous.

remoting/client/plugin/pepper_port_allocator.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/client/plugin/pepper_port_allocator.h"
 
 #include <stdint.h>
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "net/base/net_util.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/cpp/url_loader.h"
 #include "ppapi/cpp/url_request_info.h"
 #include "ppapi/cpp/url_response_info.h"
 #include "ppapi/utility/completion_callback_factory.h"
 #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 {
 
 namespace {
 
 // Read buffer we allocate per read when reading response from
 // URLLoader. Normally the response from URL loader is smaller than 1kB.
 const int kReadSize = 1024;
 
 class PepperPortAllocatorSession : public protocol::PortAllocatorSessionBase {
  public:
   PepperPortAllocatorSession(
       protocol::PortAllocatorBase* 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:
+  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_;
   std::vector<char> relay_response_body_;
   bool relay_response_received_ = false;
 
   pp::CompletionCallbackFactory<PepperPortAllocatorSession> callback_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PepperPortAllocatorSession);
 };
 
 PepperPortAllocatorSession::PepperPortAllocatorSession(
     protocol::PortAllocatorBase* 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) {}
 
 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");
   std::stringstream headers;
   headers << "X-Talk-Google-Relay-Auth: " << relay_token() << "\n\r";
   headers << "X-Google-Relay-Auth: " << relay_token() << "\n\r";
   headers << "X-Stream-Type: " << "chromoting" << "\n\r";
   request_info.SetHeaders(headers.str());
 
   pp::CompletionCallback callback =
@@ skipping 61 matching lines @@
     ReceiveSessionResponse(std::string(relay_response_body_.begin(),
                                        relay_response_body_.end()));
     return;
   }
 
   ReadResponseBody();
 }
 
 }  // 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() {}
 
 cricket::PortAllocatorSession* PepperPortAllocator::CreateSessionInternal(
     const std::string& content_name,
     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