Prepare to move packet_socket_factory to api/.

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 <phoglund@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29091}

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 = [

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",
   ],

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 <string>
+#include <vector>
+
+#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<std::string> tls_alpn_protocols;
+  std::vector<std::string> 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_

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<cricket::PortAllocator> allocator;
+  std::unique_ptr<rtc::PacketSocketFactory> packet_socket_factory;
   std::unique_ptr<webrtc::AsyncResolverFactory> async_resolver_factory;
   std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator;
   std::unique_ptr<rtc::SSLCertificateVerifier> tls_cert_verifier;

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",

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();
 }

p2p/base/basic_packet_socket_factory.h

@@ -13,7 +13,7 @@
 
 #include <string>
 
-#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,

p2p/base/packet_socket_factory.cc

@@ -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 <string>
-
-#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

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 <string>
+#include "api/packet_socket_factory.h"
-#include <vector>
-
-#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<std::string> tls_alpn_protocols;
-  std::vector<std::string> 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
 
 #endif  // P2P_BASE_PACKET_SOCKET_FACTORY_H_

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
+  AsyncPacketSocket* CreateClientTcpSocket(
-  // per-factory and not when socket is created.
+      const SocketAddress& local_address,
-  AsyncPacketSocket* CreateClientTcpSocket(const SocketAddress& local_address,
+      const SocketAddress& remote_address,
-                                           const SocketAddress& remote_address,
+      const rtc::ProxyInfo& proxy_info,
-                                           const rtc::ProxyInfo& proxy_info,
+      const std::string& user_agent,
-                                           const std::string& user_agent,
+      const rtc::PacketSocketTcpOptions& opts) override {
-                                           int opts) override {
     EXPECT_TRUE(next_client_tcp_socket_ != NULL);
     AsyncPacketSocket* result = next_client_tcp_socket_;
     next_client_tcp_socket_ = NULL;

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;
   }

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()

pc/peer_connection_factory.cc

@@ -229,7 +229,7 @@ PeerConnectionFactory::CreatePeerConnection(
     std::unique_ptr<cricket::PortAllocator> allocator,
     std::unique_ptr<rtc::RTCCertificateGeneratorInterface> 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<void>(RTC_FROM_HERE, [this, &configuration,
-                                                  &dependencies]() {
+                                                  &dependencies,
+                                                  &packet_socket_factory]() {
       dependencies.allocator = absl::make_unique<cricket::BasicPortAllocator>(
-          default_network_manager_.get(), default_socket_factory_.get(),
+          default_network_manager_.get(), packet_socket_factory,
           configuration.turn_customizer);
     });
   }

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",