Pass TransportConfig to TransportFactory instead of Transport.

LibjingleTransport transport instances will be sharing a single
PortAllocator, so config needs to be passed to TransportFactory.

BUG=103991

Review URL: https://chromiumcodereview.appspot.com/10160012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@134637 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 70 insertions, 36 deletions

diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index 6475331..aea2466 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -107,22 +107,19 @@ void ChromotingHost::Start() {
       new talk_base::BasicNetworkManager());
   scoped_ptr<talk_base::PacketSocketFactory> socket_factory(
       new talk_base::BasicPacketSocketFactory());
-  scoped_ptr<cricket::PortAllocator> port_allocator;
+  scoped_ptr<cricket::HttpPortAllocatorBase> port_allocator(
+      new cricket::HttpPortAllocator(
+          network_manager.get(), socket_factory.get(), ""));
+
 
   // We always use PseudoTcp to provide a reliable channel. It
   // provides poor performance when combined with TCP-based transport,
   // so we have to disable TCP ports.
   int port_allocator_flags = cricket::PORTALLOCATOR_DISABLE_TCP;
-  if (network_settings_.nat_traversal_mode ==
+  if (network_settings_.nat_traversal_mode !=
       NetworkSettings::NAT_TRAVERSAL_ENABLED) {
-      port_allocator.reset(new cricket::HttpPortAllocator(
-          network_manager.get(), socket_factory.get(), ""));
-  } else {
     port_allocator_flags |= cricket::PORTALLOCATOR_DISABLE_STUN |
         cricket::PORTALLOCATOR_DISABLE_RELAY;
-    port_allocator.reset(
-        new cricket::BasicPortAllocator(network_manager.get(),
-                                        socket_factory.get()));
   }
   port_allocator->set_flags(port_allocator_flags);
   port_allocator->SetPortRange(network_settings_.min_port,
diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc
index bd3d720..8e067ea 100644
--- a/remoting/protocol/jingle_session.cc
+++ b/remoting/protocol/jingle_session.cc
@@ -187,8 +187,7 @@ void JingleSession::CreateStreamChannel(
       authenticator_->CreateChannelAuthenticator();
   scoped_ptr<StreamTransport> channel =
       session_manager_->transport_factory_->CreateStreamTransport();
-  channel->Initialize(name, session_manager_->transport_config_,
-                      this, channel_authenticator.Pass());
+  channel->Initialize(name, this, channel_authenticator.Pass());
   channel->Connect(callback);
   channels_[name] = channel.release();
 }
@@ -202,8 +201,7 @@ void JingleSession::CreateDatagramChannel(
       authenticator_->CreateChannelAuthenticator();
   scoped_ptr<DatagramTransport> channel =
       session_manager_->transport_factory_->CreateDatagramTransport();
-  channel->Initialize(name, session_manager_->transport_config_,
-                      this, channel_authenticator.Pass());
+  channel->Initialize(name, this, channel_authenticator.Pass());
   channel->Connect(callback);
   channels_[name] = channel.release();
 }
diff --git a/remoting/protocol/jingle_session_manager.cc b/remoting/protocol/jingle_session_manager.cc
index 0a9e87f..f3231af 100644
--- a/remoting/protocol/jingle_session_manager.cc
+++ b/remoting/protocol/jingle_session_manager.cc
@@ -12,6 +12,8 @@
 #include "remoting/protocol/content_description.h"
 #include "remoting/protocol/jingle_messages.h"
 #include "remoting/protocol/jingle_session.h"
+#include "remoting/protocol/transport.h"
+#include "remoting/protocol/transport_config.h"
 #include "third_party/libjingle/source/talk/base/socketaddress.h"
 #include "third_party/libjingle/source/talk/xmllite/xmlelement.h"
 
@@ -54,12 +56,16 @@ void JingleSessionManager::OnJingleInfo(
 
   // TODO(sergeyu): Add support for multiple STUN/relay servers when
   // it's implemented in libjingle and P2P Transport API.
-  transport_config_.stun_server = stun_hosts[0].ToString();
-  transport_config_.relay_server = relay_hosts[0];
-  transport_config_.relay_token = relay_token;
-  VLOG(1) << "STUN server: " << transport_config_.stun_server
-          << " Relay server: " << transport_config_.relay_server
-          << " Relay token: " << transport_config_.relay_token;
+  TransportConfig config;
+  config.stun_server = stun_hosts[0].ToString();
+  config.relay_server = relay_hosts[0];
+  config.relay_token = relay_token;
+  transport_factory_->SetTransportConfig(config);
+
+  VLOG(1) << "STUN server: " << config.stun_server
+          << " Relay server: " << config.relay_server
+          << " Relay token: " << config.relay_token;
+
 
   if (!ready_) {
     ready_ = true;
diff --git a/remoting/protocol/jingle_session_manager.h b/remoting/protocol/jingle_session_manager.h
index 651b968..6a3dc41 100644
--- a/remoting/protocol/jingle_session_manager.h
+++ b/remoting/protocol/jingle_session_manager.h
@@ -14,8 +14,6 @@
 #include "remoting/jingle_glue/signal_strategy.h"
 #include "remoting/protocol/jingle_messages.h"
 #include "remoting/protocol/session_manager.h"
-#include "remoting/protocol/transport.h"
-#include "remoting/protocol/transport_config.h"
 
 namespace pp {
 class Instance;
@@ -37,6 +35,7 @@ class JingleInfoRequest;
 namespace protocol {
 
 class JingleSession;
+class TransportFactory;
 
 // JingleSessionManager and JingleSession implement the subset of the
 // Jingle protocol used in Chromoting. JingleSessionManager provides
@@ -101,8 +100,6 @@ class JingleSessionManager : public SessionManager,
 
   bool ready_;
 
-  TransportConfig transport_config_;
-
   scoped_ptr<JingleInfoRequest> jingle_info_request_;
 
   SessionsMap sessions_;
diff --git a/remoting/protocol/libjingle_transport_factory.cc b/remoting/protocol/libjingle_transport_factory.cc
index 6790f9a..1c39f80 100644
--- a/remoting/protocol/libjingle_transport_factory.cc
+++ b/remoting/protocol/libjingle_transport_factory.cc
@@ -16,6 +16,7 @@
 #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/basicportallocator.h"
+#include "third_party/libjingle/source/talk/p2p/client/httpportallocator.h"
 
 namespace remoting {
 namespace protocol {
@@ -41,7 +42,6 @@ class LibjingleStreamTransport : public StreamTransport,
   // StreamTransport interface.
   virtual void Initialize(
       const std::string& name,
-      const TransportConfig& config,
       Transport::EventHandler* event_handler,
       scoped_ptr<ChannelAuthenticator> authenticator) OVERRIDE;
   virtual void Connect(
@@ -70,7 +70,6 @@ class LibjingleStreamTransport : public StreamTransport,
   bool incoming_only_;
 
   std::string name_;
-  TransportConfig config_;
   EventHandler* event_handler_;
   StreamTransport::ConnectedCallback callback_;
   scoped_ptr<ChannelAuthenticator> authenticator_;
@@ -105,7 +104,6 @@ LibjingleStreamTransport::~LibjingleStreamTransport() {
 
 void LibjingleStreamTransport::Initialize(
     const std::string& name,
-    const TransportConfig& config,
     Transport::EventHandler* event_handler,
     scoped_ptr<ChannelAuthenticator> authenticator) {
   DCHECK(CalledOnValidThread());
@@ -117,7 +115,6 @@ void LibjingleStreamTransport::Initialize(
   DCHECK(name_.empty());
 
   name_ = name;
-  config_ = config;
   event_handler_ = event_handler;
   authenticator_ = authenticator.Pass();
 }
@@ -294,10 +291,11 @@ void LibjingleStreamTransport::NotifyConnectFailed() {
 LibjingleTransportFactory::LibjingleTransportFactory(
     scoped_ptr<talk_base::NetworkManager> network_manager,
     scoped_ptr<talk_base::PacketSocketFactory> socket_factory,
-    scoped_ptr<cricket::PortAllocator> port_allocator,
+    scoped_ptr<cricket::HttpPortAllocatorBase> port_allocator,
     bool incoming_only)
     : network_manager_(network_manager.Pass()),
       socket_factory_(socket_factory.Pass()),
+      http_port_allocator_(port_allocator.get()),
       port_allocator_(port_allocator.Pass()),
       incoming_only_(incoming_only) {
 }
@@ -305,6 +303,7 @@ LibjingleTransportFactory::LibjingleTransportFactory(
 LibjingleTransportFactory::LibjingleTransportFactory()
     : network_manager_(new talk_base::BasicNetworkManager()),
       socket_factory_(new talk_base::BasicPacketSocketFactory()),
+      http_port_allocator_(NULL),
       port_allocator_(new cricket::BasicPortAllocator(
           network_manager_.get(), socket_factory_.get())),
       incoming_only_(false) {
@@ -321,6 +320,26 @@ LibjingleTransportFactory::~LibjingleTransportFactory() {
       FROM_HERE, network_manager_.release());
 }
 
+void LibjingleTransportFactory::SetTransportConfig(
+    const TransportConfig& config) {
+  if (http_port_allocator_) {
+    std::vector<talk_base::SocketAddress> stun_hosts;
+    talk_base::SocketAddress stun_address;
+    if (stun_address.FromString(config.stun_server)) {
+      stun_hosts.push_back(stun_address);
+      http_port_allocator_->SetStunHosts(stun_hosts);
+    } else {
+      LOG(ERROR) << "Failed to parse stun server address: "
+                 << config.stun_server;
+    }
+
+    std::vector<std::string> relay_hosts;
+    relay_hosts.push_back(config.relay_server);
+    http_port_allocator_->SetRelayHosts(relay_hosts);
+    http_port_allocator_->SetRelayToken(config.relay_token);
+  }
+}
+
 scoped_ptr<StreamTransport> LibjingleTransportFactory::CreateStreamTransport() {
   return scoped_ptr<StreamTransport>(
       new LibjingleStreamTransport(port_allocator_.get(), incoming_only_));
diff --git a/remoting/protocol/libjingle_transport_factory.h b/remoting/protocol/libjingle_transport_factory.h
index 0ea8fc6..fb47b00 100644
--- a/remoting/protocol/libjingle_transport_factory.h
+++ b/remoting/protocol/libjingle_transport_factory.h
@@ -8,6 +8,7 @@
 #include "remoting/protocol/transport.h"
 
 namespace cricket {
+class HttpPortAllocatorBase;
 class PortAllocator;
 }  // namespace cricket
 
@@ -24,7 +25,7 @@ class LibjingleTransportFactory : public TransportFactory {
   LibjingleTransportFactory(
       scoped_ptr<talk_base::NetworkManager> network_manager,
       scoped_ptr<talk_base::PacketSocketFactory> socket_factory,
-      scoped_ptr<cricket::PortAllocator> port_allocator,
+      scoped_ptr<cricket::HttpPortAllocatorBase> port_allocator,
       bool incoming_only);
 
   // Creates BasicNetworkManager, BasicPacketSocketFactory and
@@ -33,12 +34,17 @@ class LibjingleTransportFactory : public TransportFactory {
 
   virtual ~LibjingleTransportFactory();
 
+  // TransportFactory interface.
+  virtual void SetTransportConfig(const TransportConfig& config) OVERRIDE;
   virtual scoped_ptr<StreamTransport> CreateStreamTransport() OVERRIDE;
   virtual scoped_ptr<DatagramTransport> CreateDatagramTransport() OVERRIDE;
 
  private:
   scoped_ptr<talk_base::NetworkManager> network_manager_;
   scoped_ptr<talk_base::PacketSocketFactory> socket_factory_;
+  // Points to the same port allocator as |port_allocator_| or NULL if
+  // |port_allocator_| is not HttpPortAllocatorBase.
+  cricket::HttpPortAllocatorBase* http_port_allocator_;
   scoped_ptr<cricket::PortAllocator> port_allocator_;
   bool incoming_only_;
 
diff --git a/remoting/protocol/pepper_transport_factory.cc b/remoting/protocol/pepper_transport_factory.cc
index f593dfc..315981a 100644
--- a/remoting/protocol/pepper_transport_factory.cc
+++ b/remoting/protocol/pepper_transport_factory.cc
@@ -38,13 +38,13 @@ const int kTcpSendBufferSize = kTcpReceiveBufferSize + 30 * 1024;
 class PepperStreamTransport : public StreamTransport,
                               public PepperTransportSocketAdapter::Observer {
  public:
-  PepperStreamTransport(pp::Instance* pp_instance);
+  PepperStreamTransport(pp::Instance* pp_instance,
+                        const TransportConfig& config);
   virtual ~PepperStreamTransport();
 
   // StreamTransport interface.
   virtual void Initialize(
       const std::string& name,
-      const TransportConfig& config,
       Transport::EventHandler* event_handler,
       scoped_ptr<ChannelAuthenticator> authenticator) OVERRIDE;
   virtual void Connect(
@@ -67,8 +67,8 @@ class PepperStreamTransport : public StreamTransport,
   void NotifyConnectFailed();
 
   pp::Instance* pp_instance_;
-  std::string name_;
   TransportConfig config_;
+  std::string name_;
   EventHandler* event_handler_;
   StreamTransport::ConnectedCallback callback_;
   scoped_ptr<ChannelAuthenticator> authenticator_;
@@ -84,8 +84,10 @@ class PepperStreamTransport : public StreamTransport,
   DISALLOW_COPY_AND_ASSIGN(PepperStreamTransport);
 };
 
-PepperStreamTransport::PepperStreamTransport(pp::Instance* pp_instance)
+PepperStreamTransport::PepperStreamTransport(pp::Instance* pp_instance,
+                                             const TransportConfig& config)
     : pp_instance_(pp_instance),
+      config_(config),
       event_handler_(NULL),
       channel_(NULL),
       connected_(false) {
@@ -100,7 +102,6 @@ PepperStreamTransport::~PepperStreamTransport() {
 
 void PepperStreamTransport::Initialize(
     const std::string& name,
-    const TransportConfig& config,
     Transport::EventHandler* event_handler,
     scoped_ptr<ChannelAuthenticator> authenticator) {
   DCHECK(CalledOnValidThread());
@@ -112,7 +113,6 @@ void PepperStreamTransport::Initialize(
   DCHECK(name_.empty());
 
   name_ = name;
-  config_ = config;
   event_handler_ = event_handler;
   authenticator_ = authenticator.Pass();
 }
@@ -280,8 +280,13 @@ PepperTransportFactory::PepperTransportFactory(
 PepperTransportFactory::~PepperTransportFactory() {
 }
 
+void PepperTransportFactory::SetTransportConfig(const TransportConfig& config) {
+  config_ = config;
+}
+
 scoped_ptr<StreamTransport> PepperTransportFactory::CreateStreamTransport() {
-  return scoped_ptr<StreamTransport>(new PepperStreamTransport(pp_instance_));
+  return scoped_ptr<StreamTransport>(
+      new PepperStreamTransport(pp_instance_, config_));
 }
 
 scoped_ptr<DatagramTransport>
diff --git a/remoting/protocol/pepper_transport_factory.h b/remoting/protocol/pepper_transport_factory.h
index 94e7bbe..4678df9 100644
--- a/remoting/protocol/pepper_transport_factory.h
+++ b/remoting/protocol/pepper_transport_factory.h
@@ -6,6 +6,7 @@
 #define REMOTING_PROTOCOL_PEPPER_TRANSPORT_FACTORY_H_
 
 #include "remoting/protocol/transport.h"
+#include "remoting/protocol/transport_config.h"
 
 namespace pp {
 class Instance;
@@ -19,11 +20,14 @@ class PepperTransportFactory : public TransportFactory {
   PepperTransportFactory(pp::Instance* pp_instance);
   virtual ~PepperTransportFactory();
 
+  // TransportFactory interface.
+  virtual void SetTransportConfig(const TransportConfig& config) OVERRIDE;
   virtual scoped_ptr<StreamTransport> CreateStreamTransport() OVERRIDE;
   virtual scoped_ptr<DatagramTransport> CreateDatagramTransport() OVERRIDE;
 
  private:
   pp::Instance* pp_instance_;
+  TransportConfig config_;
 
   DISALLOW_COPY_AND_ASSIGN(PepperTransportFactory);
 };
diff --git a/remoting/protocol/transport.h b/remoting/protocol/transport.h
index 47de7de..251724c 100644
--- a/remoting/protocol/transport.h
+++ b/remoting/protocol/transport.h
@@ -91,7 +91,6 @@ class Transport : public base::NonThreadSafe {
   // Intialize the transport with the specified parameters.
   // |authenticator| is used to secure and authenticate the connection.
   virtual void Initialize(const std::string& name,
-                          const TransportConfig& config,
                           Transport::EventHandler* event_handler,
                           scoped_ptr<ChannelAuthenticator> authenticator) = 0;
 
@@ -141,6 +140,9 @@ class TransportFactory {
   TransportFactory() { }
   virtual ~TransportFactory() { }
 
+  // Sets configuration for the transports created by this factory.
+  virtual void SetTransportConfig(const TransportConfig& config) = 0;
+
   virtual scoped_ptr<StreamTransport> CreateStreamTransport() = 0;
   virtual scoped_ptr<DatagramTransport> CreateDatagramTransport() = 0;