Commit graph

77 commits

Author SHA1 Message Date
Florent Castelli
8037fc6ffa Migrate absl::optional to std::optional
Bug: webrtc:342905193
No-Try: True
Change-Id: Icc968be43b8830038ea9a1f5f604307220457807
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361021
Auto-Submit: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42911}
2024-09-02 12:16:47 +00:00
Florent Castelli
0012bfa128 Change DataChannelInit::priority to integer and forward to SCTP transport
The new type PriorityValue is a strong 16-bit integer matching RFC 8831
requirements that can be built from a Priority enum.
The value is now propagated and used by the SCTP transport, but enabling
the feature still requires a field trial for now.

Bug: webrtc:42225365
Change-Id: I56c9f48744c70999a8c2d01415a08a0b6761df4b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/357941
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42695}
2024-07-30 15:07:25 +00:00
Victor Boivie
cdecc4e6df Expose bufferedAmountLowThreshold
This code was extracted to make the next following CL easier to review.

This CL simply exposes the getters, setters and callbacks to set the
buffered amount low threshold on a specific SCTP stream. It will be
used in a follow-up CL, but is just boilerplate.

Bug: chromium:40072842
Change-Id: Iccd72208b369ddc252cc5886f6446b9c2ceeb0b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343360
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41943}
2024-03-21 19:59:39 +00:00
Victor Boivie
fea41f540c pc: Include SCTP queued bytes in buffered_amount
Before this change, calling buffered_amount only included what was
buffered on top of what was already buffered in the SCTP socket. With
the defaults, the SCTP socket can buffer up to 2MB of data (that is not
put on the wire) before the additional external bufferering in
SctpDataChannel will be used. The buffering that I am working on
removing completely.

Until it's removed completely, to avoid the issue reported in
crbug.com/41221056, include the bytes buffered in the SCTP socket to
what is returned when calling RTCDataChannel::buffered_amount.

This means that when this value is zero, it can be safe to know that all
bytes have been sent, but not necessarily acknowledged. And calling
close will not discard any messages.

This is a stopgap solution, but as functional as the proper solution
that removes all additional buffering. Follow-up CLs will merely improve
this solution.

Bug: chromium:41221056
Change-Id: I06edd52188d3bf13a17827381a15a4730722685a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342520
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41898}
2024-03-13 15:44:17 +00:00
Victor Boivie
cd3d29b6fb pc: Simplify StreamId class
Before this CL, the StreamId class represented either a valid SCTP
stream ID, or "nothing", which means that it was a wrapped
absl::optional. Since created data channels don't have a SCTP stream ID
until it's known whether this peer will use odd or even numbers, the
"nothing" value was used for that state.

This unfortunately made it a bit hard to work with objects of this type,
as one always had to check if it contained a value. And even if a caller
would check this, and then pass the StreamId to a different function,
that function would have to do the check itself (often as a RTC_DCHECK)
since the passed StreamId always could have that state.

This CL simply extracts the "absl::optional" part of it, forcing holders
to wrap it in an optional type - when it can be "nothing". But allowing
the other code to just pass StreamId that can't be "nothing". That
simplifies the code a bit, potentially removing some bugs.

Bug: chromium:41221056
Change-Id: I93104cdd5d2f5fc1dbeb9d9dfc4cf361f11a9d68
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342440
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41880}
2024-03-12 10:57:56 +00:00
Tommi
eec1810760 Avoid touching channel after OnSctpDataChannelClosed
Bug: chromium:1454086
Change-Id: I39573b706c4031d091c45a182b13cb3b2dba6233
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/309920
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40332}
2023-06-22 09:13:29 +00:00
Tommi
44ebe2abc9 Refactor HasDataChannels
Follow-up to: https://webrtc-review.googlesource.com/c/src/+/304241

This changes `HasDataChannels()` to not block on the network thread.

