Always ask for an SCTP m-section if datachannels have been used

This removes the behavior of not requesting datachannel if the first
datachannel is closed before the offer is created.

Bug: chromium:1423562
Change-Id: I90eab0f908507e65d9ee3dff51842ee6d61a8aa9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297860
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39570}
This commit is contained in:
Harald Alvestrand 2023-03-15 20:39:42 +00:00 committed by WebRTC LUCI CQ
parent 3a71e3a8f0
commit 5da3eb0d89
5 changed files with 29 additions and 5 deletions

View File

@ -28,6 +28,11 @@ bool DataChannelController::HasDataChannels() const {
return !sctp_data_channels_.empty();
}
bool DataChannelController::HasUsedDataChannels() const {
RTC_DCHECK_RUN_ON(signaling_thread());
return has_used_data_channels_;
}
bool DataChannelController::SendData(int sid,
const SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
@ -296,6 +301,7 @@ DataChannelController::InternalCreateSctpDataChannel(
return nullptr;
}
sctp_data_channels_.push_back(channel);
has_used_data_channels_ = true;
return channel;
}

View File

@ -89,7 +89,10 @@ class DataChannelController : public SctpDataChannelControllerInterface,
void AllocateSctpSids(rtc::SSLRole role);
// Checks if any data channel has been added.
// A data channel currently exist.
bool HasDataChannels() const;
// At some point in time, a data channel has existed.
bool HasUsedDataChannels() const;
bool HasSctpDataChannels() const {
RTC_DCHECK_RUN_ON(signaling_thread());
return !sctp_data_channels_.empty();
@ -148,6 +151,7 @@ class DataChannelController : public SctpDataChannelControllerInterface,
SctpSidAllocator sid_allocator_ /* RTC_GUARDED_BY(signaling_thread()) */;
std::vector<rtc::scoped_refptr<SctpDataChannel>> sctp_data_channels_
RTC_GUARDED_BY(signaling_thread());
bool has_used_data_channels_ RTC_GUARDED_BY(signaling_thread()) = false;
// Owning PeerConnection.
PeerConnectionInternal* const pc_;

View File

@ -70,6 +70,20 @@ TEST_F(DataChannelControllerTest, CreateDataChannelEarlyRelease) {
channel = nullptr; // dcc holds a reference to channel, so not destroyed yet
}
TEST_F(DataChannelControllerTest, CreateDataChannelEarlyClose) {
DataChannelController dcc(pc_.get());
EXPECT_FALSE(dcc.HasDataChannels());
EXPECT_FALSE(dcc.HasUsedDataChannels());
auto channel = dcc.InternalCreateDataChannelWithProxy(
"label",
std::make_unique<InternalDataChannelInit>(DataChannelInit()).get());
EXPECT_TRUE(dcc.HasDataChannels());
EXPECT_TRUE(dcc.HasUsedDataChannels());
channel->Close();
EXPECT_FALSE(dcc.HasDataChannels());
EXPECT_TRUE(dcc.HasUsedDataChannels());
}
TEST_F(DataChannelControllerTest, CreateDataChannelLateRelease) {
auto dcc = std::make_unique<DataChannelController>(pc_.get());
auto channel = dcc->InternalCreateDataChannelWithProxy(

View File

@ -1393,7 +1393,7 @@ PeerConnection::CreateDataChannelOrError(const std::string& label,
RTC_DCHECK_RUN_ON(signaling_thread());
TRACE_EVENT0("webrtc", "PeerConnection::CreateDataChannel");
bool first_datachannel = !data_channel_controller_.HasDataChannels();
bool first_datachannel = !data_channel_controller_.HasUsedDataChannels();
std::unique_ptr<InternalDataChannelInit> internal_config;
if (config) {

View File

@ -3955,7 +3955,7 @@ void SdpOfferAnswerHandler::GetOptionsForPlanBOffer(
const PeerConnectionInterface::RTCOfferAnswerOptions& offer_answer_options,
cricket::MediaSessionOptions* session_options) {
bool offer_new_data_description =
data_channel_controller()->HasDataChannels();
data_channel_controller()->HasUsedDataChannels();
bool send_audio = false;
bool send_video = false;
bool recv_audio = false;
@ -4172,9 +4172,9 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanOffer(
transceiver->set_mline_index(mline_index);
}
}
// Lastly, add a m-section if we have local data channels and an m section
// does not already exist.
if (!pc_->GetDataMid() && data_channel_controller()->HasDataChannels()) {
// Lastly, add a m-section if we have requested local data channels and an
// m section does not already exist.
if (!pc_->GetDataMid() && data_channel_controller()->HasUsedDataChannels()) {
session_options->media_description_options.push_back(
GetMediaDescriptionOptionsForActiveData(
mid_generator_.GenerateString()));