Delete the "initialize" method of PeerConnectionFactory
Also remove the "initialized" concept from ConnectionContext.
This CL also always creates the objects on the signaling thread.
Makes the initialization code slightly more readable.
Bug: webrtc:11967
Change-Id: I5e451a3c5225c29c30d32bb4843df8c107ec30c0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/188626
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32453}
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index 821802f..fbd0295 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -228,6 +228,7 @@
"../api:array_view",
"../api:audio_options_api",
"../api:call_api",
+ "../api:callfactory_api",
"../api:fec_controller_api",
"../api:frame_transformer_interface",
"../api:ice_transport_factory",
@@ -246,12 +247,15 @@
"../api/adaptation:resource_adaptation_api",
"../api/crypto:frame_decryptor_interface",
"../api/crypto:options",
+ "../api/neteq:neteq_api",
"../api/rtc_event_log",
"../api/task_queue",
"../api/transport:bitrate_settings",
"../api/transport:datagram_transport_interface",
"../api/transport:enums",
"../api/transport:field_trial_based_config",
+ "../api/transport:network_control",
+ "../api/transport:sctp_transport_factory_interface",
"../api/transport:webrtc_key_value_config",
"../api/units:data_rate",
"../api/video:builtin_video_bitrate_allocator_factory",
@@ -305,14 +309,19 @@
]
deps = [
":rtc_pc_base",
+ "../api:callfactory_api",
"../api:libjingle_peerconnection_api",
"../api:media_stream_interface",
"../api:scoped_refptr",
+ "../api/neteq:neteq_api",
"../api/transport:field_trial_based_config",
+ "../api/transport:sctp_transport_factory_interface",
+ "../api/transport:webrtc_key_value_config",
"../media:rtc_data",
"../media:rtc_media_base",
"../p2p:rtc_p2p",
"../rtc_base",
+ "../rtc_base:checks",
]
}
diff --git a/pc/connection_context.cc b/pc/connection_context.cc
index ea9fb72..7477d8e 100644
--- a/pc/connection_context.cc
+++ b/pc/connection_context.cc
@@ -10,10 +10,15 @@
#include "pc/connection_context.h"
+#include <string>
+#include <type_traits>
#include <utility>
#include "api/transport/field_trial_based_config.h"
#include "media/base/rtp_data_engine.h"
+#include "rtc_base/helpers.h"
+#include "rtc_base/ref_counted_object.h"
+#include "rtc_base/time_utils.h"
namespace webrtc {
@@ -67,29 +72,59 @@
} // namespace
+// Static
+rtc::scoped_refptr<ConnectionContext> ConnectionContext::Create(
+ PeerConnectionFactoryDependencies* dependencies) {
+ auto context = new rtc::RefCountedObject<ConnectionContext>(dependencies);
+ if (!context->channel_manager_->Init()) {
+ return nullptr;
+ }
+ return context;
+}
+
ConnectionContext::ConnectionContext(
- PeerConnectionFactoryDependencies& dependencies)
- : network_thread_(MaybeStartThread(dependencies.network_thread,
+ PeerConnectionFactoryDependencies* dependencies)
+ : network_thread_(MaybeStartThread(dependencies->network_thread,
"pc_network_thread",
true,
owned_network_thread_)),
- worker_thread_(MaybeStartThread(dependencies.worker_thread,
+ worker_thread_(MaybeStartThread(dependencies->worker_thread,
"pc_worker_thread",
false,
owned_worker_thread_)),
- signaling_thread_(MaybeWrapThread(dependencies.signaling_thread,
+ signaling_thread_(MaybeWrapThread(dependencies->signaling_thread,
wraps_current_thread_)),
- network_monitor_factory_(std::move(dependencies.network_monitor_factory)),
- call_factory_(std::move(dependencies.call_factory)),
- media_engine_(std::move(dependencies.media_engine)),
- sctp_factory_(MaybeCreateSctpFactory(std::move(dependencies.sctp_factory),
- network_thread())),
- trials_(dependencies.trials ? std::move(dependencies.trials)
- : std::make_unique<FieldTrialBasedConfig>()) {
+ network_monitor_factory_(
+ std::move(dependencies->network_monitor_factory)),
+ call_factory_(std::move(dependencies->call_factory)),
+ media_engine_(std::move(dependencies->media_engine)),
+ sctp_factory_(
+ MaybeCreateSctpFactory(std::move(dependencies->sctp_factory),
+ network_thread())),
+ trials_(dependencies->trials
+ ? std::move(dependencies->trials)
+ : std::make_unique<FieldTrialBasedConfig>()) {
signaling_thread_->AllowInvokesToThread(worker_thread_);
signaling_thread_->AllowInvokesToThread(network_thread_);
worker_thread_->AllowInvokesToThread(network_thread_);
network_thread_->DisallowAllInvokes();
+
+ RTC_DCHECK_RUN_ON(signaling_thread_);
+ rtc::InitRandom(rtc::Time32());
+
+ // If network_monitor_factory_ is non-null, it will be used to create a
+ // network monitor while on the network thread.
+ default_network_manager_ = std::make_unique<rtc::BasicNetworkManager>(
+ network_monitor_factory_.get());
+
+ default_socket_factory_ =
+ std::make_unique<rtc::BasicPacketSocketFactory>(network_thread());
+
+ channel_manager_ = std::make_unique<cricket::ChannelManager>(
+ std::move(media_engine_), std::make_unique<cricket::RtpDataEngine>(),
+ worker_thread(), network_thread());
+
+ channel_manager_->SetVideoRtxEnabled(true);
}
ConnectionContext::~ConnectionContext() {
@@ -111,32 +146,6 @@
options_ = options;
}
-bool ConnectionContext::Initialize() {
- RTC_DCHECK_RUN_ON(signaling_thread_);
- rtc::InitRandom(rtc::Time32());
-
- // If network_monitor_factory_ is non-null, it will be used to create a
- // network monitor while on the network thread.
- default_network_manager_.reset(
- new rtc::BasicNetworkManager(network_monitor_factory_.get()));
- if (!default_network_manager_) {
- return false;
- }
-
- default_socket_factory_.reset(
- new rtc::BasicPacketSocketFactory(network_thread()));
- if (!default_socket_factory_) {
- return false;
- }
-
- channel_manager_ = std::make_unique<cricket::ChannelManager>(
- std::move(media_engine_), std::make_unique<cricket::RtpDataEngine>(),
- worker_thread(), network_thread());
-
- channel_manager_->SetVideoRtxEnabled(true);
- return channel_manager_->Init();
-}
-
cricket::ChannelManager* ConnectionContext::channel_manager() const {
return channel_manager_.get();
}
diff --git a/pc/connection_context.h b/pc/connection_context.h
index 502ba5a..2f95869 100644
--- a/pc/connection_context.h
+++ b/pc/connection_context.h
@@ -14,14 +14,24 @@
#include <memory>
#include <string>
+#include "api/call/call_factory_interface.h"
#include "api/media_stream_interface.h"
#include "api/peer_connection_interface.h"
#include "api/scoped_refptr.h"
+#include "api/transport/sctp_transport_factory_interface.h"
+#include "api/transport/webrtc_key_value_config.h"
+#include "media/base/media_engine.h"
#include "media/sctp/sctp_transport_internal.h"
#include "p2p/base/basic_packet_socket_factory.h"
#include "pc/channel_manager.h"
+#include "rtc_base/checks.h"
+#include "rtc_base/network.h"
+#include "rtc_base/network_monitor_factory.h"
+#include "rtc_base/ref_count.h"
#include "rtc_base/rtc_certificate_generator.h"
+#include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/thread.h"
+#include "rtc_base/thread_annotations.h"
namespace rtc {
class BasicNetworkManager;
@@ -36,8 +46,16 @@
// objects. A reference to this object is passed to each PeerConnection. The
// methods on this object are assumed not to change the state in any way that
// interferes with the operation of other PeerConnections.
+//
+// This class must be created and destroyed on the signaling thread.
class ConnectionContext : public rtc::RefCountInterface {
public:
+ // Creates a ConnectionContext. May return null if initialization fails.
+ // The Dependencies class allows simple management of all new dependencies
+ // being added to the ConnectionContext.
+ static rtc::scoped_refptr<ConnectionContext> Create(
+ PeerConnectionFactoryDependencies* dependencies);
+
// This class is not copyable or movable.
ConnectionContext(const ConnectionContext&) = delete;
ConnectionContext& operator=(const ConnectionContext&) = delete;
@@ -45,8 +63,6 @@
// Functions called from PeerConnectionFactory
void SetOptions(const PeerConnectionFactoryInterface::Options& options);
- bool Initialize();
-
// Functions called from PeerConnection and friends
SctpTransportFactoryInterface* sctp_transport_factory() const {
RTC_DCHECK_RUN_ON(signaling_thread_);
@@ -83,9 +99,7 @@
}
protected:
- // The Dependencies class allows simple management of all new dependencies
- // being added to the ConnectionContext.
- explicit ConnectionContext(PeerConnectionFactoryDependencies& dependencies);
+ explicit ConnectionContext(PeerConnectionFactoryDependencies* dependencies);
virtual ~ConnectionContext();
diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc
index 9e7c17c..ba81c29 100644
--- a/pc/peer_connection_factory.cc
+++ b/pc/peer_connection_factory.cc
@@ -12,23 +12,25 @@
#include <cstdio>
#include <memory>
+#include <type_traits>
#include <utility>
-#include <vector>
#include "absl/strings/match.h"
+#include "api/async_resolver_factory.h"
+#include "api/call/call_factory_interface.h"
#include "api/fec_controller.h"
+#include "api/ice_transport_interface.h"
#include "api/media_stream_proxy.h"
#include "api/media_stream_track_proxy.h"
#include "api/network_state_predictor.h"
+#include "api/packet_socket_factory.h"
#include "api/peer_connection_factory_proxy.h"
#include "api/peer_connection_proxy.h"
#include "api/rtc_event_log/rtc_event_log.h"
-#include "api/transport/field_trial_based_config.h"
-#include "api/turn_customizer.h"
+#include "api/transport/bitrate_settings.h"
#include "api/units/data_rate.h"
-#include "api/video_track_source_proxy.h"
-#include "media/base/rtp_data_engine.h"
-#include "media/sctp/sctp_transport.h"
+#include "call/audio_state.h"
+#include "media/base/media_engine.h"
#include "p2p/base/basic_async_resolver_factory.h"
#include "p2p/base/basic_packet_socket_factory.h"
#include "p2p/base/default_ice_transport_factory.h"
@@ -38,12 +40,17 @@
#include "pc/media_stream.h"
#include "pc/peer_connection.h"
#include "pc/rtp_parameters_conversion.h"
+#include "pc/session_description.h"
#include "pc/video_track.h"
#include "rtc_base/bind.h"
#include "rtc_base/checks.h"
#include "rtc_base/experiments/field_trial_parser.h"
#include "rtc_base/experiments/field_trial_units.h"
+#include "rtc_base/location.h"
+#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
+#include "rtc_base/ref_counted_object.h"
+#include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/system/file_wrapper.h"
namespace webrtc {
@@ -51,42 +58,61 @@
rtc::scoped_refptr<PeerConnectionFactoryInterface>
CreateModularPeerConnectionFactory(
PeerConnectionFactoryDependencies dependencies) {
- rtc::scoped_refptr<PeerConnectionFactory> pc_factory(
- new rtc::RefCountedObject<PeerConnectionFactory>(
- std::move(dependencies)));
- // Call Initialize synchronously but make sure it is executed on
- // |signaling_thread|.
- MethodCall<PeerConnectionFactory, bool> call(
- pc_factory.get(), &PeerConnectionFactory::Initialize);
- bool result = call.Marshal(RTC_FROM_HERE, pc_factory->signaling_thread());
+ // The PeerConnectionFactory must be created on the signaling thread.
+ if (dependencies.signaling_thread &&
+ !dependencies.signaling_thread->IsCurrent()) {
+ return dependencies.signaling_thread
+ ->Invoke<rtc::scoped_refptr<PeerConnectionFactoryInterface>>(
+ RTC_FROM_HERE, [&dependencies] {
+ return CreateModularPeerConnectionFactory(
+ std::move(dependencies));
+ });
+ }
- if (!result) {
+ auto pc_factory = PeerConnectionFactory::Create(std::move(dependencies));
+ if (!pc_factory) {
return nullptr;
}
+ // Verify that the invocation and the initialization ended up agreeing on the
+ // thread.
+ RTC_DCHECK_RUN_ON(pc_factory->signaling_thread());
return PeerConnectionFactoryProxy::Create(pc_factory->signaling_thread(),
pc_factory);
}
+// Static
+rtc::scoped_refptr<PeerConnectionFactory> PeerConnectionFactory::Create(
+ PeerConnectionFactoryDependencies dependencies) {
+ auto context = ConnectionContext::Create(&dependencies);
+ if (!context) {
+ return nullptr;
+ }
+ return new rtc::RefCountedObject<PeerConnectionFactory>(context,
+ &dependencies);
+}
+
+PeerConnectionFactory::PeerConnectionFactory(
+ rtc::scoped_refptr<ConnectionContext> context,
+ PeerConnectionFactoryDependencies* dependencies)
+ : context_(context),
+ task_queue_factory_(std::move(dependencies->task_queue_factory)),
+ event_log_factory_(std::move(dependencies->event_log_factory)),
+ fec_controller_factory_(std::move(dependencies->fec_controller_factory)),
+ network_state_predictor_factory_(
+ std::move(dependencies->network_state_predictor_factory)),
+ injected_network_controller_factory_(
+ std::move(dependencies->network_controller_factory)),
+ neteq_factory_(std::move(dependencies->neteq_factory)) {}
+
PeerConnectionFactory::PeerConnectionFactory(
PeerConnectionFactoryDependencies dependencies)
- : context_(new rtc::RefCountedObject<ConnectionContext>(dependencies)),
- task_queue_factory_(std::move(dependencies.task_queue_factory)),
- event_log_factory_(std::move(dependencies.event_log_factory)),
- fec_controller_factory_(std::move(dependencies.fec_controller_factory)),
- network_state_predictor_factory_(
- std::move(dependencies.network_state_predictor_factory)),
- injected_network_controller_factory_(
- std::move(dependencies.network_controller_factory)),
- neteq_factory_(std::move(dependencies.neteq_factory)) {}
+ : PeerConnectionFactory(ConnectionContext::Create(&dependencies),
+ &dependencies) {}
PeerConnectionFactory::~PeerConnectionFactory() {
RTC_DCHECK_RUN_ON(signaling_thread());
}
-bool PeerConnectionFactory::Initialize() {
- return context_->Initialize();
-}
-
void PeerConnectionFactory::SetOptions(const Options& options) {
context_->SetOptions(options);
}
diff --git a/pc/peer_connection_factory.h b/pc/peer_connection_factory.h
index 430eca5..499ea0b 100644
--- a/pc/peer_connection_factory.h
+++ b/pc/peer_connection_factory.h
@@ -12,13 +12,30 @@
#ifndef PC_PEER_CONNECTION_FACTORY_H_
#define PC_PEER_CONNECTION_FACTORY_H_
+#include <stdint.h>
+#include <stdio.h>
#include <memory>
#include <string>
+#include "absl/strings/string_view.h"
+#include "api/audio_options.h"
+#include "api/fec_controller.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/peer_connection_interface.h"
+#include "api/rtc_event_log/rtc_event_log.h"
+#include "api/rtc_event_log/rtc_event_log_factory_interface.h"
+#include "api/rtp_parameters.h"
#include "api/scoped_refptr.h"
+#include "api/task_queue/task_queue_factory.h"
+#include "api/transport/network_control.h"
+#include "api/transport/sctp_transport_factory_interface.h"
+#include "api/transport/webrtc_key_value_config.h"
+#include "call/call.h"
#include "media/sctp/sctp_transport_internal.h"
+#include "p2p/base/port_allocator.h"
#include "pc/channel_manager.h"
#include "pc/connection_context.h"
#include "rtc_base/rtc_certificate_generator.h"
@@ -35,6 +52,14 @@
class PeerConnectionFactory : public PeerConnectionFactoryInterface {
public:
+ // Creates a PeerConnectionFactory. It returns nullptr on initialization
+ // error.
+ //
+ // The Dependencies structure allows simple management of all new
+ // dependencies being added to the PeerConnectionFactory.
+ static rtc::scoped_refptr<PeerConnectionFactory> Create(
+ PeerConnectionFactoryDependencies dependencies);
+
void SetOptions(const Options& options) override;
rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
@@ -47,8 +72,6 @@
const PeerConnectionInterface::RTCConfiguration& configuration,
PeerConnectionDependencies dependencies) override;
- bool Initialize();
-
RtpCapabilities GetRtpSenderCapabilities(
cricket::MediaType kind) const override;
@@ -83,16 +106,18 @@
// created in CreatePeerConnectionFactory().
return context_->signaling_thread();
}
- rtc::Thread* worker_thread() const { return context_->worker_thread(); }
- rtc::Thread* network_thread() const { return context_->network_thread(); }
const Options& options() const { return context_->options(); }
const WebRtcKeyValueConfig& trials() const { return context_->trials(); }
protected:
- // This structure allows simple management of all new dependencies being added
- // to the PeerConnectionFactory.
+ // Constructor used by the static Create() method. Modifies the dependencies.
+ PeerConnectionFactory(rtc::scoped_refptr<ConnectionContext> context,
+ PeerConnectionFactoryDependencies* dependencies);
+
+ // Constructor for use in testing. Ignores the possibility of initialization
+ // failure. The dependencies are passed in by std::move().
explicit PeerConnectionFactory(
PeerConnectionFactoryDependencies dependencies);
@@ -103,6 +128,9 @@
virtual ~PeerConnectionFactory();
private:
+ rtc::Thread* worker_thread() const { return context_->worker_thread(); }
+ rtc::Thread* network_thread() const { return context_->network_thread(); }
+
bool IsTrialEnabled(absl::string_view key) const;
const cricket::ChannelManager* channel_manager() const {
return context_->channel_manager();
diff --git a/pc/peer_connection_histogram_unittest.cc b/pc/peer_connection_histogram_unittest.cc
index 8fcb87a..a9c5c9b 100644
--- a/pc/peer_connection_histogram_unittest.cc
+++ b/pc/peer_connection_histogram_unittest.cc
@@ -339,7 +339,6 @@
rtc::scoped_refptr<PeerConnectionFactoryForUsageHistogramTest> pc_factory(
new PeerConnectionFactoryForUsageHistogramTest());
pc_factory->SetOptions(factory_options);
- RTC_CHECK(pc_factory->Initialize());
if (immediate_report) {
pc_factory->ReturnHistogramVeryQuickly();
}
diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc
index a0e3845..abedf48 100644
--- a/pc/peer_connection_interface_unittest.cc
+++ b/pc/peer_connection_interface_unittest.cc
@@ -699,7 +699,6 @@
ASSERT_TRUE(pc_factory_);
pc_factory_for_test_ =
PeerConnectionFactoryForTest::CreatePeerConnectionFactoryForTest();
- pc_factory_for_test_->Initialize();
}
void CreatePeerConnection() {
@@ -3923,7 +3922,6 @@
protected:
void SetUp() override {
pcf_ = PeerConnectionFactoryForTest::CreatePeerConnectionFactoryForTest();
- pcf_->Initialize();
}
const cricket::MediaConfig TestCreatePeerConnection(
const RTCConfiguration& config) {