Bug: chromium:1442604
Change-Id: I880e3ed554bc4265f675fb2aa48351a7f42ef9bb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304961
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40068}
2023-05-15 14:52:12 +00:00
Philipp Hancke
522380ff73 Attempt to recycle a stopped data m-line before creating a new one
which avoids an infinitely growing SDP if the remote end rejects
the datachannel section. This will reactivate the m-line even if
all datachannels are closed.

BUG=chromium:1442604

Change-Id: If60f93b406271163df692d96102baab701923602
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304241
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40029}
2023-05-09 15:11:24 +00:00
Tommi
aa3c9f2972 Reland "Add param to DCC::SetupDataChannelTransport_n, simplify DCC* setup code."
This reverts commit 298313534d.

Changes from the original commit:
* Call OnTransportClosed() from TeardownDataChannelTransport_n()
  (same as before the original commit)
* Not call OnTransportClosed() from OnTransportChanged() when its
  called with nullptr (also preserving the behaviour from before
  the original commit).

Original change's description:
> Revert "Add param to DCC::SetupDataChannelTransport_n, simplify DCC* setup code."
>
> This reverts commit 2ec6a6c578.
>
> Reason for revert: It breaks WPT tests (e.g. https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/1361972/overview) blocking the roll into Chromium.
>
> Original change's description:
> > Add param to DCC::SetupDataChannelTransport_n, simplify DCC* setup code.
> >
> > * DCC = DataChannelController.
> >
> > * Consolidate steps to set the mid and transport name. They're now
> >   set at the same time and without a separate PostTask.
> > * Transport sink is now consistently set in DCC
> > * Order of notifications for setting up the transport is now the same
> >   regardless of the first time the transport is being set or if it's
> >   being replaced.
> > * Made set_data_channel_transport() private.
> >
> > Bug: webrtc:11547
> > Change-Id: I39e89c6e269e6f06d55981d7944678bf23c8817a
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300562
> > Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> > Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#39859}
>
> Bug: webrtc:11547
> Change-Id: I0d8d7453b71be80fbf1b7eba7d161336e29de091
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301360
> Auto-Submit: Mirko Bonadei <mbonadei@webrtc.org>
> Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#39864}

Bug: webrtc:11547
Change-Id: I8ebbc3d3a12786dff2096350a77e03e98466ff00
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301702
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39884}
2023-04-18 12:12:52 +00:00
Mirko Bonadei
298313534d Revert "Add param to DCC::SetupDataChannelTransport_n, simplify DCC* setup code."
This reverts commit 2ec6a6c578.

Reason for revert: It breaks WPT tests (e.g. https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/1361972/overview) blocking the roll into Chromium.

Original change's description:
> Add param to DCC::SetupDataChannelTransport_n, simplify DCC* setup code.
>
> * DCC = DataChannelController.
>
> * Consolidate steps to set the mid and transport name. They're now
>   set at the same time and without a separate PostTask.
> * Transport sink is now consistently set in DCC
> * Order of notifications for setting up the transport is now the same
>   regardless of the first time the transport is being set or if it's
>   being replaced.
> * Made set_data_channel_transport() private.
>
> Bug: webrtc:11547
> Change-Id: I39e89c6e269e6f06d55981d7944678bf23c8817a
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300562
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39859}

Bug: webrtc:11547
Change-Id: I0d8d7453b71be80fbf1b7eba7d161336e29de091
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301360
Auto-Submit: Mirko Bonadei <mbonadei@webrtc.org>
Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#39864}
2023-04-14 17:04:44 +00:00
Tommi
2ec6a6c578 Add param to DCC::SetupDataChannelTransport_n, simplify DCC* setup code.
* DCC = DataChannelController.

* Consolidate steps to set the mid and transport name. They're now
  set at the same time and without a separate PostTask.
* Transport sink is now consistently set in DCC
* Order of notifications for setting up the transport is now the same
  regardless of the first time the transport is being set or if it's
  being replaced.
