Move PortAllocator creation out of LibjingleTransportFactory.

remoting/protocol/libjingle_transport_factory.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/protocol/libjingle_transport_factory.h"
 
 #include "base/message_loop_proxy.h"
 #include "jingle/glue/channel_socket_adapter.h"
 #include "jingle/glue/pseudotcp_adapter.h"
 #include "jingle/glue/utils.h"
 #include "net/base/net_errors.h"
 #include "remoting/base/constants.h"
 #include "remoting/protocol/channel_authenticator.h"
 #include "remoting/protocol/transport_config.h"
 #include "third_party/libjingle/source/talk/base/basicpacketsocketfactory.h"
 #include "third_party/libjingle/source/talk/base/network.h"
 #include "third_party/libjingle/source/talk/p2p/base/p2ptransportchannel.h"
-#include "third_party/libjingle/source/talk/p2p/client/httpportallocator.h"
+#include "third_party/libjingle/source/talk/p2p/client/basicportallocator.h"
 
 namespace remoting {
 namespace protocol {
 
 namespace {
 
 // Value is choosen to balance the extra latency against the reduced
 // load due to ACK traffic.
 const int kTcpAckDelayMilliseconds = 10;
 
 // Values for the TCP send and receive buffer size. This should be tuned to
 // accomodate high latency network but not backlog the decoding pipeline.
 const int kTcpReceiveBufferSize = 256 * 1024;
 const int kTcpSendBufferSize = kTcpReceiveBufferSize + 30 * 1024;
 
 class LibjingleStreamTransport : public StreamTransport,
                                  public sigslot::has_slots<> {
  public:
-  LibjingleStreamTransport(talk_base::NetworkManager* network_manager,
-                           talk_base::PacketSocketFactory* socket_factory);
+  LibjingleStreamTransport(cricket::PortAllocator* port_allocator,
+                           bool incoming_only);
   virtual ~LibjingleStreamTransport();
 
   // StreamTransport interface.
   virtual void Initialize(
       const std::string& name,
       const TransportConfig& config,
       Transport::EventHandler* event_handler,
       scoped_ptr<ChannelAuthenticator> authenticator) OVERRIDE;
   virtual void Connect(
       const StreamTransport::ConnectedCallback& callback) OVERRIDE;
@@ skipping 10 matching lines @@
 
   void OnTcpConnected(int result);
   void OnAuthenticationDone(net::Error error,
                             scoped_ptr<net::StreamSocket> socket);
 
   void OnChannelDestroyed();
 
   void NotifyConnected(scoped_ptr<net::StreamSocket> socket);
   void NotifyConnectFailed();
 
-  talk_base::NetworkManager* network_manager_;
-  talk_base::PacketSocketFactory* socket_factory_;
+  cricket::PortAllocator* port_allocator_;

[Wez, 2012/04/27 17:58:51]

So the LibjingleTransportFactory now owns the PortAllocator - do we have any
checks to verify that the LibjingleStreamTransports don't outlive it?

[Sergey Ulanov, 2012/04/27 22:51:14]

No. Don't think we need it - it would crash it quickly if the object are
destroyed in incorrect order.

+  bool incoming_only_;
 
   std::string name_;
   TransportConfig config_;
   EventHandler* event_handler_;
   StreamTransport::ConnectedCallback callback_;
   scoped_ptr<ChannelAuthenticator> authenticator_;
 
-  scoped_ptr<cricket::PortAllocator> port_allocator_;
-  cricket::HttpPortAllocator* http_port_allocator_;
   scoped_ptr<cricket::P2PTransportChannel> channel_;
 
   // We own |socket_| until it is connected.
   scoped_ptr<jingle_glue::PseudoTcpAdapter> socket_;
 
   DISALLOW_COPY_AND_ASSIGN(LibjingleStreamTransport);
 };
 
 LibjingleStreamTransport::LibjingleStreamTransport(
-    talk_base::NetworkManager* network_manager,
-    talk_base::PacketSocketFactory* socket_factory)
-    : network_manager_(network_manager),
-      socket_factory_(socket_factory),
-      event_handler_(NULL),
-      http_port_allocator_(NULL) {
+    cricket::PortAllocator* port_allocator,
+    bool incoming_only)
+    : port_allocator_(port_allocator),
+      incoming_only_(incoming_only),
+      event_handler_(NULL) {
 }
 
 LibjingleStreamTransport::~LibjingleStreamTransport() {
   DCHECK(event_handler_);
   event_handler_->OnTransportDeleted(this);
   // Channel should be already destroyed if we were connected.
   DCHECK(!is_connected() || socket_.get() == NULL);
 
   if (channel_.get()) {
     base::MessageLoopProxy::current()->DeleteSoon(
         FROM_HERE, channel_.release());
   }
-  if (port_allocator_.get()) {
-    base::MessageLoopProxy::current()->DeleteSoon(
-        FROM_HERE, port_allocator_.release());
-  }
 }
 
 void LibjingleStreamTransport::Initialize(
     const std::string& name,
     const TransportConfig& config,
     Transport::EventHandler* event_handler,
     scoped_ptr<ChannelAuthenticator> authenticator) {
   DCHECK(CalledOnValidThread());
 
   DCHECK(!name.empty());
   DCHECK(event_handler);
 
   // Can be initialized only once.
   DCHECK(name_.empty());
 
   name_ = name;
   config_ = config;
   event_handler_ = event_handler;
   authenticator_ = authenticator.Pass();
 }
 
 void LibjingleStreamTransport::Connect(
     const StreamTransport::ConnectedCallback& callback) {
   DCHECK(CalledOnValidThread());
 
   callback_ = callback;
 
-  // Create port allocator first.
-
-  // We always use PseudoTcp to provide a reliable channel. However
-  // when it is used together with TCP the performance is very bad
-  // so we explicitly disable TCP connections.
-  int port_allocator_flags = cricket::PORTALLOCATOR_DISABLE_TCP;
-  if (config_.nat_traversal_mode == TransportConfig::NAT_TRAVERSAL_ENABLED) {
-    http_port_allocator_ = new cricket::HttpPortAllocator(
-        network_manager_, socket_factory_, "");
-    port_allocator_.reset(http_port_allocator_);
-  } else {
-    port_allocator_flags |= cricket::PORTALLOCATOR_DISABLE_STUN |
-        cricket::PORTALLOCATOR_DISABLE_RELAY;
-    port_allocator_.reset(
-        new cricket::BasicPortAllocator(network_manager_, socket_factory_));
-  }
-  port_allocator_->set_flags(port_allocator_flags);
-
-  port_allocator_->SetPortRange(config_.min_port, config_.max_port);
-
   DCHECK(!channel_.get());
 
   // Create P2PTransportChannel, attach signal handlers and connect it.
   // TODO(sergeyu): Specify correct component ID for the channel.
   channel_.reset(new cricket::P2PTransportChannel(
-      name_, 0, NULL, port_allocator_.get()));
+      name_, 0, NULL, port_allocator_));
   channel_->SignalRequestSignaling.connect(
       this, &LibjingleStreamTransport::OnRequestSignaling);
   channel_->SignalCandidateReady.connect(
       this, &LibjingleStreamTransport::OnCandidateReady);
   channel_->SignalRouteChange.connect(
       this, &LibjingleStreamTransport::OnRouteChange);
-  if (config_.nat_traversal_mode == TransportConfig::NAT_TRAVERSAL_DISABLED)
-    channel_->set_incoming_only(true);
+  channel_->set_incoming_only(incoming_only_);
 
   channel_->Connect();
 
   // Create net::Socket adapter for the P2PTransportChannel.
   scoped_ptr<jingle_glue::TransportChannelSocketAdapter> channel_adapter(
       new jingle_glue::TransportChannelSocketAdapter(channel_.get()));
 
   channel_adapter->SetOnDestroyedCallback(base::Bind(
       &LibjingleStreamTransport::OnChannelDestroyed, base::Unretained(this)));
 
@@ skipping 122 matching lines @@
   DCHECK(!is_connected());
 
   socket_.reset();
 
   // This method may be called in response to a libjingle signal, so
   // libjingle objects must be deleted asynchronously.
   if (channel_.get()) {
     base::MessageLoopProxy::current()->DeleteSoon(
         FROM_HERE, channel_.release());
   }
-  if (port_allocator_.get()) {
-    base::MessageLoopProxy::current()->DeleteSoon(
-        FROM_HERE, port_allocator_.release());
-  }
 
   authenticator_.reset();
 
   NotifyConnected(scoped_ptr<net::StreamSocket>(NULL));
 }
 
 }  // namespace
 
