Deprecate PeerConnectionFactory::CreatePeerConnection

Applications should use CreatePeerConnectionOrError instead.

Moved fallback implementations of CreatePeerConnection into the
api/peer_connection_interface.h file, so that we do not have to
declare these methods in the proxy.

Bug: webrtc:12238
Change-Id: I70c56336641c2a108b68446ae41f43409277a584
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217762
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33964}

api/BUILD.gn

@@ -197,6 +197,7 @@ rtc_library("libjingle_peerconnection_api") {
     "units:data_rate",
     "units:timestamp",
     "video:encoded_image",
+    "video:video_bitrate_allocator_factory",
     "video:video_frame",
     "video:video_rtp_headers",
 

api/DEPS

@@ -128,13 +128,18 @@ specific_include_rules = {
   "peer_connection_interface\.h": [
     "+media/base/media_config.h",
     "+media/base/media_engine.h",
+    "+p2p/base/port.h",
     "+p2p/base/port_allocator.h",
+    "+rtc_base/network.h",
+    "+rtc_base/network_constants.h",
     "+rtc_base/network_monitor_factory.h",
+    "+rtc_base/ref_count.h",
     "+rtc_base/rtc_certificate.h",
     "+rtc_base/rtc_certificate_generator.h",
     "+rtc_base/socket_address.h",
     "+rtc_base/ssl_certificate.h",
     "+rtc_base/ssl_stream_adapter.h",
+    "+rtc_base/thread.h",
   ],
 
   "proxy\.h": [

api/peer_connection_factory_proxy.h

@@ -25,16 +25,6 @@ namespace webrtc {
 BEGIN_PROXY_MAP(PeerConnectionFactory)
 PROXY_PRIMARY_THREAD_DESTRUCTOR()
 PROXY_METHOD1(void, SetOptions, const Options&)
-PROXY_METHOD4(rtc::scoped_refptr<PeerConnectionInterface>,
-              CreatePeerConnection,
-              const PeerConnectionInterface::RTCConfiguration&,
-              std::unique_ptr<cricket::PortAllocator>,
-              std::unique_ptr<rtc::RTCCertificateGeneratorInterface>,
-              PeerConnectionObserver*)
-PROXY_METHOD2(rtc::scoped_refptr<PeerConnectionInterface>,
-              CreatePeerConnection,
-              const PeerConnectionInterface::RTCConfiguration&,
-              PeerConnectionDependencies)
 PROXY_METHOD2(RTCErrorOr<rtc::scoped_refptr<PeerConnectionInterface>>,
               CreatePeerConnectionOrError,
               const PeerConnectionInterface::RTCConfiguration&,

api/peer_connection_interface.cc

@@ -10,8 +10,7 @@
 
 #include "api/peer_connection_interface.h"
 
-#include "api/dtls_transport_interface.h"
-#include "api/sctp_transport_interface.h"
+#include <utility>
 
 namespace webrtc {
 
@@ -77,14 +76,27 @@ PeerConnectionFactoryInterface::CreatePeerConnection(
     std::unique_ptr<cricket::PortAllocator> allocator,
     std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
     PeerConnectionObserver* observer) {
-  return nullptr;
+  PeerConnectionDependencies dependencies(observer);
+  dependencies.allocator = std::move(allocator);
+  dependencies.cert_generator = std::move(cert_generator);
+  auto result =
+      CreatePeerConnectionOrError(configuration, std::move(dependencies));
+  if (!result.ok()) {
+    return nullptr;
+  }
+  return result.MoveValue();
 }
 
 rtc::scoped_refptr<PeerConnectionInterface>
 PeerConnectionFactoryInterface::CreatePeerConnection(
     const PeerConnectionInterface::RTCConfiguration& configuration,
     PeerConnectionDependencies dependencies) {
-  return nullptr;
+  auto result =
+      CreatePeerConnectionOrError(configuration, std::move(dependencies));
+  if (!result.ok()) {
+    return nullptr;
+  }
+  return result.MoveValue();
 }
 
 RTCErrorOr<rtc::scoped_refptr<PeerConnectionInterface>>

api/peer_connection_interface.h

@@ -67,12 +67,16 @@
 #ifndef API_PEER_CONNECTION_INTERFACE_H_
 #define API_PEER_CONNECTION_INTERFACE_H_
 
+#include <stdint.h>
 #include <stdio.h>
 
+#include <functional>
 #include <memory>
 #include <string>
 #include <vector>
 
+#include "absl/base/attributes.h"
+#include "absl/types/optional.h"
 #include "api/adaptation/resource.h"
 #include "api/async_dns_resolver.h"
 #include "api/async_resolver_factory.h"
@@ -81,6 +85,7 @@
 #include "api/audio_codecs/audio_encoder_factory.h"
 #include "api/audio_options.h"
 #include "api/call/call_factory_interface.h"
+#include "api/candidate.h"
 #include "api/crypto/crypto_options.h"
 #include "api/data_channel_interface.h"
 #include "api/dtls_transport_interface.h"
@@ -88,15 +93,18 @@
 #include "api/ice_transport_interface.h"
 #include "api/jsep.h"
 #include "api/media_stream_interface.h"
+#include "api/media_types.h"
 #include "api/neteq/neteq_factory.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"
+#include "api/rtp_parameters.h"
 #include "api/rtp_receiver_interface.h"
 #include "api/rtp_sender_interface.h"
 #include "api/rtp_transceiver_interface.h"
+#include "api/scoped_refptr.h"
 #include "api/sctp_transport_interface.h"
 #include "api/set_local_description_observer_interface.h"
 #include "api/set_remote_description_observer_interface.h"
@@ -109,19 +117,25 @@
 #include "api/transport/sctp_transport_factory_interface.h"
 #include "api/transport/webrtc_key_value_config.h"
 #include "api/turn_customizer.h"
+#include "api/video/video_bitrate_allocator_factory.h"
 #include "media/base/media_config.h"
 #include "media/base/media_engine.h"
 // TODO(bugs.webrtc.org/7447): We plan to provide a way to let applications
 // inject a PacketSocketFactory and/or NetworkManager, and not expose
-// PortAllocator in the PeerConnection api.
+// PortAllocator in the PeerConnection api. This will let us remove nogncheck.
+#include "p2p/base/port.h"            // nogncheck
 #include "p2p/base/port_allocator.h"  // nogncheck
+#include "rtc_base/network.h"
+#include "rtc_base/network_constants.h"
 #include "rtc_base/network_monitor_factory.h"
+#include "rtc_base/ref_count.h"
 #include "rtc_base/rtc_certificate.h"
 #include "rtc_base/rtc_certificate_generator.h"
 #include "rtc_base/socket_address.h"
 #include "rtc_base/ssl_certificate.h"
 #include "rtc_base/ssl_stream_adapter.h"
 #include "rtc_base/system/rtc_export.h"
+#include "rtc_base/thread.h"
 
 namespace rtc {
 class Thread;
@@ -1432,6 +1446,7 @@ class RTC_EXPORT PeerConnectionFactoryInterface
       PeerConnectionDependencies dependencies);
   // Deprecated creator - does not return an error code on error.
   // TODO(bugs.webrtc.org:12238): Deprecate and remove.
+  ABSL_DEPRECATED("Use CreatePeerConnectionOrError")
   virtual rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
       const PeerConnectionInterface::RTCConfiguration& configuration,
       PeerConnectionDependencies dependencies);
@@ -1445,6 +1460,7 @@ class RTC_EXPORT PeerConnectionFactoryInterface
   // responsibility of the caller to delete it. It can be safely deleted after
   // Close has been called on the returned PeerConnection, which ensures no
   // more observer callbacks will be invoked.
+  ABSL_DEPRECATED("Use CreatePeerConnectionOrError")
   virtual rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
       const PeerConnectionInterface::RTCConfiguration& configuration,
       std::unique_ptr<cricket::PortAllocator> allocator,

examples/androidnativeapi/jni/android_call_client.cc

@@ -179,9 +179,9 @@ void AndroidCallClient::CreatePeerConnection() {
   config.sdp_semantics = webrtc::SdpSemantics::kUnifiedPlan;
   // DTLS SRTP has to be disabled for loopback to work.
   config.enable_dtls_srtp = false;
-  pc_ = pcf_->CreatePeerConnection(config, nullptr /* port_allocator */,
-                                   nullptr /* cert_generator */,
-                                   pc_observer_.get());
+  webrtc::PeerConnectionDependencies deps(pc_observer_.get());
+  pc_ = pcf_->CreatePeerConnectionOrError(config, std::move(deps)).MoveValue();
+
   RTC_LOG(LS_INFO) << "PeerConnection created: " << pc_;
 
   rtc::scoped_refptr<webrtc::VideoTrackInterface> local_video_track =

examples/objcnativeapi/objc/objc_call_client.mm

@@ -144,7 +144,7 @@ void ObjCCallClient::CreatePeerConnection() {
   // DTLS SRTP has to be disabled for loopback to work.
   config.enable_dtls_srtp = false;
   webrtc::PeerConnectionDependencies pc_dependencies(pc_observer_.get());
-  pc_ = pcf_->CreatePeerConnection(config, std::move(pc_dependencies));
+  pc_ = pcf_->CreatePeerConnectionOrError(config, std::move(pc_dependencies)).MoveValue();
   RTC_LOG(LS_INFO) << "PeerConnection created: " << pc_;
 
   rtc::scoped_refptr<webrtc::VideoTrackInterface> local_video_track =

examples/unityplugin/simple_peer_connection.cc

@@ -192,10 +192,14 @@ bool SimplePeerConnection::CreatePeerConnection(const char** turn_urls,
   config_.servers.push_back(stun_server);
   config_.enable_dtls_srtp = false;
 
-  peer_connection_ = g_peer_connection_factory->CreatePeerConnection(
-      config_, nullptr, nullptr, this);
-
-  return peer_connection_.get() != nullptr;
+  auto result = g_peer_connection_factory->CreatePeerConnectionOrError(
+      config_, webrtc::PeerConnectionDependencies(this));
+  if (!result.ok()) {
+    peer_connection_ = nullptr;
+    return false;
+  }
+  peer_connection_ = result.MoveValue();
+  return true;
 }
 
 void SimplePeerConnection::DeletePeerConnection() {

pc/peer_connection_factory.cc

@@ -33,6 +33,7 @@
 #include "p2p/base/basic_async_resolver_factory.h"
 #include "p2p/base/basic_packet_socket_factory.h"
 #include "p2p/base/default_ice_transport_factory.h"
+#include "p2p/base/port_allocator.h"
 #include "p2p/client/basic_port_allocator.h"
 #include "pc/audio_track.h"
 #include "pc/local_audio_source.h"
@@ -48,6 +49,7 @@
 #include "rtc_base/logging.h"
 #include "rtc_base/numerics/safe_conversions.h"
 #include "rtc_base/ref_counted_object.h"
+#include "rtc_base/rtc_certificate_generator.h"
 #include "rtc_base/system/file_wrapper.h"
 
 namespace webrtc {
@@ -184,33 +186,6 @@ void PeerConnectionFactory::StopAecDump() {
   channel_manager()->StopAecDump();
 }
 
-rtc::scoped_refptr<PeerConnectionInterface>
-PeerConnectionFactory::CreatePeerConnection(
-    const PeerConnectionInterface::RTCConfiguration& configuration,
-    std::unique_ptr<cricket::PortAllocator> allocator,
-    std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
-    PeerConnectionObserver* observer) {
-  // Convert the legacy API into the new dependency structure.
-  PeerConnectionDependencies dependencies(observer);
-  dependencies.allocator = std::move(allocator);
-  dependencies.cert_generator = std::move(cert_generator);
-  // Pass that into the new API.
-  return CreatePeerConnection(configuration, std::move(dependencies));
-}
-
-rtc::scoped_refptr<PeerConnectionInterface>
-PeerConnectionFactory::CreatePeerConnection(
-    const PeerConnectionInterface::RTCConfiguration& configuration,
-    PeerConnectionDependencies dependencies) {
-  auto result =
-      CreatePeerConnectionOrError(configuration, std::move(dependencies));
-  if (result.ok()) {
-    return result.MoveValue();
-  } else {
-    return nullptr;
-  }
-}
-
 RTCErrorOr<rtc::scoped_refptr<PeerConnectionInterface>>
 PeerConnectionFactory::CreatePeerConnectionOrError(
     const PeerConnectionInterface::RTCConfiguration& configuration,

pc/peer_connection_factory.h

@@ -14,7 +14,6 @@
 
 #include <stdint.h>
 #include <stdio.h>
-
 #include <memory>
 #include <string>
 
@@ -66,16 +65,6 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface {
 
   void SetOptions(const Options& options) override;
 
-  rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
-      const PeerConnectionInterface::RTCConfiguration& configuration,
-      std::unique_ptr<cricket::PortAllocator> allocator,
-      std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
-      PeerConnectionObserver* observer) override;
-
-  rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
-      const PeerConnectionInterface::RTCConfiguration& configuration,
-      PeerConnectionDependencies dependencies) override;
-
   RTCErrorOr<rtc::scoped_refptr<PeerConnectionInterface>>
   CreatePeerConnectionOrError(
       const PeerConnectionInterface::RTCConfiguration& configuration,

pc/peer_connection_rampup_tests.cc

@@ -192,14 +192,14 @@ class PeerConnectionRampUpTest : public ::testing::Test {
     dependencies.tls_cert_verifier =
         std::make_unique<rtc::TestCertificateVerifier>();
 
-    auto pc =
-        pc_factory_->CreatePeerConnection(config, std::move(dependencies));
-    if (!pc) {
+    auto result = pc_factory_->CreatePeerConnectionOrError(
+        config, std::move(dependencies));
+    if (!result.ok()) {
       return nullptr;
     }
 
     return std::make_unique<PeerConnectionWrapperForRampUpTest>(
-        pc_factory_, pc, std::move(observer));
+        pc_factory_, result.MoveValue(), std::move(observer));
   }
 
   void SetupOneWayCall() {

pc/test/peer_connection_test_wrapper.cc

@@ -123,10 +123,17 @@ bool PeerConnectionTestWrapper::CreatePc(
 
   std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator(
       new FakeRTCCertificateGenerator());
-  peer_connection_ = peer_connection_factory_->CreatePeerConnection(
-      config, std::move(port_allocator), std::move(cert_generator), this);
-
-  return peer_connection_.get() != NULL;
+  webrtc::PeerConnectionDependencies deps(this);
+  deps.allocator = std::move(port_allocator);
+  deps.cert_generator = std::move(cert_generator);
+  auto result = peer_connection_factory_->CreatePeerConnectionOrError(
+      config, std::move(deps));
+  if (result.ok()) {
+    peer_connection_ = result.MoveValue();
+    return true;
+  } else {
+    return false;
+  }
 }
 
 rtc::scoped_refptr<webrtc::DataChannelInterface>

sdk/android/src/jni/pc/peer_connection_factory.cc

@@ -471,14 +471,14 @@ static jlong JNI_PeerConnectionFactory_CreatePeerConnection(
             jni, j_sslCertificateVerifier);
   }
 
-  rtc::scoped_refptr<PeerConnectionInterface> pc =
-      PeerConnectionFactoryFromJava(factory)->CreatePeerConnection(
+  auto result =
+      PeerConnectionFactoryFromJava(factory)->CreatePeerConnectionOrError(
           rtc_config, std::move(peer_connection_dependencies));
-  if (!pc)
+  if (!result.ok())
     return 0;
 
-  return jlongFromPointer(
-      new OwnedPeerConnection(pc, std::move(observer), std::move(constraints)));
+  return jlongFromPointer(new OwnedPeerConnection(
+      result.MoveValue(), std::move(observer), std::move(constraints)));
 }
 
 static jlong JNI_PeerConnectionFactory_CreateVideoSource(

sdk/objc/api/peerconnection/RTCPeerConnection.mm

@@ -348,11 +348,12 @@ void PeerConnectionDelegateAdapter::OnRemoveTrack(
 
     webrtc::PeerConnectionDependencies deps = std::move(*dependencies.release());
     deps.observer = _observer.get();
-    _peerConnection = factory.nativeFactory->CreatePeerConnection(*config, std::move(deps));
+    auto result = factory.nativeFactory->CreatePeerConnectionOrError(*config, std::move(deps));
 
-    if (!_peerConnection) {
+    if (!result.ok()) {
       return nil;
     }
+    _peerConnection = result.MoveValue();
     _factory = factory;
     _localStreams = [[NSMutableArray alloc] init];
     _delegate = delegate;

test/network/network_emulation_pc_unittest.cc

@@ -99,7 +99,12 @@ rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
     rtc_configuration.servers.push_back(server);
   }
 
-  return pcf->CreatePeerConnection(rtc_configuration, std::move(pc_deps));
+  auto result =
+      pcf->CreatePeerConnectionOrError(rtc_configuration, std::move(pc_deps));
+  if (!result.ok()) {
+    return nullptr;
+  }
+  return result.MoveValue();
 }
 
 }  // namespace

test/pc/e2e/test_peer_factory.cc

@@ -348,8 +348,10 @@ std::unique_ptr<TestPeer> TestPeerFactory::CreateTestPeer(
   PeerConnectionDependencies pc_deps = CreatePCDependencies(
       observer.get(), std::move(components->pc_dependencies));
   rtc::scoped_refptr<PeerConnectionInterface> peer_connection =
-      peer_connection_factory->CreatePeerConnection(params->rtc_configuration,
-                                                    std::move(pc_deps));
+      peer_connection_factory
+          ->CreatePeerConnectionOrError(params->rtc_configuration,
+                                        std::move(pc_deps))
+          .MoveValue();
   peer_connection->SetBitrate(params->bitrate_settings);
 
   return absl::WrapUnique(new TestPeer(

test/peer_scenario/peer_scenario_client.cc

@@ -241,7 +241,9 @@ PeerScenarioClient::PeerScenarioClient(
   pc_deps.allocator->set_flags(pc_deps.allocator->flags() |
                                cricket::PORTALLOCATOR_DISABLE_TCP);
   peer_connection_ =
-      pc_factory_->CreatePeerConnection(config.rtc_config, std::move(pc_deps));
+      pc_factory_
+          ->CreatePeerConnectionOrError(config.rtc_config, std::move(pc_deps))
+          .MoveValue();
   if (log_writer_factory_) {
     peer_connection_->StartRtcEventLog(log_writer_factory_->Create(".rtc.dat"),
                                        /*output_period_ms=*/1000);

webrtc_lib_link_test.cc

@@ -65,9 +65,10 @@ void TestCase1ModularFactory() {
   auto peer_connection_factory =
       webrtc::CreateModularPeerConnectionFactory(std::move(pcf_deps));
   webrtc::PeerConnectionInterface::RTCConfiguration rtc_config;
-  auto peer_connection = peer_connection_factory->CreatePeerConnection(
-      rtc_config, nullptr, nullptr, nullptr);
-  printf("peer_connection=%s\n", peer_connection == nullptr ? "nullptr" : "ok");
+  auto result = peer_connection_factory->CreatePeerConnectionOrError(
+      rtc_config, PeerConnectionDependencies(nullptr));
+  // Creation will fail because of null observer, but that's OK.
+  printf("peer_connection creation=%s\n", result.ok() ? "succeeded" : "failed");
 }
 
 void TestCase2RegularFactory() {
@@ -81,9 +82,10 @@ void TestCase2RegularFactory() {
       std::move(media_deps.video_encoder_factory),
       std::move(media_deps.video_decoder_factory), nullptr, nullptr);
   webrtc::PeerConnectionInterface::RTCConfiguration rtc_config;
-  auto peer_connection = peer_connection_factory->CreatePeerConnection(
-      rtc_config, nullptr, nullptr, nullptr);
-  printf("peer_connection=%s\n", peer_connection == nullptr ? "nullptr" : "ok");
+  auto result = peer_connection_factory->CreatePeerConnectionOrError(
+      rtc_config, PeerConnectionDependencies(nullptr));
+  // Creation will fail because of null observer, but that's OK.
+  printf("peer_connection creation=%s\n", result.ok() ? "succeeded" : "failed");
 }
 
 }  // namespace webrtc