[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}
diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc
index e42c969..39a31e6 100644
--- a/pc/data_channel_controller.cc
+++ b/pc/data_channel_controller.cc
@@ -117,7 +117,7 @@
StreamId sid(channel_id);
sid_allocator_.ReleaseSid(sid);
auto it = absl::c_find_if(sctp_data_channels_n_,
- [&](const auto& c) { return c->sid() == sid; });
+ [&](const auto& c) { return c->sid_n() == sid; });
if (it != sctp_data_channels_n_.end())
sctp_data_channels_n_.erase(it);
@@ -343,9 +343,10 @@
RTC_DCHECK_RUN_ON(network_thread());
for (auto it = sctp_data_channels_n_.begin();
it != sctp_data_channels_n_.end();) {
- if (!(*it)->sid().HasValue()) {
+ if (!(*it)->sid_n().HasValue()) {
StreamId sid = sid_allocator_.AllocateSid(role);
if (sid.HasValue()) {
+ (*it)->SetSctpSid_n(sid);
AddSctpDataStream(sid);
channels_to_update.push_back(std::make_pair((*it).get(), sid));
} else {
@@ -373,22 +374,20 @@
return c.get() == pair.first;
});
RTC_DCHECK(it != sctp_data_channels_.end());
- (*it)->SetSctpSid(pair.second);
+ (*it)->SetSctpSid_s(pair.second);
}
}
void DataChannelController::OnSctpDataChannelClosed(SctpDataChannel* channel) {
RTC_DCHECK_RUN_ON(signaling_thread());
- // TODO(tommi): `sid()` should be called on the network thread.
- // `sid()` and `SctpDataChannel::id_`should have thread guards to enforce
- // correct usage.
- network_thread()->BlockingCall([&, sid = channel->sid()] {
+ network_thread()->BlockingCall([&] {
RTC_DCHECK_RUN_ON(network_thread());
// After the closing procedure is done, it's safe to use this ID for
// another data channel.
- if (sid.HasValue())
- sid_allocator_.ReleaseSid(sid);
+ if (channel->sid_n().HasValue()) {
+ sid_allocator_.ReleaseSid(channel->sid_n());
+ }
auto it = absl::c_find_if(sctp_data_channels_n_, [&](const auto& c) {
return c.get() == channel;
@@ -463,15 +462,14 @@
RTC_DCHECK_RUN_ON(network_thread());
RTC_DCHECK(data_channel_transport());
- // TODO(tommi): Move the blocking call to `AddSctpDataStream` from
- // `SctpDataChannel::OnTransportChannelCreated` to here and be consistent
- // with other call sites to `AddSctpDataStream`. We're already
- // on the right (network) thread here.
+ for (const auto& channel : sctp_data_channels_n_) {
+ if (channel->sid_n().HasValue())
+ AddSctpDataStream(channel->sid_n());
+ }
signaling_thread()->PostTask(SafeTask(signaling_safety_.flag(), [this] {
RTC_DCHECK_RUN_ON(signaling_thread());
- auto copy = sctp_data_channels_;
- for (const auto& channel : copy) {
+ for (const auto& channel : sctp_data_channels_) {
channel->OnTransportChannelCreated();
}
}));
@@ -480,8 +478,9 @@
std::vector<rtc::scoped_refptr<SctpDataChannel>>::iterator
DataChannelController::FindChannel(StreamId stream_id) {
RTC_DCHECK_RUN_ON(signaling_thread());
- return absl::c_find_if(sctp_data_channels_,
- [&](const auto& c) { return c->sid() == stream_id; });
+ return absl::c_find_if(sctp_data_channels_, [&](const auto& c) {
+ return c->sid_s() == stream_id;
+ });
}
rtc::Thread* DataChannelController::network_thread() const {
