Skip to content

Commit

Permalink
Bug 1843113 - Vendor libwebrtc from ecab2a49da
Browse files Browse the repository at this point in the history
Upstream commit: https://webrtc.googlesource.com/src/+/ecab2a49da48f0279a3b6422dab602614630005e
    [m114] Attempt to recycle a stopped data m-line before creating a new one

    which avoids an infinitely growing SDP if the remote end rejects
    the datachannel section. This will reactivate the m-line even if
    all datachannels are closed.

    BUG=chromium:1442604
    (cherry picked from commit 522380ff734174faab694e1b67c9d20fffa8738e)

    No-Try: True
    Change-Id: If60f93b406271163df692d96102baab701923602
    Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304241
    Reviewed-by: Harald Alvestrand <hta@webrtc.org>
    Commit-Queue: Philipp Hancke <phancke@microsoft.com>
    Reviewed-by: Henrik Boström <hbos@webrtc.org>
    Cr-Original-Commit-Position: refs/heads/main@{#40029}
    Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305420
    Commit-Queue: Harald Alvestrand <hta@webrtc.org>
    Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
    Cr-Commit-Position: refs/branch-heads/5735@{#2}
    Cr-Branched-From: df7df199abd619e75b9f1d9a7e12fc3f3f748775-refs/heads/main@{#39949}
  • Loading branch information
mfromanmoz committed Jul 14, 2023
1 parent c2f3c89 commit 22cb53a
Show file tree
Hide file tree
Showing 8 changed files with 131 additions and 14 deletions.
3 changes: 3 additions & 0 deletions third_party/libwebrtc/README.moz-ff-commit
Original file line number Diff line number Diff line change
Expand Up @@ -23493,3 +23493,6 @@ df7df199ab
# MOZ_LIBWEBRTC_SRC=/home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc MOZ_LIBWEBRTC_BRANCH=mozpatches bash dom/media/webrtc/third_party_build/fast-forward-libwebrtc.sh
# base of lastest vendoring
9d99682446
# MOZ_LIBWEBRTC_SRC=/home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc MOZ_LIBWEBRTC_BRANCH=mozpatches bash dom/media/webrtc/third_party_build/fast-forward-libwebrtc.sh
# base of lastest vendoring
ecab2a49da
2 changes: 2 additions & 0 deletions third_party/libwebrtc/README.mozilla
Original file line number Diff line number Diff line change
Expand Up @@ -15684,3 +15684,5 @@ libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc
libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2023-07-14T20:58:08.778095.
# ./mach python dom/media/webrtc/third_party_build/vendor-libwebrtc.py --from-local /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc --commit mozpatches libwebrtc
libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2023-07-14T20:59:06.341337.
# ./mach python dom/media/webrtc/third_party_build/vendor-libwebrtc.py --from-local /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc --commit mozpatches libwebrtc
libwebrtc updated from /home/mfroman/mozilla/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2023-07-14T21:00:06.469027.
2 changes: 1 addition & 1 deletion third_party/libwebrtc/pc/data_channel_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ DataChannelController::~DataChannelController() {
<< "Missing call to PrepareForShutdown?";
}

bool DataChannelController::HasDataChannelsForTest() const {
bool DataChannelController::HasDataChannels() const {
auto has_channels = [&] {
RTC_DCHECK_RUN_ON(network_thread());
return !sctp_data_channels_n_.empty();
Expand Down
5 changes: 3 additions & 2 deletions third_party/libwebrtc/pc/data_channel_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ class DataChannelController : public SctpDataChannelControllerInterface,
const InternalDataChannelInit& config);
void AllocateSctpSids(rtc::SSLRole role);

// Used by tests to check if data channels are currently tracked.
bool HasDataChannelsForTest() const;
// Check if data channels are currently tracked. Used to decide whether a
// rejected m=application section should be reoffered.
bool HasDataChannels() const;

// At some point in time, a data channel has existed.
bool HasUsedDataChannels() const;
Expand Down
6 changes: 3 additions & 3 deletions third_party/libwebrtc/pc/data_channel_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,17 @@ TEST_F(DataChannelControllerTest, CreateDataChannelEarlyRelease) {

TEST_F(DataChannelControllerTest, CreateDataChannelEarlyClose) {
DataChannelControllerForTest dcc(pc_.get());
EXPECT_FALSE(dcc.HasDataChannelsForTest());
EXPECT_FALSE(dcc.HasDataChannels());
EXPECT_FALSE(dcc.HasUsedDataChannels());
auto ret = dcc.InternalCreateDataChannelWithProxy(
"label", InternalDataChannelInit(DataChannelInit()));
ASSERT_TRUE(ret.ok());
auto channel = ret.MoveValue();
EXPECT_TRUE(dcc.HasDataChannelsForTest());
EXPECT_TRUE(dcc.HasDataChannels());
EXPECT_TRUE(dcc.HasUsedDataChannels());
channel->Close();
run_loop_.Flush();
EXPECT_FALSE(dcc.HasDataChannelsForTest());
EXPECT_FALSE(dcc.HasDataChannels());
EXPECT_TRUE(dcc.HasUsedDataChannels());
}

Expand Down
7 changes: 4 additions & 3 deletions third_party/libwebrtc/pc/peer_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1422,9 +1422,10 @@ PeerConnection::CreateDataChannelOrError(const std::string& label,

rtc::scoped_refptr<DataChannelInterface> channel = ret.MoveValue();

// Trigger the onRenegotiationNeeded event for
// the first SCTP DataChannel.
if (first_datachannel) {
// Check the onRenegotiationNeeded event (with plan-b backward compat)
if (configuration_.sdp_semantics == SdpSemantics::kUnifiedPlan ||
(configuration_.sdp_semantics == SdpSemantics::kPlanB_DEPRECATED &&
first_datachannel)) {
sdp_handler_->UpdateNegotiationNeeded();
}
NoteUsageEvent(UsageEvent::DATA_ADDED);
Expand Down
40 changes: 35 additions & 5 deletions third_party/libwebrtc/pc/sdp_offer_answer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3342,8 +3342,20 @@ bool SdpOfferAnswerHandler::CheckIfNegotiationIsNeeded() {
// 4. If connection has created any RTCDataChannels, and no m= section in
// description has been negotiated yet for data, return true.
if (data_channel_controller()->HasUsedDataChannels()) {
if (!cricket::GetFirstDataContent(description->description()->contents()))
const cricket::ContentInfo* data_content =
cricket::GetFirstDataContent(description->description()->contents());
if (!data_content) {
return true;
}
// The remote end might have rejected the data content.
const cricket::ContentInfo* remote_data_content =
current_remote_description()
? current_remote_description()->description()->GetContentByName(
data_content->name)
: nullptr;
if (remote_data_content && remote_data_content->rejected) {
return true;
}
}
if (!ConfiguredForMedia()) {
return false;
Expand Down Expand Up @@ -4263,10 +4275,28 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanOffer(
}
// 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()));
if (!pc_->GetDataMid() && data_channel_controller()->HasUsedDataChannels() &&
data_channel_controller()->HasDataChannels()) {
// Attempt to recycle a stopped m-line.
// TODO(crbug.com/1442604): GetDataMid() should return the mid if one was
// ever created but rejected.
bool recycled = false;
for (size_t i = 0; i < session_options->media_description_options.size();
i++) {
auto media_description = session_options->media_description_options[i];
if (media_description.type == cricket::MEDIA_TYPE_DATA &&
media_description.stopped) {
session_options->media_description_options[i] =
GetMediaDescriptionOptionsForActiveData(media_description.mid);
recycled = true;
break;
}
}
if (!recycled) {
session_options->media_description_options.push_back(
GetMediaDescriptionOptionsForActiveData(
mid_generator_.GenerateString()));
}
}
}

Expand Down
80 changes: 80 additions & 0 deletions third_party/libwebrtc/pc/sdp_offer_answer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "modules/audio_processing/include/audio_processing.h"
#include "p2p/base/port_allocator.h"
#include "pc/peer_connection_wrapper.h"
#include "pc/session_description.h"
#include "pc/test/fake_audio_capture_module.h"
#include "pc/test/mock_peer_connection_observers.h"
#include "rtc_base/rtc_certificate_generator.h"
Expand Down Expand Up @@ -467,4 +468,83 @@ TEST_F(SdpOfferAnswerTest, RollbackPreservesAddTrackMid) {
EXPECT_EQ(saved_mid, first_transceiver->mid());
}

#ifdef WEBRTC_HAVE_SCTP

TEST_F(SdpOfferAnswerTest, RejectedDataChannelsDoNotGetReoffered) {
auto pc = CreatePeerConnection();
EXPECT_TRUE(pc->pc()->CreateDataChannelOrError("dc", nullptr).ok());
EXPECT_TRUE(pc->CreateOfferAndSetAsLocal());
auto mid = pc->pc()->local_description()->description()->contents()[0].mid();

// An answer that rejects the datachannel content.
std::string sdp =
"v=0\r\n"
"o=- 4131505339648218884 3 IN IP4 **-----**\r\n"
"s=-\r\n"
"t=0 0\r\n"
"a=ice-ufrag:zGWFZ+fVXDeN6UoI/136\r\n"
"a=ice-pwd:9AUNgUqRNI5LSIrC1qFD2iTR\r\n"
"a=fingerprint:sha-256 "
"AD:52:52:E0:B1:37:34:21:0E:15:8E:B7:56:56:7B:B4:39:0E:6D:1C:F5:84:A7:EE:"
"B5:27:3E:30:B1:7D:69:42\r\n"
"a=setup:passive\r\n"
"m=application 0 UDP/DTLS/SCTP webrtc-datachannel\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=sctp-port:5000\r\n"
"a=max-message-size:262144\r\n"
"a=mid:" +
mid + "\r\n";
auto answer = CreateSessionDescription(SdpType::kAnswer, sdp);
ASSERT_TRUE(pc->SetRemoteDescription(std::move(answer)));
// The subsequent offer should not recycle the m-line since the existing data
// channel is closed.
auto offer = pc->CreateOffer();
const auto& offer_contents = offer->description()->contents();
ASSERT_EQ(offer_contents.size(), 1u);
EXPECT_EQ(offer_contents[0].mid(), mid);
EXPECT_EQ(offer_contents[0].rejected, true);
}

TEST_F(SdpOfferAnswerTest, RejectedDataChannelsDoGetReofferedWhenActive) {
auto pc = CreatePeerConnection();
EXPECT_TRUE(pc->pc()->CreateDataChannelOrError("dc", nullptr).ok());
EXPECT_TRUE(pc->CreateOfferAndSetAsLocal());
auto mid = pc->pc()->local_description()->description()->contents()[0].mid();

// An answer that rejects the datachannel content.
std::string sdp =
"v=0\r\n"
"o=- 4131505339648218884 3 IN IP4 **-----**\r\n"
"s=-\r\n"
"t=0 0\r\n"
"a=ice-ufrag:zGWFZ+fVXDeN6UoI/136\r\n"
"a=ice-pwd:9AUNgUqRNI5LSIrC1qFD2iTR\r\n"
"a=fingerprint:sha-256 "
"AD:52:52:E0:B1:37:34:21:0E:15:8E:B7:56:56:7B:B4:39:0E:6D:1C:F5:84:A7:EE:"
"B5:27:3E:30:B1:7D:69:42\r\n"
"a=setup:passive\r\n"
"m=application 0 UDP/DTLS/SCTP webrtc-datachannel\r\n"
"c=IN IP4 0.0.0.0\r\n"
"a=sctp-port:5000\r\n"
"a=max-message-size:262144\r\n"
"a=mid:" +
mid + "\r\n";
auto answer = CreateSessionDescription(SdpType::kAnswer, sdp);
ASSERT_TRUE(pc->SetRemoteDescription(std::move(answer)));

// The subsequent offer should recycle the m-line when there is a new data
// channel.
EXPECT_TRUE(pc->pc()->CreateDataChannelOrError("dc2", nullptr).ok());
EXPECT_TRUE(pc->pc()->ShouldFireNegotiationNeededEvent(
pc->observer()->latest_negotiation_needed_event()));

auto offer = pc->CreateOffer();
const auto& offer_contents = offer->description()->contents();
ASSERT_EQ(offer_contents.size(), 1u);
EXPECT_EQ(offer_contents[0].mid(), mid);
EXPECT_EQ(offer_contents[0].rejected, false);
}

#endif // WEBRTC_HAVE_SCTP

} // namespace webrtc

0 comments on commit 22cb53a

Please sign in to comment.