Remove DataChannelType and deprecated option disable_sctp_data_channels

Since there is only a single type of DataChannel now, the enum was only used
when data channels were disabled at the PC API. That option has been
deprecated 4 years ago, it's now time to remove it.

Bug: webrtc:6625
Change-Id: I9e4ada1756da186e9639dd0fbf0249c55ea0b6c7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215661
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33778}
This commit is contained in:
Florent Castelli 2021-04-19 15:29:50 +02:00 committed by Commit Bot
parent eb9c3f237b
commit 516e284351
13 changed files with 35 additions and 171 deletions

View File

@ -1398,10 +1398,6 @@ class RTC_EXPORT PeerConnectionFactoryInterface
// testing/debugging.
bool disable_encryption = false;
// Deprecated. The only effect of setting this to true is that
// CreateDataChannel will fail, which is not that useful.
bool disable_sctp_data_channels = false;
// If set to true, any platform-supported network monitoring capability
// won't be used, and instead networks will only be updated via polling.
//

View File

@ -156,11 +156,6 @@ class CompositeMediaEngine : public MediaEngineInterface {
const std::unique_ptr<VideoEngineInterface> video_engine_;
};
enum DataChannelType {
DCT_NONE = 0,
DCT_SCTP = 2,
};
webrtc::RtpParameters CreateRtpParametersWithOneEncoding();
webrtc::RtpParameters CreateRtpParametersWithEncodings(StreamParams sp);

View File

@ -262,17 +262,11 @@ DataChannelController::InternalCreateDataChannelWithProxy(
if (pc_->IsClosed()) {
return nullptr;
}
if (data_channel_type_ == cricket::DCT_NONE) {
RTC_LOG(LS_ERROR)
<< "InternalCreateDataChannel: Data is not supported in this call.";
return nullptr;
}
if (IsSctpLike(data_channel_type())) {
rtc::scoped_refptr<SctpDataChannel> channel =
InternalCreateSctpDataChannel(label, config);
if (channel) {
return SctpDataChannel::CreateProxy(channel);
}
rtc::scoped_refptr<SctpDataChannel> channel =
InternalCreateSctpDataChannel(label, config);
if (channel) {
return SctpDataChannel::CreateProxy(channel);
}
return nullptr;
@ -377,18 +371,6 @@ SctpDataChannel* DataChannelController::FindDataChannelBySid(int sid) const {
return nullptr;
}
cricket::DataChannelType DataChannelController::data_channel_type() const {
// TODO(bugs.webrtc.org/9987): Should be restricted to the signaling thread.
// RTC_DCHECK_RUN_ON(signaling_thread());
return data_channel_type_;
}
void DataChannelController::set_data_channel_type(
cricket::DataChannelType type) {
RTC_DCHECK_RUN_ON(signaling_thread());
data_channel_type_ = type;
}
DataChannelTransportInterface* DataChannelController::data_channel_transport()
const {
// TODO(bugs.webrtc.org/11547): Only allow this accessor to be called on the

View File

@ -102,8 +102,6 @@ class DataChannelController : public SctpDataChannelProviderInterface,
}
// Accessors
cricket::DataChannelType data_channel_type() const;
void set_data_channel_type(cricket::DataChannelType type);
DataChannelTransportInterface* data_channel_transport() const;
void set_data_channel_transport(DataChannelTransportInterface* transport);
@ -144,11 +142,6 @@ class DataChannelController : public SctpDataChannelProviderInterface,
rtc::Thread* network_thread() const;
rtc::Thread* signaling_thread() const;
// Specifies whether or not SCTP data channels are allowed.
cricket::DataChannelType data_channel_type_ =
cricket::DCT_NONE; // TODO(bugs.webrtc.org/9987): Accessed on both
// signaling and network thread.
// Plugin transport used for data channels. Pointer may be accessed and
// checked from any thread, but the object may only be touched on the
// network thread.

View File

@ -51,8 +51,4 @@ void PacketQueue::Swap(PacketQueue* other) {
other->packets_.swap(packets_);
}
bool IsSctpLike(cricket::DataChannelType type) {
return type == cricket::DCT_SCTP;
}
} // namespace webrtc

View File

@ -57,8 +57,6 @@ struct DataChannelStats {
uint64_t bytes_received;
};
bool IsSctpLike(cricket::DataChannelType type);
} // namespace webrtc
#endif // PC_DATA_CHANNEL_UTILS_H_

