From 662e31ffecbc920667a9f56dc781c9689b64ca5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrik=20H=C3=B6glund?= Date: Thu, 5 Sep 2019 14:35:04 +0200 Subject: [PATCH] Prepare to move packet_socket_factory to api/. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I gave up on removing proxy_info, user_agent and tcp_options. I don't think it's feasible to remove them without removing all the proxy code. The assumption that you can set the proxy and user agent long after you have created the factory is entrenched in unit tests and the code itself. So is the ability to set tcp opts depending on protocol or endpoint properties. It may be easier to untangle proxy stuff from the factory later, when it becomes a more first-class citizen and isn't passed via the allocator. Requires https://chromium-review.googlesource.com/c/chromium/src/+/1778870 to land first. Bug: webrtc:7447 Change-Id: Ib496e2bb689ea415e9f8ec1dfedff13a83fa4a8a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150799 Commit-Queue: Patrik Höglund Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#29091} --- api/BUILD.gn | 16 +++++ api/DEPS | 5 ++ api/packet_socket_factory.h | 81 +++++++++++++++++++++++ api/peer_connection_interface.h | 5 ++ p2p/BUILD.gn | 3 +- p2p/base/basic_packet_socket_factory.cc | 19 ------ p2p/base/basic_packet_socket_factory.h | 10 +-- p2p/base/packet_socket_factory.cc | 50 -------------- p2p/base/packet_socket_factory.h | 86 ++----------------------- p2p/base/port_unittest.cc | 13 ++-- p2p/base/relay_port.cc | 4 +- p2p/base/tcp_port.cc | 4 +- pc/peer_connection_factory.cc | 16 ++++- test/pc/e2e/BUILD.gn | 1 + 14 files changed, 140 insertions(+), 173 deletions(-) create mode 100644 api/packet_socket_factory.h delete mode 100644 p2p/base/packet_socket_factory.cc diff --git a/api/BUILD.gn b/api/BUILD.gn index 46aea1d8eb..17f6c6cb97 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -135,6 +135,7 @@ rtc_static_library("libjingle_peerconnection_api") { "media_transport_interface.cc", "media_transport_interface.h", "notifier.h", + "packet_socket_factory.h", "peer_connection_factory_proxy.h", "peer_connection_interface.cc", "peer_connection_interface.h", @@ -165,6 +166,7 @@ rtc_static_library("libjingle_peerconnection_api") { ":fec_controller_api", ":libjingle_logging_api", ":network_state_predictor_api", + ":packet_socket_factory", ":rtc_stats_api", ":rtp_packet_info", ":rtp_parameters", @@ -202,6 +204,20 @@ rtc_static_library("libjingle_peerconnection_api") { ] } +rtc_source_set("packet_socket_factory") { + visibility = [ "*" ] + sources = [ + # TODO(bugs.webrtc.org/7447: remove .h files from the api target once + # downstream is updated to use the new target. + "async_resolver_factory.h", + "packet_socket_factory.h", + ] + deps = [ + "../rtc_base:rtc_base", + "../rtc_base/system:rtc_export", + ] +} + rtc_source_set("scoped_refptr") { visibility = [ "*" ] sources = [ diff --git a/api/DEPS b/api/DEPS index f7210d803b..f23b1b86b2 100644 --- a/api/DEPS +++ b/api/DEPS @@ -115,6 +115,11 @@ specific_include_rules = { "+rtc_base/network_route.h", ], + "packet_socket_factory\.h": [ + "+rtc_base/proxy_info.h", + "+rtc_base/async_packet_socket.h", + ], + "peer_connection_factory_proxy\.h": [ "+rtc_base/bind.h", ], diff --git a/api/packet_socket_factory.h b/api/packet_socket_factory.h new file mode 100644 index 0000000000..1e9f470357 --- /dev/null +++ b/api/packet_socket_factory.h @@ -0,0 +1,81 @@ +/* + * Copyright 2019 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef API_PACKET_SOCKET_FACTORY_H_ +#define API_PACKET_SOCKET_FACTORY_H_ + +#include +#include + +#include "rtc_base/async_packet_socket.h" +#include "rtc_base/proxy_info.h" +#include "rtc_base/system/rtc_export.h" + +namespace rtc { + +class SSLCertificateVerifier; +class AsyncResolverInterface; + +struct PacketSocketTcpOptions { + PacketSocketTcpOptions() = default; + ~PacketSocketTcpOptions() = default; + + int opts = 0; + std::vector tls_alpn_protocols; + std::vector tls_elliptic_curves; + // An optional custom SSL certificate verifier that an API user can provide to + // inject their own certificate verification logic (not available to users + // outside of the WebRTC repo). + SSLCertificateVerifier* tls_cert_verifier = nullptr; +}; + +class RTC_EXPORT PacketSocketFactory { + public: + enum Options { + OPT_STUN = 0x04, + + // The TLS options below are mutually exclusive. + OPT_TLS = 0x02, // Real and secure TLS. + OPT_TLS_FAKE = 0x01, // Fake TLS with a dummy SSL handshake. + OPT_TLS_INSECURE = 0x08, // Insecure TLS without certificate validation. + + // Deprecated, use OPT_TLS_FAKE. + OPT_SSLTCP = OPT_TLS_FAKE, + }; + + PacketSocketFactory() = default; + virtual ~PacketSocketFactory() = default; + + virtual AsyncPacketSocket* CreateUdpSocket(const SocketAddress& address, + uint16_t min_port, + uint16_t max_port) = 0; + virtual AsyncPacketSocket* CreateServerTcpSocket( + const SocketAddress& local_address, + uint16_t min_port, + uint16_t max_port, + int opts) = 0; + + virtual AsyncPacketSocket* CreateClientTcpSocket( + const SocketAddress& local_address, + const SocketAddress& remote_address, + const ProxyInfo& proxy_info, + const std::string& user_agent, + const PacketSocketTcpOptions& tcp_options) = 0; + + virtual AsyncResolverInterface* CreateAsyncResolver() = 0; + + private: + PacketSocketFactory(const PacketSocketFactory&) = delete; + PacketSocketFactory& operator=(const PacketSocketFactory&) = delete; +}; + +} // namespace rtc + +#endif // API_PACKET_SOCKET_FACTORY_H_ diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 124d12ad8c..afa771fa79 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -86,6 +86,7 @@ #include "api/media_stream_interface.h" #include "api/media_transport_interface.h" #include "api/network_state_predictor.h" +#include "api/packet_socket_factory.h" #include "api/rtc_error.h" #include "api/rtc_event_log/rtc_event_log_factory_interface.h" #include "api/rtc_event_log_output.h" @@ -1255,7 +1256,11 @@ struct PeerConnectionDependencies final { // Mandatory dependencies PeerConnectionObserver* observer = nullptr; // Optional dependencies + // TODO(bugs.webrtc.org/7447): remove port allocator once downstream is + // updated. For now, you can only set one of allocator and + // packet_socket_factory, not both. std::unique_ptr allocator; + std::unique_ptr packet_socket_factory; std::unique_ptr async_resolver_factory; std::unique_ptr cert_generator; std::unique_ptr tls_cert_verifier; diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 5b9a5d5ac2..bc80b7501b 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -50,7 +50,6 @@ rtc_static_library("rtc_p2p") { "base/p2p_constants.h", "base/p2p_transport_channel.cc", "base/p2p_transport_channel.h", - "base/packet_socket_factory.cc", "base/packet_socket_factory.h", "base/packet_transport_interface.h", "base/packet_transport_internal.cc", @@ -93,6 +92,7 @@ rtc_static_library("rtc_p2p") { deps = [ "../api:libjingle_peerconnection_api", + "../api:packet_socket_factory", "../api:scoped_refptr", "../api/rtc_event_log", "../api/transport:enums", @@ -164,6 +164,7 @@ if (rtc_include_tests) { ":p2p_server_utils", ":rtc_p2p", "../api:libjingle_peerconnection_api", + "../api:packet_socket_factory", "../rtc_base", "../rtc_base:gunit_helpers", "../rtc_base:rtc_base_approved", diff --git a/p2p/base/basic_packet_socket_factory.cc b/p2p/base/basic_packet_socket_factory.cc index 3204092412..1476939a3d 100644 --- a/p2p/base/basic_packet_socket_factory.cc +++ b/p2p/base/basic_packet_socket_factory.cc @@ -188,25 +188,6 @@ AsyncPacketSocket* BasicPacketSocketFactory::CreateClientTcpSocket( return tcp_socket; } -AsyncPacketSocket* BasicPacketSocketFactory::CreateClientTcpSocket( - const SocketAddress& local_address, - const SocketAddress& remote_address, - const ProxyInfo& proxy_info, - const std::string& user_agent, - int opts) { - PacketSocketTcpOptions tcp_options; - tcp_options.opts = opts; - return CreateClientTcpSocket(local_address, remote_address, proxy_info, - user_agent, tcp_options); -} - -AsyncPacketSocket* BasicPacketSocketFactory::CreateClientTcpSocket( - const SocketAddress& local_address, - const SocketAddress& remote_address) { - return CreateClientTcpSocket(local_address, remote_address, ProxyInfo(), "", - PacketSocketTcpOptions()); -} - AsyncResolverInterface* BasicPacketSocketFactory::CreateAsyncResolver() { return new AsyncResolver(); } diff --git a/p2p/base/basic_packet_socket_factory.h b/p2p/base/basic_packet_socket_factory.h index ba6c59ddb5..337efca843 100644 --- a/p2p/base/basic_packet_socket_factory.h +++ b/p2p/base/basic_packet_socket_factory.h @@ -13,7 +13,7 @@ #include -#include "p2p/base/packet_socket_factory.h" +#include "api/packet_socket_factory.h" namespace rtc { @@ -35,14 +35,6 @@ class BasicPacketSocketFactory : public PacketSocketFactory { uint16_t min_port, uint16_t max_port, int opts) override; - AsyncPacketSocket* CreateClientTcpSocket( - const SocketAddress& local_address, - const SocketAddress& remote_address) override; - AsyncPacketSocket* CreateClientTcpSocket(const SocketAddress& local_address, - const SocketAddress& remote_address, - const ProxyInfo& proxy_info, - const std::string& user_agent, - int opts) override; AsyncPacketSocket* CreateClientTcpSocket( const SocketAddress& local_address, const SocketAddress& remote_address, diff --git a/p2p/base/packet_socket_factory.cc b/p2p/base/packet_socket_factory.cc deleted file mode 100644 index 403dc26e99..0000000000 --- a/p2p/base/packet_socket_factory.cc +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright 2017 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "p2p/base/packet_socket_factory.h" - -#include - -#include "rtc_base/checks.h" - -namespace rtc { - -PacketSocketTcpOptions::PacketSocketTcpOptions() = default; - -PacketSocketTcpOptions::~PacketSocketTcpOptions() = default; - -AsyncPacketSocket* PacketSocketFactory::CreateClientTcpSocket( - const SocketAddress& local_address, - const SocketAddress& remote_address, - const ProxyInfo& proxy_info, - const std::string& user_agent, - const PacketSocketTcpOptions& tcp_options) { - return CreateClientTcpSocket(local_address, remote_address, proxy_info, - user_agent, tcp_options.opts); -} - -AsyncPacketSocket* PacketSocketFactory::CreateClientTcpSocket( - const SocketAddress& local_address, - const SocketAddress& remote_address, - const ProxyInfo& proxy_info, - const std::string& user_agent, - int opts) { - RTC_NOTREACHED(); - return nullptr; -} - -AsyncPacketSocket* PacketSocketFactory::CreateClientTcpSocket( - const SocketAddress& local_address, - const SocketAddress& remote_address) { - RTC_NOTREACHED(); - return nullptr; -} - -} // namespace rtc diff --git a/p2p/base/packet_socket_factory.h b/p2p/base/packet_socket_factory.h index 5c90d6da47..139a7782b4 100644 --- a/p2p/base/packet_socket_factory.h +++ b/p2p/base/packet_socket_factory.h @@ -8,90 +8,12 @@ * be found in the AUTHORS file in the root of the source tree. */ +// TODO(bugs.webrtc.org/7447): Remove this file once downstream points to the +// new location in api/. + #ifndef P2P_BASE_PACKET_SOCKET_FACTORY_H_ #define P2P_BASE_PACKET_SOCKET_FACTORY_H_ -#include -#include - -#include "rtc_base/proxy_info.h" -#include "rtc_base/system/rtc_export.h" - -namespace rtc { - -class SSLCertificateVerifier; -class AsyncPacketSocket; -class AsyncResolverInterface; - -// TODO(bugs.webrtc.org/7447): move this to basic_packet_socket_factory. -struct PacketSocketTcpOptions { - PacketSocketTcpOptions(); - ~PacketSocketTcpOptions(); - - int opts = 0; - std::vector tls_alpn_protocols; - std::vector tls_elliptic_curves; - // An optional custom SSL certificate verifier that an API user can provide to - // inject their own certificate verification logic (not available to users - // outside of the WebRTC repo). - SSLCertificateVerifier* tls_cert_verifier = nullptr; -}; - -class RTC_EXPORT PacketSocketFactory { - public: - enum Options { - OPT_STUN = 0x04, - - // The TLS options below are mutually exclusive. - OPT_TLS = 0x02, // Real and secure TLS. - OPT_TLS_FAKE = 0x01, // Fake TLS with a dummy SSL handshake. - OPT_TLS_INSECURE = 0x08, // Insecure TLS without certificate validation. - - // Deprecated, use OPT_TLS_FAKE. - OPT_SSLTCP = OPT_TLS_FAKE, - }; - - PacketSocketFactory() {} - virtual ~PacketSocketFactory() = default; - - virtual AsyncPacketSocket* CreateUdpSocket(const SocketAddress& address, - uint16_t min_port, - uint16_t max_port) = 0; - virtual AsyncPacketSocket* CreateServerTcpSocket( - const SocketAddress& local_address, - uint16_t min_port, - uint16_t max_port, - int opts) = 0; - - // TODO(bugs.webrtc.org/7447): This should be the only CreateClientTcpSocket - // implementation left; the two other are deprecated. - virtual AsyncPacketSocket* CreateClientTcpSocket( - const SocketAddress& local_address, - const SocketAddress& remote_address); - - // TODO(bugs.webrtc.org/7447): Deprecated, about to be removed. - virtual AsyncPacketSocket* CreateClientTcpSocket( - const SocketAddress& local_address, - const SocketAddress& remote_address, - const ProxyInfo& proxy_info, - const std::string& user_agent, - int opts); - - // TODO(bugs.webrtc.org/7447): Deprecated, about to be removed. - virtual AsyncPacketSocket* CreateClientTcpSocket( - const SocketAddress& local_address, - const SocketAddress& remote_address, - const ProxyInfo& proxy_info, - const std::string& user_agent, - const PacketSocketTcpOptions& tcp_options); - - virtual AsyncResolverInterface* CreateAsyncResolver() = 0; - - private: - PacketSocketFactory(const PacketSocketFactory&) = delete; - PacketSocketFactory& operator=(const PacketSocketFactory&) = delete; -}; - -} // namespace rtc +#include "api/packet_socket_factory.h" #endif // P2P_BASE_PACKET_SOCKET_FACTORY_H_ diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc index 4e32867f36..bef8426a43 100644 --- a/p2p/base/port_unittest.cc +++ b/p2p/base/port_unittest.cc @@ -1034,13 +1034,12 @@ class FakePacketSocketFactory : public rtc::PacketSocketFactory { return result; } - // TODO(?): |proxy_info| and |user_agent| should be set - // per-factory and not when socket is created. - AsyncPacketSocket* CreateClientTcpSocket(const SocketAddress& local_address, - const SocketAddress& remote_address, - const rtc::ProxyInfo& proxy_info, - const std::string& user_agent, - int opts) override { + AsyncPacketSocket* CreateClientTcpSocket( + const SocketAddress& local_address, + const SocketAddress& remote_address, + const rtc::ProxyInfo& proxy_info, + const std::string& user_agent, + const rtc::PacketSocketTcpOptions& opts) override { EXPECT_TRUE(next_client_tcp_socket_ != NULL); AsyncPacketSocket* result = next_client_tcp_socket_; next_client_tcp_socket_ = NULL; diff --git a/p2p/base/relay_port.cc b/p2p/base/relay_port.cc index 662a44b525..bb62ebb90f 100644 --- a/p2p/base/relay_port.cc +++ b/p2p/base/relay_port.cc @@ -519,9 +519,11 @@ void RelayEntry::Connect() { int opts = (ra->proto == PROTO_SSLTCP) ? rtc::PacketSocketFactory::OPT_TLS_FAKE : 0; + rtc::PacketSocketTcpOptions tcp_opts; + tcp_opts.opts = opts; socket = port_->socket_factory()->CreateClientTcpSocket( rtc::SocketAddress(port_->Network()->GetBestIP(), 0), ra->address, - port_->proxy(), port_->user_agent(), opts); + port_->proxy(), port_->user_agent(), tcp_opts); } else { RTC_LOG(LS_WARNING) << "Unknown protocol: " << ra->proto; } diff --git a/p2p/base/tcp_port.cc b/p2p/base/tcp_port.cc index 2cc2c945cf..91b6e1468a 100644 --- a/p2p/base/tcp_port.cc +++ b/p2p/base/tcp_port.cc @@ -550,10 +550,12 @@ void TCPConnection::CreateOutgoingTcpSocket() { int opts = (remote_candidate().protocol() == SSLTCP_PROTOCOL_NAME) ? rtc::PacketSocketFactory::OPT_TLS_FAKE : 0; + rtc::PacketSocketTcpOptions tcp_opts; + tcp_opts.opts = opts; socket_.reset(port()->socket_factory()->CreateClientTcpSocket( rtc::SocketAddress(port()->Network()->GetBestIP(), 0), remote_candidate().address(), port()->proxy(), port()->user_agent(), - opts)); + tcp_opts)); if (socket_) { RTC_LOG(LS_VERBOSE) << ToString() << ": Connecting from " << socket_->GetLocalAddress().ToSensitiveString() diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index 08ef0d45cc..1052b3b9eb 100644 --- a/pc/peer_connection_factory.cc +++ b/pc/peer_connection_factory.cc @@ -229,7 +229,7 @@ PeerConnectionFactory::CreatePeerConnection( std::unique_ptr allocator, std::unique_ptr cert_generator, PeerConnectionObserver* observer) { - // Convert the legacy API into the new depnedency structure. + // Convert the legacy API into the new dependency structure. PeerConnectionDependencies dependencies(observer); dependencies.allocator = std::move(allocator); dependencies.cert_generator = std::move(cert_generator); @@ -242,6 +242,9 @@ PeerConnectionFactory::CreatePeerConnection( const PeerConnectionInterface::RTCConfiguration& configuration, PeerConnectionDependencies dependencies) { RTC_DCHECK(signaling_thread_->IsCurrent()); + RTC_DCHECK(!(dependencies.allocator && dependencies.packet_socket_factory)) + << "You can't set both allocator and packet_socket_factory; " + "the former is going away (see bugs.webrtc.org/7447"; // Set internal defaults if optional dependencies are not set. if (!dependencies.cert_generator) { @@ -250,10 +253,17 @@ PeerConnectionFactory::CreatePeerConnection( network_thread_); } if (!dependencies.allocator) { + rtc::PacketSocketFactory* packet_socket_factory; + if (dependencies.packet_socket_factory) + packet_socket_factory = dependencies.packet_socket_factory.get(); + else + packet_socket_factory = default_socket_factory_.get(); + network_thread_->Invoke(RTC_FROM_HERE, [this, &configuration, - &dependencies]() { + &dependencies, + &packet_socket_factory]() { dependencies.allocator = absl::make_unique( - default_network_manager_.get(), default_socket_factory_.get(), + default_network_manager_.get(), packet_socket_factory, configuration.turn_customizer); }); } diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 9c497810d7..afd343a26a 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -52,6 +52,7 @@ rtc_source_set("peer_connection_quality_test_params") { "../../../api:callfactory_api", "../../../api:fec_controller_api", "../../../api:libjingle_peerconnection_api", + "../../../api:packet_socket_factory", "../../../api:peer_connection_quality_test_fixture_api", "../../../api/rtc_event_log", "../../../api/task_queue",