* Made set_data_channel_transport() private.

Bug: webrtc:11547
Change-Id: I39e89c6e269e6f06d55981d7944678bf23c8817a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300562
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39859}
2023-04-14 06:57:51 +00:00
Tommi
b00d63c88b Merge TeardownDataChannelTransport_n and OnTransportChannelClosed.
This consolidates termination logic in the DataChannelController
to make shut down consistent between when the transport notifies
of termination and when termination is initiated from the PC side.

This removes the need for `OnTransportChannelClosed` from the PC
side since we can just use TeardownDataChannelTransport_n (the two
were always being called together).

Bug: webrtc:11547
Change-Id: I1763f82cbfe1a3d5b8bfabb8d4cba0ee0fa95738
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300561
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39838}
2023-04-13 07:32:23 +00:00
Tommi
add7ac0ded Remove calls to data_channel_transport() from the wrong thread.
Applying thread guards and removing the accessor that was being
called from the wrong context.

Bug: webrtc:11547, webrtc:9987
Change-Id: I80953aab48e5d155fc9d101526a3fa1f2704c39f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300544
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39832}
2023-04-12 17:28:28 +00:00
Tommi
f9e13f8813 Reland "[DataChannel] Send and receive packets on the network thread."
This reverts commit 7f16fcda0f.

Reason for reland: Re-landing after addressing issues in downstream
code and hardening the ObserverAdapter from situations where attempted
usage of data channel proxies could occur after shutting down the
peer connection and terminating the network thread.

Original change's description:
> Revert "[DataChannel] Send and receive packets on the network thread."
>
> This reverts commit fe53fec24e.
>
> Reason for revert: Speculative revert, may be breaking downstream project
>
> Original change's description:
> > [DataChannel] Send and receive packets on the network thread.
> >
> > This updates sctp channels, including work that happens between the
> > data channel controller and the transport, to run on the network
> > thread. Previously all network traffic related to data channels was
> > routed through the signaling thread before going to either the network
> > thread or the caller's thread (e.g. js thread in chrome). Now the
> > calls can go straight from the network thread to the JS thread with
> > enabling a special flag on the observer (see below) and similarly
> > calls to send data, involve 2 threads instead of 3.
> >
> > * Custom data channel observer adapter implementation that
> >   maintains compatibility with existing observer implementations in
> >   that notifications are delivered on the signaling thread.
> >   The adapter can be explicitly disabled for implementations that
> >   want to optimize the callback path and promise to not block the
> >   network thread.
> > * Remove the signaling thread copy of data channels in the controller.
> > * Remove several PostTask operations that were needed to keep things
> >   in sync (but the need has gone away).
> > * Update tests for the controller to consistently call
> >   TeardownDataChannelTransport_n to match with production.
> > * Update stats collectors (current and legacy) to fetch the data
> >   channel stats on the network thread where they're maintained.
> > * Remove the AsyncChannelCloseTeardown test since the async teardown
> >   step has gone away.
> > * Remove `sid_s` in the channel code since we only need the network
> >   state now.
> > * For the custom observer support (with and without data adapter) and
> >   maintain compatibility with existing implementations, added a new
> >   proxy macro that allows an implementation to selectively provide
> >   its own implementation without being proxied. This is used for
> >   registering/unregistering a data channel observer.
> > * Update the data channel proxy to map most methods to the network
> >   thread, avoiding the interim jump to the signaling thread.
> > * Update a plethora of thread checkers from signaling to network.
> >
> > Bug: webrtc:11547
> > Change-Id: Ib4cff1482e31c46008e187189a79e967389bc518
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299142
> > Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> > Reviewed-by: Henrik Boström <hbos@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#39760}
>
> Bug: webrtc:11547
> Change-Id: Id0d65594bf727ccea5c49093c942b09714d101ad
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300341
> Auto-Submit: Andrey Logvin <landrey@webrtc.org>
> Owners-Override: Andrey Logvin <landrey@webrtc.org>
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39764}