View File

@ -2656,8 +2656,7 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer(
bool secure = bundle_transport ? bundle_transport->description.secure()
: data_transport->secure();
bool rejected = session_options.data_channel_type == DCT_NONE ||
media_description_options.stopped ||
bool rejected = media_description_options.stopped ||
offer_content->rejected ||
!IsMediaProtocolSupported(MEDIA_TYPE_DATA,
data_answer->protocol(), secure);

View File

@ -23,7 +23,6 @@
#include "api/rtp_parameters.h"
#include "api/rtp_transceiver_direction.h"
#include "media/base/media_constants.h"
#include "media/base/media_engine.h" // For DataChannelType
#include "media/base/rid_description.h"
#include "media/base/stream_params.h"
#include "p2p/base/ice_credentials_iterator.h"
@ -106,7 +105,6 @@ struct MediaSessionOptions {
bool HasMediaDescription(MediaType type) const;
DataChannelType data_channel_type = DCT_NONE;
bool vad_enabled = true; // When disabled, removes all CN codecs from SDP.
bool rtcp_mux_enabled = true;
bool bundle_enabled = false;

View File

@ -329,10 +329,8 @@ static void AddAudioVideoSections(RtpTransceiverDirection direction,
opts);
}
static void AddDataSection(cricket::DataChannelType dct,
RtpTransceiverDirection direction,
static void AddDataSection(RtpTransceiverDirection direction,
MediaSessionOptions* opts) {
opts->data_channel_type = dct;
AddMediaDescriptionOptions(MEDIA_TYPE_DATA, "data", direction, kActive, opts);
}
@ -869,7 +867,6 @@ TEST_F(MediaSessionDescriptionFactoryTest,
AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video",
RtpTransceiverDirection::kInactive, kStopped,
&opts);
opts.data_channel_type = cricket::DCT_NONE;
opts.bundle_enabled = true;
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, NULL);
std::unique_ptr<SessionDescription> answer =
@ -898,7 +895,7 @@ TEST_F(MediaSessionDescriptionFactoryTest,
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSctpDataOffer) {
MediaSessionOptions opts;
opts.bundle_enabled = true;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
f1_.set_secure(SEC_ENABLED);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, NULL);
EXPECT_TRUE(offer.get() != NULL);
@ -913,7 +910,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSctpDataOffer) {
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSecureSctpDataOffer) {
MediaSessionOptions opts;
opts.bundle_enabled = true;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
f1_.set_secure(SEC_ENABLED);
tdf1_.set_secure(SEC_ENABLED);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, NULL);
@ -929,7 +926,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSecureSctpDataOffer) {
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateImplicitSctpDataOffer) {
MediaSessionOptions opts;
opts.bundle_enabled = true;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
f1_.set_secure(SEC_ENABLED);
std::unique_ptr<SessionDescription> offer1(f1_.CreateOffer(opts, NULL));
ASSERT_TRUE(offer1.get() != NULL);
@ -937,10 +934,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateImplicitSctpDataOffer) {
ASSERT_TRUE(data != NULL);
ASSERT_EQ(cricket::kMediaProtocolSctp, data->media_description()->protocol());
// Now set data_channel_type to 'none' (default) and make sure that the
// datachannel type that gets generated from the previous offer, is of the
// same type.
opts.data_channel_type = cricket::DCT_NONE;
std::unique_ptr<SessionDescription> offer2(
f1_.CreateOffer(opts, offer1.get()));
data = offer2->GetContentByName("data");
@ -1144,7 +1137,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateSendOnlyOffer) {
// SessionDescription is preserved in the new SessionDescription.
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateOfferContentOrder) {
MediaSessionOptions opts;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
std::unique_ptr<SessionDescription> offer1(f1_.CreateOffer(opts, NULL));
ASSERT_TRUE(offer1.get() != NULL);
@ -1282,7 +1275,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoAnswerGcmAnswer) {
// default. The answer's use_sctpmap flag should match the offer's.
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerUsesSctpmap) {
MediaSessionOptions opts;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, NULL);
ASSERT_TRUE(offer.get() != NULL);
ContentInfo* dc_offer = offer->GetContentByName("data");
@ -1303,7 +1296,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerUsesSctpmap) {
// The answer's use_sctpmap flag should match the offer's.
TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerWithoutSctpmap) {
MediaSessionOptions opts;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, NULL);
ASSERT_TRUE(offer.get() != NULL);
ContentInfo* dc_offer = offer->GetContentByName("data");
@ -1333,7 +1326,7 @@ TEST_F(MediaSessionDescriptionFactoryTest,
tdf2_.set_secure(SEC_ENABLED);
MediaSessionOptions opts;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, nullptr);
ASSERT_TRUE(offer.get() != nullptr);
ContentInfo* dc_offer = offer->GetContentByName("data");
@ -1367,7 +1360,7 @@ TEST_F(MediaSessionDescriptionFactoryTest,
tdf2_.set_secure(SEC_ENABLED);
MediaSessionOptions opts;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, nullptr);
ASSERT_TRUE(offer.get() != nullptr);
ContentInfo* dc_offer = offer->GetContentByName("data");
@ -1396,7 +1389,7 @@ TEST_F(MediaSessionDescriptionFactoryTest,
tdf2_.set_secure(SEC_ENABLED);
MediaSessionOptions opts;
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
std::unique_ptr<SessionDescription> offer = f1_.CreateOffer(opts, nullptr);
ASSERT_TRUE(offer.get() != nullptr);
ContentInfo* dc_offer = offer->GetContentByName("data");
@ -1421,7 +1414,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAnswerContentOrder) {
MediaSessionOptions opts;
// Creates a data only offer.
AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kSendRecv, &opts);
AddDataSection(RtpTransceiverDirection::kSendRecv, &opts);
std::unique_ptr<SessionDescription> offer1(f1_.CreateOffer(opts, NULL));
ASSERT_TRUE(offer1.get() != NULL);
@ -3799,7 +3792,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestMIDsMatchesExistingOffer) {
AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video_modified",
RtpTransceiverDirection::kRecvOnly, kActive,
&opts);
opts.data_channel_type = cricket::DCT_SCTP;
AddMediaDescriptionOptions(MEDIA_TYPE_DATA, "data_modified",
RtpTransceiverDirection::kSendRecv, kActive,
&opts);

