diff --git a/api/data_channel_interface.h b/api/data_channel_interface.h index a02bc5e0d4..4f74918ff9 100644 --- a/api/data_channel_interface.h +++ b/api/data_channel_interface.h @@ -183,12 +183,10 @@ class RTC_EXPORT DataChannelInterface : public rtc::RefCountInterface { // Sends `data` to the remote peer. If the data can't be sent at the SCTP // level (due to congestion control), it's buffered at the data channel level, - // up to a maximum of MaxSendQueueSize(). If Send is called while this buffer - // is full, the data channel will be closed abruptly. - // - // So, it's important to use buffered_amount() and OnBufferedAmountChange to - // ensure the data channel is used efficiently but without filling this - // buffer. + // up to a maximum of MaxSendQueueSize(). + // Returns false if the data channel is not in open state or if the send + // buffer is full. + // TODO(webrtc:13289): Return an RTCError with information about the failure. virtual bool Send(const DataBuffer& buffer) = 0; // Amount of bytes that can be queued for sending on the data channel. diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc index 770892cbe1..44c080bbda 100644 --- a/pc/data_channel_unittest.cc +++ b/pc/data_channel_unittest.cc @@ -544,23 +544,29 @@ TEST_F(SctpDataChannelTest, OpenAckRoleInitialization) { EXPECT_EQ(webrtc::InternalDataChannelInit::kNone, init2.open_handshake_role); } -// Tests that the DataChannel is closed if the sending buffer is full. -TEST_F(SctpDataChannelTest, ClosedWhenSendBufferFull) { +// Tests that that Send() returns false if the sending buffer is full +// and the channel stays open. +TEST_F(SctpDataChannelTest, OpenWhenSendBufferFull) { SetChannelReady(); - rtc::CopyOnWriteBuffer buffer(1024); + const size_t packetSize = 1024; + + rtc::CopyOnWriteBuffer buffer(packetSize); memset(buffer.MutableData(), 0, buffer.size()); webrtc::DataBuffer packet(buffer, true); provider_->set_send_blocked(true); - for (size_t i = 0; i < 16 * 1024 + 1; ++i) { + for (size_t i = 0; + i < webrtc::DataChannelInterface::MaxSendQueueSize() / packetSize; ++i) { EXPECT_TRUE(webrtc_data_channel_->Send(packet)); } - EXPECT_TRUE( - webrtc::DataChannelInterface::kClosed == webrtc_data_channel_->state() || - webrtc::DataChannelInterface::kClosing == webrtc_data_channel_->state()); + // The sending buffer shoul be full, send returns false. + EXPECT_FALSE(webrtc_data_channel_->Send(packet)); + + EXPECT_TRUE(webrtc::DataChannelInterface::kOpen == + webrtc_data_channel_->state()); } // Tests that the DataChannel is closed on transport errors. diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc index f59dd1ec32..c63f820acf 100644 --- a/pc/sctp_data_channel.cc +++ b/pc/sctp_data_channel.cc @@ -309,12 +309,8 @@ bool SctpDataChannel::Send(const DataBuffer& buffer) { // so just add to the end of the queue and keep waiting. if (!queued_send_data_.Empty()) { if (!QueueSendDataMessage(buffer)) { - RTC_LOG(LS_ERROR) << "Closing the DataChannel due to a failure to queue " - "additional data."; - // https://w3c.github.io/webrtc-pc/#dom-rtcdatachannel-send step 5 - // Note that the spec doesn't explicitly say to close in this situation. - CloseAbruptlyWithError(RTCError(RTCErrorType::RESOURCE_EXHAUSTED, - "Unable to queue data for sending")); + // Queue is full + return false; } return true; }