Bug: webrtc:11547
Change-Id: I47dfa7e7168be0cd2faab4f8f3ebf110c3728af5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300360
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39786}
2023-04-07 09:04:30 +00:00
Andrey Logvin
7f16fcda0f Revert "[DataChannel] Send and receive packets on the network thread."
This reverts commit fe53fec24e.

Reason for revert: Speculative revert, may be breaking downstream project

Original change's description:
> [DataChannel] Send and receive packets on the network thread.
>
> This updates sctp channels, including work that happens between the
> data channel controller and the transport, to run on the network
> thread. Previously all network traffic related to data channels was
> routed through the signaling thread before going to either the network
> thread or the caller's thread (e.g. js thread in chrome). Now the
> calls can go straight from the network thread to the JS thread with
> enabling a special flag on the observer (see below) and similarly
> calls to send data, involve 2 threads instead of 3.
>
> * Custom data channel observer adapter implementation that
>   maintains compatibility with existing observer implementations in
>   that notifications are delivered on the signaling thread.
>   The adapter can be explicitly disabled for implementations that
>   want to optimize the callback path and promise to not block the
>   network thread.
> * Remove the signaling thread copy of data channels in the controller.
> * Remove several PostTask operations that were needed to keep things
>   in sync (but the need has gone away).
> * Update tests for the controller to consistently call
>   TeardownDataChannelTransport_n to match with production.
> * Update stats collectors (current and legacy) to fetch the data
>   channel stats on the network thread where they're maintained.
> * Remove the AsyncChannelCloseTeardown test since the async teardown
>   step has gone away.
> * Remove `sid_s` in the channel code since we only need the network
>   state now.
> * For the custom observer support (with and without data adapter) and
>   maintain compatibility with existing implementations, added a new
>   proxy macro that allows an implementation to selectively provide
>   its own implementation without being proxied. This is used for
>   registering/unregistering a data channel observer.
> * Update the data channel proxy to map most methods to the network
>   thread, avoiding the interim jump to the signaling thread.
> * Update a plethora of thread checkers from signaling to network.
>
> Bug: webrtc:11547
> Change-Id: Ib4cff1482e31c46008e187189a79e967389bc518
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299142
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39760}

Bug: webrtc:11547
Change-Id: Id0d65594bf727ccea5c49093c942b09714d101ad
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300341
Auto-Submit: Andrey Logvin <landrey@webrtc.org>
Owners-Override: Andrey Logvin <landrey@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39764}
2023-04-05 09:34:23 +00:00
Tommi
fe53fec24e [DataChannel] Send and receive packets on the network thread.
This updates sctp channels, including work that happens between the
data channel controller and the transport, to run on the network
thread. Previously all network traffic related to data channels was
routed through the signaling thread before going to either the network
thread or the caller's thread (e.g. js thread in chrome). Now the
calls can go straight from the network thread to the JS thread with
enabling a special flag on the observer (see below) and similarly
calls to send data, involve 2 threads instead of 3.

* Custom data channel observer adapter implementation that
  maintains compatibility with existing observer implementations in
  that notifications are delivered on the signaling thread.
  The adapter can be explicitly disabled for implementations that
  want to optimize the callback path and promise to not block the
  network thread.
* Remove the signaling thread copy of data channels in the controller.
* Remove several PostTask operations that were needed to keep things
  in sync (but the need has gone away).
* Update tests for the controller to consistently call
  TeardownDataChannelTransport_n to match with production.
* Update stats collectors (current and legacy) to fetch the data
  channel stats on the network thread where they're maintained.
* Remove the AsyncChannelCloseTeardown test since the async teardown
  step has gone away.
* Remove `sid_s` in the channel code since we only need the network
  state now.