+LibjingleTransportFactory::LibjingleTransportFactory(
+    scoped_ptr<talk_base::NetworkManager> network_manager,
+    scoped_ptr<talk_base::PacketSocketFactory> socket_factory,
+    scoped_ptr<cricket::PortAllocator> port_allocator,
+    bool incoming_only)
+    : network_manager_(network_manager.Pass()),
+      socket_factory_(socket_factory.Pass()),
+      port_allocator_(port_allocator.Pass()),
+      incoming_only_(incoming_only) {
+}
+
 LibjingleTransportFactory::LibjingleTransportFactory()
     : network_manager_(new talk_base::BasicNetworkManager()),
-      socket_factory_(new talk_base::BasicPacketSocketFactory(
-          talk_base::Thread::Current())) {
+      socket_factory_(new talk_base::BasicPacketSocketFactory()),
+      port_allocator_(new cricket::BasicPortAllocator(
+          network_manager_.get(), socket_factory_.get())) {
 }
 
 LibjingleTransportFactory::~LibjingleTransportFactory() {
   // This method may be called in response to a libjingle signal, so
   // libjingle objects must be deleted asynchronously.
   base::MessageLoopProxy::current()->DeleteSoon(
+      FROM_HERE, port_allocator_.release());
+  base::MessageLoopProxy::current()->DeleteSoon(
       FROM_HERE, socket_factory_.release());
   base::MessageLoopProxy::current()->DeleteSoon(
       FROM_HERE, network_manager_.release());
 }
 
 scoped_ptr<StreamTransport> LibjingleTransportFactory::CreateStreamTransport() {
   return scoped_ptr<StreamTransport>(
-      new LibjingleStreamTransport(network_manager_.get(),
-                                   socket_factory_.get()));
+      new LibjingleStreamTransport(port_allocator_.get(), incoming_only_));
 }
 
 scoped_ptr<DatagramTransport>
 LibjingleTransportFactory::CreateDatagramTransport() {
   NOTIMPLEMENTED();
   return scoped_ptr<DatagramTransport>(NULL);
 }
 
 }  // namespace protocol
 }  // namespace remoting