sctp: Pass webrtc::Environment to DcSctpTransport

The DcSctpTransport will soon use field trials to conditionally enable
some options.

And overall, there is a migration project to start using the Environment
and this CL is in that direction, also setting the boundary; The dcSCTP
library should not depend on it. But the transport is allowed to.

Bug: webrtc:14997
Change-Id: I1f3c2c0d8dd7bdc698dd1d58bde7651b682bcba4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/341480
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41872}
This commit is contained in:
Victor Boivie 2024-02-28 12:16:15 +01:00 committed by WebRTC LUCI CQ
parent 4c1c9157d6
commit cd54fd8606
18 changed files with 74 additions and 44 deletions

View File

@ -94,6 +94,7 @@ rtc_library("goog_cc") {
rtc_source_set("sctp_transport_factory_interface") {
visibility = [ "*" ]
sources = [ "sctp_transport_factory_interface.h" ]
deps = [ "../../api/environment" ]
}
rtc_source_set("stun_types") {

View File

@ -13,6 +13,8 @@
#include <memory>
#include "api/environment/environment.h"
// These classes are not part of the API, and are treated as opaque pointers.
namespace cricket {
class SctpTransportInternal;
@ -34,6 +36,7 @@ class SctpTransportFactoryInterface {
// Create an SCTP transport using `channel` for the underlying transport.
virtual std::unique_ptr<cricket::SctpTransportInternal> CreateSctpTransport(
const Environment& env,
rtc::PacketTransportInternal* channel) = 0;
};

View File

@ -643,6 +643,7 @@ if (rtc_build_dcsctp) {
":media_channel",
":rtc_data_sctp_transport_internal",
"../api:array_view",
"../api/environment",
"../api/task_queue:pending_task_safety_flag",
"../api/task_queue:task_queue",
"../media:rtc_media_base",
@ -682,6 +683,7 @@ rtc_library("rtc_data_sctp_transport_factory") {
]
deps = [
":rtc_data_sctp_transport_internal",
"../api/environment",
"../api/transport:sctp_transport_factory_interface",
"../rtc_base:threading",
"../rtc_base/system:unused",

View File

@ -19,6 +19,7 @@
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "api/array_view.h"
#include "api/environment/environment.h"
#include "media/base/media_channel.h"
#include "net/dcsctp/public/dcsctp_socket_factory.h"
#include "net/dcsctp/public/packet_observer.h"
@ -114,23 +115,22 @@ bool IsEmptyPPID(dcsctp::PPID ppid) {
}
} // namespace
DcSctpTransport::DcSctpTransport(rtc::Thread* network_thread,
rtc::PacketTransportInternal* transport,
Clock* clock)
: DcSctpTransport(network_thread,
DcSctpTransport::DcSctpTransport(const Environment& env,
rtc::Thread* network_thread,
rtc::PacketTransportInternal* transport)
: DcSctpTransport(env,
network_thread,
transport,
clock,
std::make_unique<dcsctp::DcSctpSocketFactory>()) {}
DcSctpTransport::DcSctpTransport(
const Environment& env,
rtc::Thread* network_thread,
rtc::PacketTransportInternal* transport,
Clock* clock,
std::unique_ptr<dcsctp::DcSctpSocketFactory> socket_factory)
: network_thread_(network_thread),
transport_(transport),
clock_(clock),
random_(clock_->TimeInMicroseconds()),
env_(env),
random_(env_.clock().TimeInMicroseconds()),
socket_factory_(std::move(socket_factory)),
task_queue_timeout_factory_(
*network_thread,
@ -423,7 +423,7 @@ std::unique_ptr<dcsctp::Timeout> DcSctpTransport::CreateTimeout(
}
dcsctp::TimeMs DcSctpTransport::TimeMillis() {
return dcsctp::TimeMs(clock_->TimeInMilliseconds());
return dcsctp::TimeMs(env_.clock().TimeInMilliseconds());
}
uint32_t DcSctpTransport::GetRandomInt(uint32_t low, uint32_t high) {

View File

@ -17,6 +17,7 @@
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "api/array_view.h"
#include "api/environment/environment.h"
#include "api/task_queue/task_queue_base.h"
#include "media/sctp/sctp_transport_internal.h"
#include "net/dcsctp/public/dcsctp_options.h"
@ -40,12 +41,12 @@ class DcSctpTransport : public cricket::SctpTransportInternal,
public dcsctp::DcSctpSocketCallbacks,
public sigslot::has_slots<> {
public:
DcSctpTransport(rtc::Thread* network_thread,
DcSctpTransport(const Environment& env,
rtc::Thread* network_thread,
rtc::PacketTransportInternal* transport);
DcSctpTransport(const Environment& env,
rtc::Thread* network_thread,
rtc::PacketTransportInternal* transport,
Clock* clock);
DcSctpTransport(rtc::Thread* network_thread,
rtc::PacketTransportInternal* transport,
Clock* clock,
std::unique_ptr<dcsctp::DcSctpSocketFactory> socket_factory);
~DcSctpTransport() override;
@ -102,7 +103,7 @@ class DcSctpTransport : public cricket::SctpTransportInternal,
rtc::Thread* network_thread_;
rtc::PacketTransportInternal* transport_;
Clock* clock_;
Environment env_;
Random random_;
std::unique_ptr<dcsctp::DcSctpSocketFactory> socket_factory_;

View File

@ -13,6 +13,8 @@
#include <memory>
#include <utility>
#include "api/environment/environment.h"
#include "api/environment/environment_factory.h"
#include "net/dcsctp/public/mock_dcsctp_socket.h"
#include "net/dcsctp/public/mock_dcsctp_socket_factory.h"
#include "p2p/base/fake_packet_transport.h"
@ -49,7 +51,10 @@ static_assert(!std::is_abstract_v<MockDataChannelSink>);
class Peer {
public:
Peer() : fake_packet_transport_("transport"), simulated_clock_(1000) {
Peer()
: fake_packet_transport_("transport"),
simulated_clock_(1000),
env_(CreateEnvironment(&simulated_clock_)) {
auto socket_ptr = std::make_unique<dcsctp::MockDcSctpSocket>();
socket_ = socket_ptr.get();
@ -60,7 +65,7 @@ class Peer {
.WillOnce(Return(ByMove(std::move(socket_ptr))));
sctp_transport_ = std::make_unique<webrtc::DcSctpTransport>(
rtc::Thread::Current(), &fake_packet_transport_, &simulated_clock_,
env_, rtc::Thread::Current(), &fake_packet_transport_,
std::move(mock_dcsctp_socket_factory));
sctp_transport_->SetDataChannelSink(&sink_);
sctp_transport_->SetOnConnectedCallback([this]() { sink_.OnConnected(); });
@ -68,6 +73,7 @@ class Peer {
rtc::FakePacketTransport fake_packet_transport_;
webrtc::SimulatedClock simulated_clock_;
Environment env_;
dcsctp::MockDcSctpSocket* socket_;
std::unique_ptr<webrtc::DcSctpTransport> sctp_transport_;
NiceMock<MockDataChannelSink> sink_;

View File

@ -10,11 +10,11 @@
#include "media/sctp/sctp_transport_factory.h"
#include "api/environment/environment.h"
#include "rtc_base/system/unused.h"
#ifdef WEBRTC_HAVE_DCSCTP
#include "media/sctp/dcsctp_transport.h" // nogncheck
#include "system_wrappers/include/clock.h" // nogncheck
#include "media/sctp/dcsctp_transport.h" // nogncheck
#endif
namespace cricket {
@ -26,11 +26,12 @@ SctpTransportFactory::SctpTransportFactory(rtc::Thread* network_thread)
std::unique_ptr<SctpTransportInternal>
SctpTransportFactory::CreateSctpTransport(
const webrtc::Environment& env,
rtc::PacketTransportInternal* transport) {
std::unique_ptr<SctpTransportInternal> result;
#ifdef WEBRTC_HAVE_DCSCTP
result = std::unique_ptr<SctpTransportInternal>(new webrtc::DcSctpTransport(
network_thread_, transport, webrtc::Clock::GetRealTimeClock()));
result = std::unique_ptr<SctpTransportInternal>(
new webrtc::DcSctpTransport(env, network_thread_, transport));
#endif
return result;
}

View File

@ -13,6 +13,7 @@
#include <memory>
#include "api/environment/environment.h"
#include "api/transport/sctp_transport_factory_interface.h"
#include "media/sctp/sctp_transport_internal.h"
#include "rtc_base/thread.h"
@ -24,6 +25,7 @@ class SctpTransportFactory : public webrtc::SctpTransportFactoryInterface {
explicit SctpTransportFactory(rtc::Thread* network_thread);
std::unique_ptr<SctpTransportInternal> CreateSctpTransport(
const webrtc::Environment& env,
rtc::PacketTransportInternal* transport) override;
private:

View File

@ -302,6 +302,7 @@ rtc_source_set("jsep_transport_controller") {
"../api:scoped_refptr",
"../api:sequence_checker",
"../api/crypto:options",
"../api/environment",
"../api/rtc_event_log",
"../api/transport:datagram_transport_interface",
"../api/transport:enums",
@ -2086,6 +2087,7 @@ if (rtc_include_tests && !build_with_chromium) {
"../api:rtp_parameters",
"../api:scoped_refptr",
"../api:sequence_checker",
"../api/environment:environment_factory",
"../api/task_queue:pending_task_safety_flag",
"../api/task_queue:task_queue",
"../api/transport:datagram_transport_interface",

View File

@ -20,6 +20,7 @@
#include "absl/algorithm/container.h"
#include "api/dtls_transport_interface.h"
#include "api/environment/environment.h"
#include "api/rtp_parameters.h"
#include "api/sequence_checker.h"
#include "api/transport/enums.h"
@ -38,11 +39,13 @@ using webrtc::SdpType;
namespace webrtc {
JsepTransportController::JsepTransportController(
const Environment& env,
rtc::Thread* network_thread,
cricket::PortAllocator* port_allocator,
AsyncDnsResolverFactoryInterface* async_dns_resolver_factory,
Config config)
: network_thread_(network_thread),
: env_(env),
network_thread_(network_thread),
port_allocator_(port_allocator),
async_dns_resolver_factory_(async_dns_resolver_factory),
transports_(
@ -62,7 +65,6 @@ JsepTransportController::JsepTransportController(
RTC_DCHECK(config_.rtcp_handler);
RTC_DCHECK(config_.ice_transport_factory);
RTC_DCHECK(config_.on_dtls_handshake_error_);
RTC_DCHECK(config_.field_trials);
}
JsepTransportController::~JsepTransportController() {
@ -397,7 +399,7 @@ JsepTransportController::CreateIceTransport(const std::string& transport_name,
init.set_port_allocator(port_allocator_);
init.set_async_dns_resolver_factory(async_dns_resolver_factory_);
init.set_event_log(config_.event_log);
init.set_field_trials(config_.field_trials);
init.set_field_trials(&env_.field_trials());
auto transport = config_.ice_transport_factory->CreateIceTransport(
transport_name, component, std::move(init));
RTC_DCHECK(transport);
@ -494,7 +496,7 @@ std::unique_ptr<SrtpTransport> JsepTransportController::CreateSdesTransport(
cricket::DtlsTransportInternal* rtcp_dtls_transport) {
RTC_DCHECK_RUN_ON(network_thread_);
auto srtp_transport = std::make_unique<SrtpTransport>(
rtcp_dtls_transport == nullptr, *config_.field_trials);
rtcp_dtls_transport == nullptr, env_.field_trials());
RTC_DCHECK(rtp_dtls_transport);
srtp_transport->SetRtpPacketTransport(rtp_dtls_transport);
if (rtcp_dtls_transport) {
@ -513,7 +515,7 @@ JsepTransportController::CreateDtlsSrtpTransport(
cricket::DtlsTransportInternal* rtcp_dtls_transport) {
RTC_DCHECK_RUN_ON(network_thread_);
auto dtls_srtp_transport = std::make_unique<DtlsSrtpTransport>(
rtcp_dtls_transport == nullptr, *config_.field_trials);
rtcp_dtls_transport == nullptr, env_.field_trials());
if (config_.enable_external_auth) {
dtls_srtp_transport->EnableExternalAuth();
}
@ -1090,8 +1092,8 @@ RTCError JsepTransportController::MaybeCreateJsepTransport(
std::unique_ptr<cricket::SctpTransportInternal> sctp_transport;
if (config_.sctp_factory) {
sctp_transport =
config_.sctp_factory->CreateSctpTransport(rtp_dtls_transport.get());
sctp_transport = config_.sctp_factory->CreateSctpTransport(
env_, rtp_dtls_transport.get());
}
std::unique_ptr<cricket::JsepTransport> jsep_transport =

View File

@ -26,6 +26,7 @@
#include "api/async_dns_resolver.h"
#include "api/candidate.h"
#include "api/crypto/crypto_options.h"
#include "api/environment/environment.h"
#include "api/ice_transport_factory.h"
#include "api/ice_transport_interface.h"
#include "api/jsep.h"
@ -138,9 +139,6 @@ class JsepTransportController : public sigslot::has_slots<> {
// Factory for SCTP transports.
SctpTransportFactoryInterface* sctp_factory = nullptr;
std::function<void(rtc::SSLHandshakeError)> on_dtls_handshake_error_;
// Field trials.
const FieldTrialsView* field_trials;
};
// The ICE related events are fired on the `network_thread`.
@ -148,6 +146,7 @@ class JsepTransportController : public sigslot::has_slots<> {
// and destruction of the JsepTransportController must occur on the
// `network_thread`.
JsepTransportController(
const Environment& env,
rtc::Thread* network_thread,
cricket::PortAllocator* port_allocator,
AsyncDnsResolverFactoryInterface* async_dns_resolver_factory,
@ -483,6 +482,7 @@ class JsepTransportController : public sigslot::has_slots<> {
bool OnTransportChanged(const std::string& mid,
cricket::JsepTransport* transport);
const Environment env_;
rtc::Thread* const network_thread_ = nullptr;
cricket::PortAllocator* const port_allocator_ = nullptr;
AsyncDnsResolverFactoryInterface* const async_dns_resolver_factory_ = nullptr;

View File

@ -15,6 +15,7 @@
#include <utility>
#include "api/dtls_transport_interface.h"
#include "api/environment/environment_factory.h"
#include "api/transport/enums.h"
#include "p2p/base/candidate_pair_interface.h"
#include "p2p/base/dtls_transport_factory.h"
@ -83,7 +84,9 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
public ::testing::Test,
public sigslot::has_slots<> {
public:
JsepTransportControllerTest() : signaling_thread_(rtc::Thread::Current()) {
JsepTransportControllerTest()
: env_(CreateEnvironment(&field_trials_)),
signaling_thread_(rtc::Thread::Current()) {
fake_ice_transport_factory_ = std::make_unique<FakeIceTransportFactory>();
fake_dtls_transport_factory_ = std::make_unique<FakeDtlsTransportFactory>();
}
@ -100,10 +103,9 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
config.ice_transport_factory = fake_ice_transport_factory_.get();
config.dtls_transport_factory = fake_dtls_transport_factory_.get();
config.on_dtls_handshake_error_ = [](rtc::SSLHandshakeError s) {};
config.field_trials = &field_trials_;
transport_controller_ = std::make_unique<JsepTransportController>(
network_thread, port_allocator, nullptr /* async_resolver_factory */,
std::move(config));
env_, network_thread, port_allocator,
nullptr /* async_resolver_factory */, std::move(config));
SendTask(network_thread, [&] { ConnectTransportControllerSignals(); });
}
@ -345,6 +347,8 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
return true;
}
test::ScopedKeyValueConfig field_trials_;
Environment env_;
rtc::AutoThread main_thread_;
// Information received from signals from transport controller.
cricket::IceConnectionState connection_state_ =
@ -380,7 +384,6 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
// Transport controller needs to be destroyed first, because it may issue
// callbacks that modify the changed_*_by_mid in the destructor.
std::unique_ptr<JsepTransportController> transport_controller_;
test::ScopedKeyValueConfig field_trials_;
};
TEST_F(JsepTransportControllerTest, GetRtpTransport) {

View File

@ -773,10 +773,8 @@ JsepTransportController* PeerConnection::InitializeTransportController_n(
}
};
config.field_trials = &env_.field_trials();
transport_controller_.reset(new JsepTransportController(
network_thread(), port_allocator_.get(),
env_, network_thread(), port_allocator_.get(),
async_dns_resolver_factory_.get(), std::move(config)));
transport_controller_->SubscribeIceConnectionState(

View File

@ -12,6 +12,7 @@ rtc_source_set("fake_sctp_transport") {
visibility = [ "*" ]
sources = [ "fake_sctp_transport.h" ]
deps = [
"../../../api/environment",
"../../../api/transport:sctp_transport_factory_interface",
"../../../media:rtc_data_sctp_transport_internal",
]

View File

@ -13,6 +13,7 @@
#include <memory>
#include "api/environment/environment.h"
#include "api/transport/sctp_transport_factory_interface.h"
#include "media/sctp/sctp_transport_internal.h"
@ -61,6 +62,7 @@ class FakeSctpTransport : public cricket::SctpTransportInternal {
class FakeSctpTransportFactory : public webrtc::SctpTransportFactoryInterface {
public:
std::unique_ptr<cricket::SctpTransportInternal> CreateSctpTransport(
const webrtc::Environment& env,
rtc::PacketTransportInternal*) override {
last_fake_sctp_transport_ = new FakeSctpTransport();
return std::unique_ptr<cricket::SctpTransportInternal>(

View File

@ -35,6 +35,7 @@ if (rtc_include_tests) {
"../../api:time_controller",
"../../api/audio_codecs:builtin_audio_decoder_factory",
"../../api/audio_codecs:builtin_audio_encoder_factory",
"../../api/environment",
"../../api/rtc_event_log:rtc_event_log_factory",
"../../api/task_queue:default_task_queue_factory",
"../../api/transport:field_trial_based_config",

View File

@ -25,7 +25,8 @@ class ScenarioIceConnectionImpl : public ScenarioIceConnection,
private JsepTransportController::Observer,
private RtpPacketSinkInterface {
public:
ScenarioIceConnectionImpl(test::NetworkEmulationManagerImpl* net,
ScenarioIceConnectionImpl(const Environment& env,
test::NetworkEmulationManagerImpl* net,
IceConnectionObserver* observer);
~ScenarioIceConnectionImpl() override;
@ -73,12 +74,14 @@ class ScenarioIceConnectionImpl : public ScenarioIceConnection,
};
std::unique_ptr<ScenarioIceConnection> ScenarioIceConnection::Create(
const Environment& env,
webrtc::test::NetworkEmulationManagerImpl* net,
IceConnectionObserver* observer) {
return std::make_unique<ScenarioIceConnectionImpl>(net, observer);
return std::make_unique<ScenarioIceConnectionImpl>(env, net, observer);
}
ScenarioIceConnectionImpl::ScenarioIceConnectionImpl(
const Environment& env,
test::NetworkEmulationManagerImpl* net,
IceConnectionObserver* observer)
: observer_(observer),
@ -100,7 +103,8 @@ ScenarioIceConnectionImpl::ScenarioIceConnectionImpl(
new cricket::BasicPortAllocator(manager_->network_manager(),
manager_->packet_socket_factory())),
jsep_controller_(
new JsepTransportController(network_thread_,
new JsepTransportController(env,
network_thread_,
port_allocator_.get(),
/*async_resolver_factory*/ nullptr,
CreateJsepConfig())) {
@ -135,7 +139,6 @@ JsepTransportController::Config ScenarioIceConnectionImpl::CreateJsepConfig() {
RTC_DCHECK_RUN_ON(network_thread_);
observer_->OnPacketReceived(packet);
};
config.field_trials = &field_trials;
return config;
}

View File

@ -16,6 +16,7 @@
#include <vector>
#include "api/candidate.h"
#include "api/environment/environment.h"
#include "api/jsep.h"
#include "p2p/base/transport_description.h"
#include "test/network/network_emulation_manager.h"
@ -41,6 +42,7 @@ class ScenarioIceConnection {
~IceConnectionObserver() = default;
};
static std::unique_ptr<ScenarioIceConnection> Create(
const Environment& env,
test::NetworkEmulationManagerImpl* net,
IceConnectionObserver* observer);