* For the custom observer support (with and without data adapter) and
  maintain compatibility with existing implementations, added a new
  proxy macro that allows an implementation to selectively provide
  its own implementation without being proxied. This is used for
  registering/unregistering a data channel observer.
* Update the data channel proxy to map most methods to the network
  thread, avoiding the interim jump to the signaling thread.
* Update a plethora of thread checkers from signaling to network.

Bug: webrtc:11547
Change-Id: Ib4cff1482e31c46008e187189a79e967389bc518
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299142
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39760}
2023-04-04 16:49:17 +00:00
Tommi
527196508c Update DataChannelControllerTests to exercise teardown path.
This updates DataChannelControllerTests to shut down the DCC in the
same way it's shut down by the owning PeerConnection instance:

* Call TeardownDataChannelTransport_n()
* Call PrepareForShutdown()

Also calling PrepareForShutdown() from PC's dtor to be consistent with
how `sdp_handler_->PrepareForShutdown()` is called since it appears
that many tests do not call PC::Close() before destruction.

Bug: b/276434297
Change-Id: I0379baa0df0e764bc255b83ae0667032acfe3db0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300220
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39756}
2023-04-04 11:21:06 +00:00
Tommi
1f708ef2ff Cancel pending operations for the DCC during Pc::Close()
(using no-try due to bot infra issue)

No-try: true
Bug: b/276434297
Change-Id: I33f796b501f96731c4ca76cb62c2331f10c795f1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299708
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39734}
2023-03-31 18:13:59 +00:00
Tommi
1158bde7c2 [SctpDataChannel] Add a copy of the sid for the network thread.
* Rename id_ -> id_s_, add id_n_ and thread guards.
* Same for getters, sid() -> sid_s(), add sid_n()

As more things migrate over to the network thread, we'll only need the
_n variant.

Bug: webrtc:11547
Change-Id: Ic998330f4c81b0f6833967631ac70edc2ca2301c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299141
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39724}
2023-03-31 07:14:39 +00:00
Tommi
77158ace75 DataChannelController: Rename HasDataChannels->HasDataChannelsForTest
The reason is that:
* SDP/PeerConnection code should use HasUsedDataChannels() instead.
* HasDataChannels() touches the list of data channels on the signaling
  thread, which will be a problem when it's moved to the network
  thread.
* It's only needed by tests.

Bug: webrtc:11547
Change-Id: Idd47365c429e5f1d6e3812cf558c4e6fefbf733c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299481
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39721}
2023-03-30 09:47:54 +00:00
Tommi
4f7ade5c58 Create SctpDataChannel objects on the network thread.
* Change data channel creation code to return RTCError for more
  detailed/accurate errors.
* Move DataChannelController::sid_allocator_ to the network thread.
* Add a temporary duplicate vector of channels on the network thread.
  This will eventually be the main vector.
* Delete one test that turns out to be racy (as long as we're using
  both the signaling and network threads).

Bug: webrtc:11547, webrtc:12796
Change-Id: I93ab721a09872d075046a907df60e8aee4263371
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298624
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39719}
2023-03-29 20:27:54 +00:00
Tommi
56548988e9 Switch from pointer to ID for OnSctpDataChannelStateChanged
* The pointer isn't needed for this notification. Arguably using
  the internal id is more consistent with the stats code.
* Using the int makes it safer down the line to post the operation
  from the network thread to the signaling thread rather than post
  an object reference.

Bug: none
Change-Id: I1e9eb31d8386dca3feaa90ee3267ea98eb3e81ec
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299144
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39696}
2023-03-28 08:14:33 +00:00
Tommi
55f72800b4 [DataChannelController] Associate two methods with the network thread
Make DataChannelController's AddSctpDataStream and
RemoveSctpDataStream be required to be called on the network thread.
This moves blocking calls within those methods over to the
SctpDataChannel class instead.

For production code there's no functional change in this CL. However, this CL:

