From c563e691a061f90d9a52c4ebcea7157339b386d6 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Mon, 14 Dec 2015 12:30:12 -0800 Subject: Reland of Removing references to webrtc::PortAllocatorFactoryInterface. Relanding after fixing conflict with: https://codereview.chromium.org/1506383004 Original description: Now using a new CreatePeerConnection method that takes a PortAllocator as input. This removes the need for some boilerplate code and code duplication between webrtc and chromium. It also fixes an issue with TURN candidate priorities. BUG=webrtc:5209 Review URL: https://codereview.chromium.org/1516163002 Cr-Commit-Position: refs/heads/master@{#365085} --- .../webrtc/peer_connection_dependency_factory.cc | 109 +++++---------------- content/renderer/p2p/filtering_network_manager.h | 3 +- content/renderer/p2p/port_allocator.cc | 75 +------------- content/renderer/p2p/port_allocator.h | 47 --------- 4 files changed, 28 insertions(+), 206 deletions(-) (limited to 'content/renderer') diff --git a/content/renderer/media/webrtc/peer_connection_dependency_factory.cc b/content/renderer/media/webrtc/peer_connection_dependency_factory.cc index 3775d3e..a7b373e 100644 --- a/content/renderer/media/webrtc/peer_connection_dependency_factory.cc +++ b/content/renderer/media/webrtc/peer_connection_dependency_factory.cc @@ -136,87 +136,6 @@ void HarmonizeConstraintsAndEffects(RTCMediaConstraints* constraints, } } -class P2PPortAllocatorFactory - : public rtc::RefCountedObject { - public: - P2PPortAllocatorFactory( - scoped_ptr media_permission, - const scoped_refptr& socket_dispatcher, - rtc::NetworkManager* network_manager, - rtc::PacketSocketFactory* socket_factory, - const P2PPortAllocator::Config& config, - const GURL& origin, - const scoped_refptr& task_runner) - : media_permission_(media_permission.Pass()), - socket_dispatcher_(socket_dispatcher), - network_manager_(network_manager), - socket_factory_(socket_factory), - config_(config), - origin_(origin), - task_runner_(task_runner) {} - - cricket::PortAllocator* CreatePortAllocator( - const std::vector& stun_servers, - const std::vector& turn_configurations) override { - P2PPortAllocator::Config config = config_; - for (size_t i = 0; i < stun_servers.size(); ++i) { - config.stun_servers.insert(rtc::SocketAddress( - stun_servers[i].server.hostname(), - stun_servers[i].server.port())); - } - for (size_t i = 0; i < turn_configurations.size(); ++i) { - P2PPortAllocator::Config::RelayServerConfig relay_config; - relay_config.server_address = turn_configurations[i].server.hostname(); - relay_config.port = turn_configurations[i].server.port(); - relay_config.username = turn_configurations[i].username; - relay_config.password = turn_configurations[i].password; - relay_config.transport_type = turn_configurations[i].transport_type; - relay_config.secure = turn_configurations[i].secure; - config.relays.push_back(relay_config); - } - - scoped_ptr network_manager; - if (config.enable_multiple_routes) { - media::MediaPermission* media_permission = media_permission_.get(); - FilteringNetworkManager* filtering_network_manager = - new FilteringNetworkManager(network_manager_, origin_, - media_permission_.Pass()); - if (media_permission) { - // Start permission check earlier to reduce any impact to call set up - // time. It's safe to use Unretained here since both destructor and - // Initialize can only be called on the worker thread. - task_runner_->PostTask( - FROM_HERE, base::Bind(&FilteringNetworkManager::Initialize, - base::Unretained(filtering_network_manager))); - } - network_manager.reset(filtering_network_manager); - } else { - network_manager.reset(new EmptyNetworkManager(network_manager_)); - } - - return new P2PPortAllocator(socket_dispatcher_, network_manager.Pass(), - socket_factory_, config, origin_, task_runner_); - } - - protected: - ~P2PPortAllocatorFactory() override {} - - private: - // Ownership of |media_permission_| will be passed to FilteringNetworkManager - // during CreatePortAllocator. - scoped_ptr media_permission_; - - scoped_refptr socket_dispatcher_; - rtc::NetworkManager* network_manager_; - rtc::PacketSocketFactory* socket_factory_; - P2PPortAllocator::Config config_; - GURL origin_; - - // This is the worker thread where |media_permission_| and |network_manager_| - // live on. - scoped_refptr task_runner_; -}; - PeerConnectionDependencyFactory::PeerConnectionDependencyFactory( P2PSocketDispatcher* p2p_socket_dispatcher) : network_manager_(NULL), @@ -541,14 +460,30 @@ PeerConnectionDependencyFactory::CreatePeerConnection( const GURL& requesting_origin = GURL(web_frame->document().url().spec()).GetOrigin(); - scoped_refptr pa_factory = - new P2PPortAllocatorFactory( - media_permission.Pass(), p2p_socket_dispatcher_, network_manager_, - socket_factory_.get(), port_config, requesting_origin, - chrome_worker_thread_.task_runner()); + scoped_ptr network_manager; + if (port_config.enable_multiple_routes) { + media::MediaPermission* media_permission_ptr = media_permission.get(); + FilteringNetworkManager* filtering_network_manager = + new FilteringNetworkManager(network_manager_, requesting_origin, + std::move(media_permission)); + if (media_permission_ptr) { + // Start permission check earlier to reduce any impact to call set up + // time. It's safe to use Unretained here since both destructor and + // Initialize can only be called on the worker thread. + chrome_worker_thread_.task_runner()->PostTask( + FROM_HERE, base::Bind(&FilteringNetworkManager::Initialize, + base::Unretained(filtering_network_manager))); + } + network_manager.reset(filtering_network_manager); + } else { + network_manager.reset(new EmptyNetworkManager(network_manager_)); + } + rtc::scoped_ptr port_allocator(new P2PPortAllocator( + p2p_socket_dispatcher_, std::move(network_manager), socket_factory_.get(), + port_config, requesting_origin, chrome_worker_thread_.task_runner())); return GetPcFactory() - ->CreatePeerConnection(config, constraints, pa_factory.get(), + ->CreatePeerConnection(config, constraints, std::move(port_allocator), std::move(identity_store), observer) .get(); } diff --git a/content/renderer/p2p/filtering_network_manager.h b/content/renderer/p2p/filtering_network_manager.h index 167666a..8eb7bba 100644 --- a/content/renderer/p2p/filtering_network_manager.h +++ b/content/renderer/p2p/filtering_network_manager.h @@ -45,7 +45,8 @@ class FilteringNetworkManager : public rtc::NetworkManagerBase, CONTENT_EXPORT ~FilteringNetworkManager() override; - // Check mic/camera permission. This is called by P2PPortAllocatorFactory. + // Check mic/camera permission. + // This is called by PeerConnectionDependencyFactory. CONTENT_EXPORT void Initialize(); // rtc::NetworkManager: diff --git a/content/renderer/p2p/port_allocator.cc b/content/renderer/p2p/port_allocator.cc index 681e5cb..1f2d1e3 100644 --- a/content/renderer/p2p/port_allocator.cc +++ b/content/renderer/p2p/port_allocator.cc @@ -11,18 +11,6 @@ namespace content { -P2PPortAllocator::Config::Config() {} - -P2PPortAllocator::Config::~Config() { -} - -P2PPortAllocator::Config::RelayServerConfig::RelayServerConfig() - : port(0) { -} - -P2PPortAllocator::Config::RelayServerConfig::~RelayServerConfig() { -} - P2PPortAllocator::P2PPortAllocator( const scoped_refptr& socket_dispatcher, scoped_ptr network_manager, @@ -37,10 +25,12 @@ P2PPortAllocator::P2PPortAllocator( origin_(origin), network_manager_task_runner_(task_runner) { uint32 flags = 0; - if (!config_.enable_multiple_routes) + if (!config_.enable_multiple_routes) { flags |= cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION; - if (!config_.enable_default_local_candidate) + } + if (!config_.enable_default_local_candidate) { flags |= cricket::PORTALLOCATOR_DISABLE_DEFAULT_LOCAL_CANDIDATE; + } if (!config_.enable_nonproxied_udp) { flags |= cricket::PORTALLOCATOR_DISABLE_UDP | cricket::PORTALLOCATOR_DISABLE_STUN | @@ -65,61 +55,4 @@ P2PPortAllocator::~P2PPortAllocator() { network_manager_.release()); } -cricket::PortAllocatorSession* P2PPortAllocator::CreateSessionInternal( - const std::string& content_name, - int component, - const std::string& ice_username_fragment, - const std::string& ice_password) { - return new P2PPortAllocatorSession( - this, content_name, component, ice_username_fragment, ice_password); -} - -P2PPortAllocatorSession::P2PPortAllocatorSession( - P2PPortAllocator* allocator, - const std::string& content_name, - int component, - const std::string& ice_username_fragment, - const std::string& ice_password) - : cricket::BasicPortAllocatorSession(allocator, - content_name, - component, - ice_username_fragment, - ice_password), - allocator_(allocator) { -} - -P2PPortAllocatorSession::~P2PPortAllocatorSession() { -} - -void P2PPortAllocatorSession::GetPortConfigurations() { - const P2PPortAllocator::Config& config = allocator_->config_; - cricket::PortConfiguration* port_config = new cricket::PortConfiguration( - config.stun_servers, std::string(), std::string()); - - for (size_t i = 0; i < config.relays.size(); ++i) { - cricket::RelayCredentials credentials(config.relays[i].username, - config.relays[i].password); - cricket::RelayServerConfig relay_server(cricket::RELAY_TURN); - cricket::ProtocolType protocol; - if (!cricket::StringToProto(config.relays[i].transport_type.c_str(), - &protocol)) { - DLOG(WARNING) << "Ignoring TURN server " - << config.relays[i].server_address << ". " - << "Reason= Incorrect " - << config.relays[i].transport_type - << " transport parameter."; - continue; - } - - relay_server.ports.push_back(cricket::ProtocolAddress( - rtc::SocketAddress(config.relays[i].server_address, - config.relays[i].port), - protocol, - config.relays[i].secure)); - relay_server.credentials = credentials; - port_config->AddRelay(relay_server); - } - ConfigReady(port_config); -} - } // namespace content diff --git a/content/renderer/p2p/port_allocator.h b/content/renderer/p2p/port_allocator.h index 28ebf94..e843676 100644 --- a/content/renderer/p2p/port_allocator.h +++ b/content/renderer/p2p/port_allocator.h @@ -12,31 +12,11 @@ namespace content { -class P2PPortAllocatorSession; class P2PSocketDispatcher; class P2PPortAllocator : public cricket::BasicPortAllocator { public: struct Config { - Config(); - ~Config(); - - struct RelayServerConfig { - RelayServerConfig(); - ~RelayServerConfig(); - - std::string username; - std::string password; - std::string server_address; - int port; - std::string transport_type; - bool secure; - }; - - std::set stun_servers; - - std::vector relays; - // Enable non-proxied UDP-based transport when set to true. When set to // false, it effectively disables all UDP traffic until UDP-supporting proxy // RETURN is available in the future. @@ -64,14 +44,7 @@ class P2PPortAllocator : public cricket::BasicPortAllocator { const scoped_refptr task_runner); ~P2PPortAllocator() override; - cricket::PortAllocatorSession* CreateSessionInternal( - const std::string& content_name, - int component, - const std::string& ice_username_fragment, - const std::string& ice_password) override; - private: - friend class P2PPortAllocatorSession; scoped_ptr network_manager_; scoped_refptr socket_dispatcher_; Config config_; @@ -85,26 +58,6 @@ class P2PPortAllocator : public cricket::BasicPortAllocator { DISALLOW_COPY_AND_ASSIGN(P2PPortAllocator); }; -class P2PPortAllocatorSession : public cricket::BasicPortAllocatorSession { - public: - P2PPortAllocatorSession( - P2PPortAllocator* allocator, - const std::string& content_name, - int component, - const std::string& ice_username_fragment, - const std::string& ice_password); - ~P2PPortAllocatorSession() override; - - protected: - // Overrides for cricket::BasicPortAllocatorSession. - void GetPortConfigurations() override; - - private: - P2PPortAllocator* allocator_; - - DISALLOW_COPY_AND_ASSIGN(P2PPortAllocatorSession); -}; - } // namespace content #endif // CONTENT_RENDERER_P2P_PORT_ALLOCATOR_H_ -- cgit v1.1