From 1f9e6046dd1bb3e56552203dc06454dec238e495 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Tue, 21 Jan 2025 15:11:57 +0000 Subject: [PATCH] Start deprecation process for non-Optional datachannel parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old version of these returns -1 when the value is not set. Optional is better. Bug: webrtc:42220231 Change-Id: Ideb0f51fd8bb7b5aa490743eb3b5d95998efbd1f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374483 Reviewed-by: Henrik Boström Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#43786} --- api/data_channel_interface.h | 6 ++++-- pc/data_channel_integrationtest.cc | 3 ++- pc/data_channel_unittest.cc | 10 ++++++---- pc/sctp_data_channel.cc | 4 ++++ sdk/objc/api/peerconnection/RTCDataChannel.mm | 12 ++++++++++-- 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/api/data_channel_interface.h b/api/data_channel_interface.h index ea463dc781..9265f6628c 100644 --- a/api/data_channel_interface.h +++ b/api/data_channel_interface.h @@ -161,8 +161,10 @@ class RTC_EXPORT DataChannelInterface : public RefCountInterface { // DataChannel was created with. virtual bool ordered() const; // TODO(hta): Deprecate and remove the following two functions. - virtual uint16_t maxRetransmitTime() const; - virtual uint16_t maxRetransmits() const; + [[deprecated("Use maxPacketLifeTime")]] virtual uint16_t maxRetransmitTime() + const; + [[deprecated("Use maxRetransmitsOpt")]] virtual uint16_t maxRetransmits() + const; virtual std::optional maxRetransmitsOpt() const; virtual std::optional maxPacketLifeTime() const; virtual std::string protocol() const; diff --git a/pc/data_channel_integrationtest.cc b/pc/data_channel_integrationtest.cc index e7af1bf4bf..37fc19fd8e 100644 --- a/pc/data_channel_integrationtest.cc +++ b/pc/data_channel_integrationtest.cc @@ -607,7 +607,8 @@ TEST_P(DataChannelIntegrationTest, SctpDataChannelConfigSentToOtherSide) { // 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_EQ(init.maxRetransmits, + *callee()->data_channel()->maxRetransmitsOpt()); EXPECT_FALSE(callee()->data_channel()->negotiated()); } diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc index b977b47026..00609a778f 100644 --- a/pc/data_channel_unittest.cc +++ b/pc/data_channel_unittest.cc @@ -170,10 +170,14 @@ TEST_F(SctpDataChannelTest, VerifyConfigurationGetters) { EXPECT_EQ(channel_->ordered(), init_.ordered); EXPECT_EQ(channel_->negotiated(), init_.negotiated); EXPECT_EQ(channel_->priority(), PriorityValue(Priority::kLow)); - EXPECT_EQ(channel_->maxRetransmitTime(), static_cast(-1)); EXPECT_EQ(channel_->maxPacketLifeTime(), init_.maxRetransmitTime); - EXPECT_EQ(channel_->maxRetransmits(), static_cast(-1)); EXPECT_EQ(channel_->maxRetransmitsOpt(), init_.maxRetransmits); + // TODO: issues.webrtc.org/42220231 - remove when deprecation done +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + EXPECT_EQ(channel_->maxRetransmitTime(), static_cast(-1)); + EXPECT_EQ(channel_->maxRetransmits(), static_cast(-1)); +#pragma clang diagnostic pop // Check the non-const part of the configuration. EXPECT_EQ(channel_->id(), init_.id); @@ -742,8 +746,6 @@ class NoImplObserver : public DataChannelObserver { TEST(DataChannelInterfaceTest, Coverage) { auto channel = rtc::make_ref_counted(); EXPECT_FALSE(channel->ordered()); - EXPECT_EQ(channel->maxRetransmitTime(), 0u); - EXPECT_EQ(channel->maxRetransmits(), 0u); EXPECT_FALSE(channel->maxRetransmitsOpt()); EXPECT_FALSE(channel->maxPacketLifeTime()); EXPECT_TRUE(channel->protocol().empty()); diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc index bd8d3fa20c..278f7301ed 100644 --- a/pc/sctp_data_channel.cc +++ b/pc/sctp_data_channel.cc @@ -43,8 +43,12 @@ BYPASS_PROXY_METHOD0(void, UnregisterObserver) BYPASS_PROXY_CONSTMETHOD0(std::string, label) BYPASS_PROXY_CONSTMETHOD0(bool, reliable) BYPASS_PROXY_CONSTMETHOD0(bool, ordered) +// TODO: issues.webrtc.org/42220231 - remove when deprecation done +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" BYPASS_PROXY_CONSTMETHOD0(uint16_t, maxRetransmitTime) BYPASS_PROXY_CONSTMETHOD0(uint16_t, maxRetransmits) +#pragma clang diagnostic pop BYPASS_PROXY_CONSTMETHOD0(std::optional, maxRetransmitsOpt) BYPASS_PROXY_CONSTMETHOD0(std::optional, maxPacketLifeTime) BYPASS_PROXY_CONSTMETHOD0(std::string, protocol) diff --git a/sdk/objc/api/peerconnection/RTCDataChannel.mm b/sdk/objc/api/peerconnection/RTCDataChannel.mm index 7d0aaa82a3..ae91cda491 100644 --- a/sdk/objc/api/peerconnection/RTCDataChannel.mm +++ b/sdk/objc/api/peerconnection/RTCDataChannel.mm @@ -118,11 +118,19 @@ class DataChannelDelegateAdapter : public DataChannelObserver { } - (uint16_t)maxPacketLifeTime { - return _nativeDataChannel->maxRetransmitTime(); + // Emulate deprecated API that will be removed. + if (_nativeDataChannel->maxPacketLifeTime()) { + return *_nativeDataChannel->maxPacketLifeTime(); + } + return -1; } - (uint16_t)maxRetransmits { - return _nativeDataChannel->maxRetransmits(); + // Emulate deprecated API that will be removed. + if (_nativeDataChannel->maxRetransmitsOpt()) { + return *_nativeDataChannel->maxRetransmitsOpt(); + } + return -1; } - (NSString *)protocol {