1) Introduces an actual dedicated network thread to
   DataChannelController and SctpDataChannel tests.
2) Removes two data_channel_transport() checks inside DCC that
   were being done on the wrong thread (signaling) and
3) introduces a network calling block to SctpDataChannel, where more
   network thread related work needs to be done and can be bundled.
   (to be done in follow-up CLs).

Bug: webrtc:11547
Change-Id: I6787ac395e61d4a25ae3a74a123e3357cbb46b54
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298052
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39688}
2023-03-27 16:01:47 +00:00
Tommi
335d084b3b Remove GetSctpSslRole, only use GetSctpSslRole_n
This updates DataChannelController and test classes to use
GetSctpSslRole_n instead and query the role on the network thread.

Along the way this CL makes the init config struct for when constructing
data channels, mandatory. It's now passed via const& instead of by pointer. In practice a valid pointer was always being passed.

Bug: webrtc:11547
Change-Id: I0f4bbf364969cc2dec07871c297ddbef0c175f86
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298307
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39676}
2023-03-25 11:40:35 +00:00
Tommi
c61eee28f3 Split up GetSctpSslRole to include an _n variant.
This allows the SslRole to be queried from the network thread which
will simplify some code paths and avoid thread hopping.

The next steps will be to remove GetSctpSslRole and only query the
DTLS role on the network thread and start combining other operations.

Bug: webrtc:11547
Change-Id: I222dc838fc5ee274a294c8d81d38b5a4ea8fea1f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298302
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39642}
2023-03-22 16:47:28 +00:00
Tommi
8efaec62b1 Add thread checker to SctpSidAllocator
Also changing AllocateSid to return StreamId instead of bool.

Bug: webrtc:11547
Change-Id: I776e917300ddfdbb79e78c01ef880209ec2c5917
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298301
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39630}
2023-03-22 00:28:02 +00:00
Tommi
9296a16f9d Remove SctpDataChannel::Init()
This is a small tweak to explicitly remove this second construction
step from SctpDataChannel (async call to OnTransportReady) and move
it over to DataChannelController, which is where OnTransportReady()
is called from otherwise.

Bug: webrtc:11547
Change-Id: Ie86fa85cbb79b405248f88b47d5920c7f163dba6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297921
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39627}
2023-03-21 17:20:38 +00:00
Tommi
1fabbac6b6 Update SctpTransportInternal to use RTCError.
This avoids a couple of layers of error code conversion, reduces
dependency on cricket error types and allows us to preserve error
information from dcsctp. Along the way remove SendDataResult.

Bug: none
Change-Id: I1ad18a8f0b2fb181745b19c49f36f270708720c0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298305
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39619}
2023-03-21 13:57:47 +00:00
Tommi
4c842224e1 Adopt StreamId in SctpDataChannelControllerInterface
Bug: webrtc:11547
Change-Id: Iea2d706228b5a533eb7fae84613462165d7c9b54
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298300
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39618}
2023-03-21 13:45:51 +00:00
Tommi
e9aa8675d7 Remove SctpDataChannelControllerInterface::ConnectDataChannel
Bug: webrtc:11547
Change-Id: I389cb641746ef892106c22fd46b8d70218b99f58
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297421
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39608}
2023-03-20 14:57:44 +00:00
Tommi
dc90a9c583 [DataChannelController] Don't fire events from within a loop
Removes a couple of temporary workarounds added in this CL:
https://webrtc-review.googlesource.com/c/src/+/297981

Bug: webrtc:15004
Change-Id: I93e025725be1db56528726cc9f79740e366b8a68
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298200
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39602}
2023-03-20 11:06:16 +00:00
Tommi
ac87c8df27 Temporary fix to guard against sctp_data_channels_ being modified.
Following https://webrtc-review.googlesource.com/c/src/+/297100
it seems that sctp_data_channels_ gets modified while we're iterating
through it. This temporary fix creates a copy of the array and iterates
through the copy instead of sctp_data_channels_. A follow-up CL (or CLs)
will provide more clarity, testing and regression guards.