View File

@ -597,11 +597,6 @@ RTCError PeerConnection::Initialize(
NoteUsageEvent(UsageEvent::TURN_SERVER_ADDED);
}
// DTLS has to be enabled to use SCTP.
if (!options_.disable_sctp_data_channels && dtls_enabled_) {
data_channel_controller_.set_data_channel_type(cricket::DCT_SCTP);
}
// Network thread initialization.
network_thread()->Invoke<void>(RTC_FROM_HERE, [this, &stun_servers,
&turn_servers, &configuration,
@ -680,7 +675,7 @@ void PeerConnection::InitializeTransportController_n(
config.active_reset_srtp_params = configuration.active_reset_srtp_params;
// DTLS has to be enabled to use SCTP.
if (!options_.disable_sctp_data_channels && dtls_enabled_) {
if (dtls_enabled_) {
config.sctp_factory = context_->sctp_transport_factory();
}
@ -1943,12 +1938,7 @@ void PeerConnection::OnSelectedCandidatePairChanged(
absl::optional<std::string> PeerConnection::GetDataMid() const {
RTC_DCHECK_RUN_ON(signaling_thread());
switch (data_channel_type()) {
case cricket::DCT_SCTP:
return sctp_mid_s_;
default:
return absl::nullopt;
}
return sctp_mid_s_;
}
void PeerConnection::SetSctpDataMid(const std::string& mid) {
@ -2231,10 +2221,6 @@ std::unique_ptr<rtc::SSLCertChain> PeerConnection::GetRemoteSSLCertChain(
return transport_controller_->GetRemoteSSLCertChain(transport_name);
}
cricket::DataChannelType PeerConnection::data_channel_type() const {
return data_channel_controller_.data_channel_type();
}
bool PeerConnection::IceRestartPending(const std::string& content_name) const {
RTC_DCHECK_RUN_ON(signaling_thread());
return sdp_handler_->IceRestartPending(content_name);

View File

@ -363,7 +363,6 @@ class PeerConnection : public PeerConnectionInternal,
const PeerConnectionFactoryInterface::Options* options() const {
return &options_;
}
cricket::DataChannelType data_channel_type() const;
void SetIceConnectionState(IceConnectionState new_state);
void NoteUsageEvent(UsageEvent event);

View File

@ -289,34 +289,6 @@ TEST_P(PeerConnectionDataChannelTest,
EXPECT_TRUE(caller->pc()->CreateDataChannel("dc", nullptr));
}
TEST_P(PeerConnectionDataChannelTest, CreateDataChannelWithSctpDisabledFails) {
PeerConnectionFactoryInterface::Options options;
options.disable_sctp_data_channels = true;
auto caller = CreatePeerConnection(RTCConfiguration(), options);
EXPECT_FALSE(caller->pc()->CreateDataChannel("dc", nullptr));
}
// Test that if a callee has SCTP disabled and receives an offer with an SCTP
// data channel, the data section is rejected and no SCTP transport is created
// on the callee.
TEST_P(PeerConnectionDataChannelTest,
DataSectionRejectedIfCalleeHasSctpDisabled) {
auto caller = CreatePeerConnectionWithDataChannel();
PeerConnectionFactoryInterface::Options options;
options.disable_sctp_data_channels = true;
auto callee = CreatePeerConnection(RTCConfiguration(), options);
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
EXPECT_FALSE(callee->sctp_transport_factory()->last_fake_sctp_transport());
auto answer = callee->CreateAnswer();
auto* data_content = cricket::GetFirstDataContent(answer->description());
ASSERT_TRUE(data_content);
EXPECT_TRUE(data_content->rejected);
}
TEST_P(PeerConnectionDataChannelTest, SctpPortPropagatedFromSdpToTransport) {
constexpr int kNewSendPort = 9998;
constexpr int kNewRecvPort = 7775;
@ -371,28 +343,4 @@ INSTANTIATE_TEST_SUITE_P(PeerConnectionDataChannelTest,
Values(SdpSemantics::kPlanB,
SdpSemantics::kUnifiedPlan));
TEST_F(PeerConnectionDataChannelUnifiedPlanTest,
ReOfferAfterPeerRejectsDataChannel) {
auto caller = CreatePeerConnectionWithDataChannel();
PeerConnectionFactoryInterface::Options options;
options.disable_sctp_data_channels = true;
auto callee = CreatePeerConnection(RTCConfiguration(), options);
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
auto offer = caller->CreateOffer();
ASSERT_TRUE(offer);
const auto& contents = offer->description()->contents();
ASSERT_EQ(1u, contents.size());
EXPECT_TRUE(contents[0].rejected);
ASSERT_TRUE(
caller->SetLocalDescription(CloneSessionDescription(offer.get())));
ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
auto answer = callee->CreateAnswerAndSetAsLocal();
ASSERT_TRUE(answer);
EXPECT_TRUE(caller->SetRemoteDescription(std::move(answer)));
}
} // namespace webrtc

View File

@ -1375,7 +1375,7 @@ RTCError SdpOfferAnswerHandler::ApplyLocalDescription(
// If setting the description decided our SSL role, allocate any necessary
// SCTP sids.
rtc::SSLRole role;
if (IsSctpLike(pc_->data_channel_type()) && pc_->GetSctpSslRole(&role)) {
if (pc_->GetSctpSslRole(&role)) {
data_channel_controller()->AllocateSctpSids(role);
}
@ -1644,7 +1644,7 @@ RTCError SdpOfferAnswerHandler::ApplyRemoteDescription(
// If setting the description decided our SSL role, allocate any necessary
// SCTP sids.
rtc::SSLRole role;
if (IsSctpLike(pc_->data_channel_type()) && pc_->GetSctpSslRole(&role)) {
if (pc_->GetSctpSslRole(&role)) {
data_channel_controller()->AllocateSctpSids(role);
}
@ -3352,11 +3352,6 @@ RTCError SdpOfferAnswerHandler::UpdateDataChannel(
cricket::ContentSource source,
const cricket::ContentInfo& content,
const cricket::ContentGroup* bundle_group) {
if (pc_->data_channel_type() == cricket::DCT_NONE) {
// If data channels are disabled, ignore this media section. CreateAnswer
// will take care of rejecting it.
return RTCError::OK();
}
if (content.rejected) {
RTC_LOG(LS_INFO) << "Rejected data channel, mid=" << content.mid();
DestroyDataChannelTransport();
@ -3494,8 +3489,6 @@ void SdpOfferAnswerHandler::GetOptionsForOffer(
GetOptionsForPlanBOffer(offer_answer_options, session_options);
}
session_options->data_channel_type = pc_->data_channel_type();
// Apply ICE restart flag and renomination flag.
bool ice_restart = offer_answer_options.ice_restart || HasNewIceCredentials();
for (auto& options : session_options->media_description_options) {
@ -3753,8 +3746,6 @@ void SdpOfferAnswerHandler::GetOptionsForAnswer(
GetOptionsForPlanBAnswer(offer_answer_options, session_options);
}
session_options->data_channel_type = pc_->data_channel_type();
// Apply ICE renomination flag.
for (auto& options : session_options->media_description_options) {
options.transport_options.enable_ice_renomination =
@ -3856,8 +3847,7 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanAnswer(
// Reject all data sections if data channels are disabled.
// Reject a data section if it has already been rejected.
// Reject all data sections except for the first one.
if (pc_->data_channel_type() == cricket::DCT_NONE || content.rejected ||
content.name != *(pc_->GetDataMid())) {
if (content.rejected || content.name != *(pc_->GetDataMid())) {
session_options->media_description_options.push_back(
GetMediaDescriptionOptionsForRejectedData(content.name));
} else {
@ -4497,8 +4487,8 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) {
}
const cricket::ContentInfo* data = cricket::GetFirstDataContent(&desc);
if (pc_->data_channel_type() != cricket::DCT_NONE && data &&
!data->rejected && !data_channel_controller()->data_channel_transport()) {
if (data && !data->rejected &&
!data_channel_controller()->data_channel_transport()) {
if (!CreateDataChannel(data->name)) {
LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR,
"Failed to create data channel.");
@ -4548,26 +4538,18 @@ cricket::VideoChannel* SdpOfferAnswerHandler::CreateVideoChannel(
bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) {
RTC_DCHECK_RUN_ON(signaling_thread());
switch (pc_->data_channel_type()) {
case cricket::DCT_SCTP:
if (!pc_->network_thread()->Invoke<bool>(RTC_FROM_HERE, [this, &mid] {
RTC_DCHECK_RUN_ON(pc_->network_thread());
return pc_->SetupDataChannelTransport_n(mid);
})) {
return false;
}
// TODO(tommi): Is this necessary? SetupDataChannelTransport_n() above
// will have queued up updating the transport name on the signaling thread
// and could update the mid at the same time. This here is synchronous
// though, but it changes the state of PeerConnection and makes it be
// out of sync (transport name not set while the mid is set).
pc_->SetSctpDataMid(mid);
break;
case cricket::DCT_NONE:
// User error.
RTC_NOTREACHED();
return false;
if (!pc_->network_thread()->Invoke<bool>(RTC_FROM_HERE, [this, &mid] {
RTC_DCHECK_RUN_ON(pc_->network_thread());
return pc_->SetupDataChannelTransport_n(mid);
})) {
return false;
}
// TODO(tommi): Is this necessary? SetupDataChannelTransport_n() above
// will have queued up updating the transport name on the signaling thread
// and could update the mid at the same time. This here is synchronous
// though, but it changes the state of PeerConnection and makes it be
// out of sync (transport name not set while the mid is set).
pc_->SetSctpDataMid(mid);
return true;
}