From 5c4d2ee615eea6651d2d35fc1ec4ea03bc63ef02 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Mon, 1 Apr 2019 12:58:15 +0200 Subject: [PATCH] RTCDataChannel: Ignore "id" when "negotiated" is false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This updates behavior to be aligned with the WebRTC spec. Bug: chromium:948055 Change-Id: Id3bbf05b3df084c9b7f7d12598c09187679d60fc Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/130493 Reviewed-by: Henrik Boström Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#27394} --- pc/data_channel.h | 4 ++++ pc/peer_connection_integrationtest.cc | 6 ++++-- pc/peer_connection_interface_unittest.cc | 3 +++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/pc/data_channel.h b/pc/data_channel.h index eef127928f..3e58c2b020 100644 --- a/pc/data_channel.h +++ b/pc/data_channel.h @@ -62,6 +62,10 @@ struct InternalDataChannelInit : public DataChannelInit { // If the channel is externally negotiated, do not send the OPEN message. if (base.negotiated) { open_handshake_role = kNone; + } else { + // Datachannel is externally negotiated. Ignore the id value. + // Specified in createDataChannel, WebRTC spec section 6.1 bullet 13. + id = -1; } } diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 33aa038136..218a8bb50f 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -3320,7 +3320,8 @@ TEST_P(PeerConnectionIntegrationTest, SctpDataChannelConfigSentToOtherSide) { ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); ASSERT_TRUE_WAIT(callee()->data_channel() != nullptr, kDefaultTimeout); ASSERT_TRUE_WAIT(callee()->data_observer()->IsOpen(), kDefaultTimeout); - EXPECT_EQ(init.id, callee()->data_channel()->id()); + // Since "negotiated" is false, the "id" parameter should be ignored. + EXPECT_NE(init.id, callee()->data_channel()->id()); EXPECT_EQ("data-channel", callee()->data_channel()->label()); EXPECT_EQ(init.maxRetransmits, callee()->data_channel()->maxRetransmits()); EXPECT_FALSE(callee()->data_channel()->negotiated()); @@ -3576,7 +3577,8 @@ TEST_P(PeerConnectionIntegrationTest, // configuration. ASSERT_TRUE_WAIT(callee()->data_channel() != nullptr, kDefaultTimeout); ASSERT_TRUE_WAIT(callee()->data_observer()->IsOpen(), kDefaultTimeout); - EXPECT_EQ(init.id, callee()->data_channel()->id()); + // Since "negotiate" is false, the "id" parameter is ignored. + EXPECT_NE(init.id, callee()->data_channel()->id()); EXPECT_EQ("data-channel", callee()->data_channel()->label()); EXPECT_EQ(init.maxRetransmits, callee()->data_channel()->maxRetransmits()); EXPECT_FALSE(callee()->data_channel()->negotiated()); diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc index 90c520817b..371cdc150d 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -2141,6 +2141,7 @@ TEST_P(PeerConnectionInterfaceTest, rtc::scoped_refptr channel; config.id = 1; + config.negotiated = true; channel = pc_->CreateDataChannel("1", &config); EXPECT_TRUE(channel != NULL); EXPECT_EQ(1, channel->id()); @@ -2149,11 +2150,13 @@ TEST_P(PeerConnectionInterfaceTest, EXPECT_TRUE(channel == NULL); config.id = cricket::kMaxSctpSid; + config.negotiated = true; channel = pc_->CreateDataChannel("max", &config); EXPECT_TRUE(channel != NULL); EXPECT_EQ(config.id, channel->id()); config.id = cricket::kMaxSctpSid + 1; + config.negotiated = true; channel = pc_->CreateDataChannel("x", &config); EXPECT_TRUE(channel == NULL); }