Bug: webrtc:15004
Change-Id: I0cb5dfb6829d36b51328875c8c9cfa392ff393a7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297981
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39584}
2023-03-16 23:22:15 +00:00
Harald Alvestrand
5da3eb0d89 Always ask for an SCTP m-section if datachannels have been used
This removes the behavior of not requesting datachannel if the first
datachannel is closed before the offer is created.

Bug: chromium:1423562
Change-Id: I90eab0f908507e65d9ee3dff51842ee6d61a8aa9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297860
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39570}
2023-03-15 21:54:21 +00:00
Tommi
df3e4caf77 Remove SctpDataChannelControllerInterface::DisconnectDataChannel
Bug: webrtc:11547
Change-Id: Ibc8c64da5289d5cb8c8a777cd0e764f71eb14fa7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297361
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39565}
2023-03-15 15:32:31 +00:00
Tommi
4e1c9570ed Remove cricket::ReceiveDataParams
This struct only contains two member variables now and there isn't
much value added by having it.

Low-Coverage-Reason: No change in coverage, CL modifies uncovered RTC_LOG lines.
Bug: none
Change-Id: I924d450f4c8f8e49b1cfeabaebee9fd5235a90cc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297360
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39563}
2023-03-15 13:27:55 +00:00
Tommi
00264ca712 Remove remaining sigslots from DataChannelController
This includes:
* SignalDataChannelTransportWritable_s
* SignalDataChannelTransportReceivedData_s
* SignalDataChannelTransportChannelClosing_s
* Removing sigslot::has_slots<> inheritance from SctpDataChannel

Instead, we use the existing sctp_data_channels_ vector of channels
known to the DCC to deliver the callbacks.

Bug: webrtc:11943, webrtc:11547
Change-Id: I7935d7505856eedf04981b8ba665ef8419166c1d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297100
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39557}
2023-03-14 21:41:36 +00:00
Tommi
51edb56884 Remove SignalDataChannelTransportChannelClosed_s
This removes one sigslot and also simplifies the teardown procedure
of a data channel when the channel is closed by the transport.
In this case we no longer need an additional async teardown task that
releases the last remaining reference to the channel.

Bug: webrtc:11943, webrtc:11547
Change-Id: I1c170349a6cbb3cb3c5a47d284e3a3d416c92b11
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296981
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39551}
2023-03-14 10:07:22 +00:00
Tommi
492296cc3c Remove the SctpDataChannel::config_ member variable.
Instead there are direct member variables for the various relevant
states, some weren't needed, some can be const but the `id` member
in particular needs special handling and can't be const.

For dealing with the stream id, we now have SctpSid. A class that does range validation, checks thread safety, handles the special `-1` case (for what's essentially an unsigned 16 bit int). Using a special type
for this also has the effect that range checking happens more
consistently (although I'm not modifying the structs in api/).
With upcoming steps of avoiding thread hops, the ID may need to
migrate to the network thread, which the thread checks will help with.

Along the way, update SctpSidAllocator to use flat_set instead of std::set and moving some of the sctp data channel code to the cc file
to help with more accurately tracking code coverage.

Bug: webrtc:11547
Change-Id: Iea6e7647ab8f93052044c5afbcc449115206b4e9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296444
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39539}
2023-03-12 17:28:14 +00:00
Tommi
2a44872da7 Remove DataChannelController::sctp_data_channels_to_free_
Instead, just use the posted task to release the reference to a
pending data channel object.

Bug: none
Change-Id: I34f0bfd604cab88587a892eaa218856c890fc907
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296767
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39527}
2023-03-10 12:43:42 +00:00
Tommi
13759bac93 Add safety flag to DataChannelController for PostTask.
Bug: none
Change-Id: If70c1cd9fb4e6fefefa96df819deac052655c49a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296140
Reviewed-by: Markus Handell <handellm@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39485}
2023-03-06 15:29:51 +00:00
Tommi
5bbfb00d1d Check for sctp open messages on the network thread.
Previously we did this on the worker thread, but are transitioning
network traffic away from thread hopping and this is one step.

