diff --git a/net/dcsctp/tx/rr_send_queue.cc b/net/dcsctp/tx/rr_send_queue.cc index 7f42f3d9b1..b3e695bc5c 100644 --- a/net/dcsctp/tx/rr_send_queue.cc +++ b/net/dcsctp/tx/rr_send_queue.cc @@ -136,19 +136,13 @@ void RRSendQueue::OutgoingStream::Add(DcSctpMessage message, RTC_DCHECK(IsConsistent()); } -absl::optional RRSendQueue::OutgoingStream::Produce( - TimeMs now, - size_t max_size) { +SendQueue::DataToSend RRSendQueue::OutgoingStream::Produce(TimeMs now, + size_t max_size) { RTC_DCHECK(!items_.empty()); Item* item = &items_.front(); DcSctpMessage& message = item->message; - if (item->remaining_size > max_size && max_size < kMinimumFragmentedPayload) { - RTC_DCHECK(IsConsistent()); - return absl::nullopt; - } - // Allocate Message ID and SSN when the first fragment is sent. if (!item->message_id.has_value()) { MID& mid = @@ -354,22 +348,20 @@ absl::optional RRSendQueue::Produce(TimeMs now, RTC_DCHECK(stream_it != streams_.end()); } - absl::optional data = stream_it->second.Produce(now, max_size); - if (data.has_value()) { - RTC_DLOG(LS_VERBOSE) << log_prefix_ << "Producing DATA, type=" - << (data->data.is_unordered ? "unordered" : "ordered") - << "::" - << (*data->data.is_beginning && *data->data.is_end - ? "complete" - : *data->data.is_beginning - ? "first" - : *data->data.is_end ? "last" : "middle") - << ", stream_id=" << *stream_it->first - << ", ppid=" << *data->data.ppid - << ", length=" << data->data.payload.size(); + DataToSend data = stream_it->second.Produce(now, max_size); + RTC_DLOG(LS_VERBOSE) << log_prefix_ << "Producing DATA, type=" + << (data.data.is_unordered ? "unordered" : "ordered") + << "::" + << (*data.data.is_beginning && *data.data.is_end + ? "complete" + : *data.data.is_beginning + ? "first" + : *data.data.is_end ? "last" : "middle") + << ", stream_id=" << *stream_it->first + << ", ppid=" << *data.data.ppid + << ", length=" << data.data.payload.size(); - previous_message_has_ended_ = *data->data.is_end; - } + previous_message_has_ended_ = *data.data.is_end; RTC_DCHECK(IsConsistent()); return data; diff --git a/net/dcsctp/tx/rr_send_queue.h b/net/dcsctp/tx/rr_send_queue.h index fecb6e0f2e..6da585df85 100644 --- a/net/dcsctp/tx/rr_send_queue.h +++ b/net/dcsctp/tx/rr_send_queue.h @@ -40,9 +40,6 @@ namespace dcsctp { // established, this send queue is always present - even for closed connections. class RRSendQueue : public SendQueue { public: - // How small a data chunk's payload may be, if having to fragment a message. - static constexpr size_t kMinimumFragmentedPayload = 10; - RRSendQueue(absl::string_view log_prefix, size_t buffer_size, std::function on_buffered_amount_low, @@ -126,8 +123,9 @@ class RRSendQueue : public SendQueue { TimeMs expires_at, const SendOptions& send_options); - // Possibly produces a data chunk to send. - absl::optional Produce(TimeMs now, size_t max_size); + // Produces a data chunk to send. This is only called on streams that have + // data available. + DataToSend Produce(TimeMs now, size_t max_size); const ThresholdWatcher& buffered_amount() const { return buffered_amount_; } ThresholdWatcher& buffered_amount() { return buffered_amount_; } diff --git a/net/dcsctp/tx/rr_send_queue_test.cc b/net/dcsctp/tx/rr_send_queue_test.cc index 78f7616766..a93c4a3d1c 100644 --- a/net/dcsctp/tx/rr_send_queue_test.cc +++ b/net/dcsctp/tx/rr_send_queue_test.cc @@ -154,35 +154,6 @@ TEST_F(RRSendQueueTest, BufferBecomesFullAndEmptied) { EXPECT_TRUE(buf_.IsEmpty()); } -TEST_F(RRSendQueueTest, WillNotSendTooSmallPacket) { - std::vector payload(RRSendQueue::kMinimumFragmentedPayload + 1); - buf_.Add(kNow, DcSctpMessage(kStreamID, kPPID, payload)); - - // Wouldn't fit enough payload (wouldn't want to fragment) - EXPECT_FALSE( - buf_.Produce(kNow, - /*max_size=*/RRSendQueue::kMinimumFragmentedPayload - 1) - .has_value()); - - // Minimum fragment - absl::optional chunk_one = - buf_.Produce(kNow, - /*max_size=*/RRSendQueue::kMinimumFragmentedPayload); - ASSERT_TRUE(chunk_one.has_value()); - EXPECT_EQ(chunk_one->data.stream_id, kStreamID); - EXPECT_EQ(chunk_one->data.ppid, kPPID); - - // There is only one byte remaining - it can be fetched as it doesn't require - // additional fragmentation. - absl::optional chunk_two = - buf_.Produce(kNow, /*max_size=*/1); - ASSERT_TRUE(chunk_two.has_value()); - EXPECT_EQ(chunk_two->data.stream_id, kStreamID); - EXPECT_EQ(chunk_two->data.ppid, kPPID); - - EXPECT_TRUE(buf_.IsEmpty()); -} - TEST_F(RRSendQueueTest, DefaultsToOrderedSend) { std::vector payload(20); @@ -774,40 +745,5 @@ TEST_F(RRSendQueueTest, WillStayInAStreamAsLongAsThatMessageIsSending) { EXPECT_FALSE(buf_.Produce(kNow, kOneFragmentPacketSize).has_value()); } - -TEST_F(RRSendQueueTest, WillStayInStreamWhenOnlySmallFragmentRemaining) { - buf_.Add(kNow, - DcSctpMessage(StreamID(5), kPPID, - std::vector(kOneFragmentPacketSize * 2))); - buf_.Add(kNow, DcSctpMessage(StreamID(6), kPPID, std::vector(1))); - - ASSERT_HAS_VALUE_AND_ASSIGN(SendQueue::DataToSend chunk1, - buf_.Produce(kNow, kOneFragmentPacketSize)); - EXPECT_EQ(chunk1.data.stream_id, StreamID(5)); - EXPECT_THAT(chunk1.data.payload, SizeIs(kOneFragmentPacketSize)); - - // Now assume that there will be a lot of previous chunks that need to be - // retransmitted, which fills up the next packet and there is little space - // left in the packet for new chunks. What it should NOT do right now is to - // try to send a message from StreamID 6. And it should not try to send a very - // small fragment from StreamID 5 either. So just skip this one. - EXPECT_FALSE(buf_.Produce(kNow, 8).has_value()); - - // When the next produce request comes with a large buffer to fill, continue - // sending from StreamID 5. - - ASSERT_HAS_VALUE_AND_ASSIGN(SendQueue::DataToSend chunk2, - buf_.Produce(kNow, kOneFragmentPacketSize)); - EXPECT_EQ(chunk2.data.stream_id, StreamID(5)); - EXPECT_THAT(chunk2.data.payload, SizeIs(kOneFragmentPacketSize)); - - // Lastly, produce a message on StreamID 6. - ASSERT_HAS_VALUE_AND_ASSIGN(SendQueue::DataToSend chunk3, - buf_.Produce(kNow, kOneFragmentPacketSize)); - EXPECT_EQ(chunk3.data.stream_id, StreamID(6)); - EXPECT_THAT(chunk3.data.payload, SizeIs(1)); - - EXPECT_FALSE(buf_.Produce(kNow, 8).has_value()); -} } // namespace } // namespace dcsctp