commit 2c1cfd047f891f3558e5114d89da292c12dacc57
author Victor Boivie <boivie@webrtc.org> Mon Mar 18 12:51:40 2024
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Mar 22 09:25:11 2024
tree 3f0065a70e111d82eb37ca38d9cf84b865e0f935
parent 22b902eea49aa002bde52e01ba611e901c68d5c6

pc: Remove additional buffering in SctpDataChannel

This CL removes the send buffers (but not the receive buffer) from
SctpDataChannel and increases the send buffer in DcSctpSocket instead.

The reasons are:
 1) Simplify the code. This additional buffering was strictly needed
    before we migrated away from usrsctp, as that send buffer was very
    limited in size (by design). But with the migration to dcSCTP, it's
    no longer needed, so it just adds complexity.
 2) Make `RTCDataChannel::bufferedAmount` correct. Before this CL, it
    represented just the data buffered in SctpDataChannel, and not the
    data accepted by the SCTP socket, but not yet put on the wire. This
    makes it hard for clients to know when a message has ever been sent.
 3) Better handle draining data on data channel close. While this is not
    implemented in dcSCTP, having a single buffer makes this easier to
    add.

While most of this CL is straightforward, the handling of bufferedAmount
in the signaling thread (in RTCDataChannel in Blink), is a bit special.
The number returned by `RTCDataChannel::bufferedAmount` is not what the
true value is inside the SCTP socket, but an eventual consistent view
of that value. When a message is sent, the value is incremented and:
  - Before this change: When a message was put on the SCTP socket, the
    view's value was decremented. Which made the view reflect what was
    buffered outside the SCTP socket, and that buffering is now gone.
  - After this change: SctpDataChannel will track what RTCDataChannel
    will think it is, and provide updates to that number as we are
    notified that it's reduced - by setting a "low threshold" callback
    trigger.

A bonus with the new behavior is that it will be eventually consistent
and auto-heal also in error conditions - when messages are dropped due
to errors (bad input, bad state, etc). Previously, the bufferedAmount
value could drift away from the correct value on errors.

Note that a big chunk of unit tests were removed with this CL, as those
tested how the buffering behaved. Now, there is no buffering, so the
removed test cases represent a simpler interface.

This CL has been extensively tested with data channel benchmarks that
use the bufferedAmount thresholds (in Javascript).

Bug: chromium:40072842
Change-Id: I1a6a4af6b6e1116832f5028f989ce9f44683d229
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343361
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41945}