Bug: webrtc:11547
Change-Id: Ia6fd6540f31a5383c70bb2bf46695e0ee526c4f7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296081
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39478}
2023-03-04 20:22:24 +00:00
Tommi
1c0d91f047 Use WeakPtr in SctpDataChannel.
DataChannelController used WeakPtr to clear outstanding references
upon destruction - except for the case of SctpDataChannel where we
had a pointer+flag for the same purpose. This change updates
SctpDataChannel and FakeDataChannelController to use a consistent
approach.

Bug: none
Change-Id: I0248471c241365a2c0de76afbb37302115650194
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295820
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39464}
2023-03-03 07:42:24 +00:00
Tommi
d2afbaf33f Remove sigslot from PeerConnectionInternal and RTCStatsCollector.
It turns out that there were several sigslot instances across data
channel, pc and stats classes that in practice only served as means
to update two counters in RTCStatsCollector. There's already a
notification path that's suitable.

This also fixes a case where the PC instance sat in the middle
of notifications from datachannels to the datachannel controller.

Bug: webrtc:11943
Change-Id: Ic60b76021584019f82085f6651230fe2fe82d465
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295781
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39456}
2023-03-02 14:21:55 +00:00
Danil Chapovalov
9e09a1f327 Replace Thread::Invoke with Thread::BlockingCall
BlockingCall doesn't take rtc::Location parameter and thus most of the dependencies on location can be removed

Bug: webrtc:11318
Change-Id: I91a17e342dd9a9e3e2c8f7fbe267474c98a8d0e5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274620
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38045}
2022-09-09 10:44:17 +00:00
Danil Chapovalov
a30439bbe6 Migrate pc/ to absl::AnyInvocable based TaskQueueBase interface
Bug: webrtc:14245
Change-Id: I9043aa507421a93f0d7ba7406e237f727999b696
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268121
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37478}
2022-07-07 10:33:28 +00:00
Artem Titov
c374d11fac Move to_queued_task.h and pending_task_safety_flag.h into public API
Bug: b/235812579
Change-Id: I9fa3dc4a65044df8b44fff4e9bfeac7233fa381c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266080
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37248}
2022-06-17 09:20:39 +00:00
Harald Alvestrand
9e5aeb9d92 Safeguard SctpDataChannel against detached controller
Since the lifetime of an SctpDataChannel is not strictly controlled
by its controller, the controller might go away before the channel
does. This CL guards against this.

Bug: webrtc:13931
Change-Id: I07046fe896d1a66bf89287429beb0587382a13a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261940
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36852}
2022-05-11 10:54:13 +00:00
Niels Möller
22b5c3ee4f Delete unused method FindDataChannelBySid
Bug: None
Change-Id: I6e7102872607a3fb52b852832ab01c44514ac1d5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259304
Auto-Submit: Niels Moller <nisse@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36574}
2022-04-19 19:41:37 +00:00
Harald Alvestrand
c24a2189d7 Update IWYU tool with a mapping file
Also apply IWYU to all .cc files in pc/, and correct BUILD file to match.
Note: Some files came out wrong when iwyu was applied. These are not included.

Bug: none
Change-Id: Ib5ea46b8fcc505414d0447cca7218ad3afc2e321
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/252280
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36064}
2022-02-24 11:05:06 +00:00
Harald Alvestrand
5b84f386fc Break out data_channel_controller from peerconnection target
Bug: webrtc:11943
Change-Id: I80c6b1be770f4e99cc7e21ef0e30977198166f88
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251323
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35949}
2022-02-08 12:32:14 +00:00