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}
diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc
index 166e181..7fea6c5 100644
--- a/pc/data_channel_controller.cc
+++ b/pc/data_channel_controller.cc
@@ -29,20 +29,13 @@
}
bool DataChannelController::HasDataChannels() const {
- auto has_channels = [&] {
- RTC_DCHECK_RUN_ON(network_thread());
- return !sctp_data_channels_n_.empty();
- };
-
- if (network_thread()->IsCurrent())
- return has_channels();
-
- return network_thread()->BlockingCall(std::move(has_channels));
+ RTC_DCHECK_RUN_ON(signaling_thread());
+ return channel_usage_ == DataChannelUsage::kInUse;
}
bool DataChannelController::HasUsedDataChannels() const {
RTC_DCHECK_RUN_ON(signaling_thread());
- return has_used_data_channels_;
+ return channel_usage_ != DataChannelUsage::kNeverUsed;
}
RTCError DataChannelController::SendData(
@@ -80,11 +73,16 @@
if (state == DataChannelInterface::DataState::kClosed)
OnSctpDataChannelClosed(channel);
- signaling_thread()->PostTask(
- SafeTask(signaling_safety_.flag(),
- [this, channel_id = channel->internal_id(), state = state] {
- pc_->OnSctpDataChannelStateChanged(channel_id, state);
- }));
+ DataChannelUsage channel_usage = sctp_data_channels_n_.empty()
+ ? DataChannelUsage::kHaveBeenUsed
+ : DataChannelUsage::kInUse;
+ signaling_thread()->PostTask(SafeTask(
+ signaling_safety_.flag(), [this, channel_id = channel->internal_id(),
+ state = state, channel_usage] {
+ RTC_DCHECK_RUN_ON(signaling_thread());
+ channel_usage_ = channel_usage;
+ pc_->OnSctpDataChannelStateChanged(channel_id, state);
+ }));
}
void DataChannelController::OnDataReceived(
@@ -169,6 +167,8 @@
void DataChannelController::PrepareForShutdown() {
RTC_DCHECK_RUN_ON(signaling_thread());
signaling_safety_.reset(PendingTaskSafetyFlag::CreateDetachedInactive());
+ if (channel_usage_ != DataChannelUsage::kNeverUsed)
+ channel_usage_ = DataChannelUsage::kHaveBeenUsed;
}
void DataChannelController::TeardownDataChannelTransport_n(RTCError error) {
@@ -237,7 +237,7 @@
void DataChannelController::OnDataChannelOpenMessage(
rtc::scoped_refptr<SctpDataChannel> channel,
bool ready_to_send) {
- has_used_data_channels_ = true;
+ channel_usage_ = DataChannelUsage::kInUse;
auto proxy = SctpDataChannel::CreateProxy(channel, signaling_safety_.flag());
pc_->Observer()->OnDataChannel(proxy);
@@ -342,7 +342,7 @@
if (!ret.ok())
return ret.MoveError();
- has_used_data_channels_ = true;
+ channel_usage_ = DataChannelUsage::kInUse;
return SctpDataChannel::CreateProxy(ret.MoveValue(),
signaling_safety_.flag());
}
diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h
index 1b5c8be..bf3ac03 100644
--- a/pc/data_channel_controller.h
+++ b/pc/data_channel_controller.h
@@ -94,13 +94,13 @@
// At some point in time, a data channel has existed.
bool HasUsedDataChannels() const;
- void OnSctpDataChannelClosed(SctpDataChannel* channel);
-
protected:
rtc::Thread* network_thread() const;
rtc::Thread* signaling_thread() const;
private:
+ void OnSctpDataChannelClosed(SctpDataChannel* channel);
+
// Creates a new SctpDataChannel object on the network thread.
RTCErrorOr<rtc::scoped_refptr<SctpDataChannel>> CreateDataChannel(
const std::string& label,
@@ -143,7 +143,13 @@
SctpSidAllocator sid_allocator_ RTC_GUARDED_BY(network_thread());
std::vector<rtc::scoped_refptr<SctpDataChannel>> sctp_data_channels_n_
RTC_GUARDED_BY(network_thread());
- bool has_used_data_channels_ RTC_GUARDED_BY(signaling_thread()) = false;
+ enum class DataChannelUsage : uint8_t {
+ kNeverUsed = 0,
+ kHaveBeenUsed,
+ kInUse
+ };
+ DataChannelUsage channel_usage_ RTC_GUARDED_BY(signaling_thread()) =
+ DataChannelUsage::kNeverUsed;
// Owning PeerConnection.
PeerConnectionInternal* const pc_;
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 0fbc7ad..7f3db91 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -4275,8 +4275,7 @@
}
// Lastly, add a m-section if we have requested local data channels and an
// m section does not already exist.
- if (!pc_->GetDataMid() && data_channel_controller()->HasUsedDataChannels() &&
- data_channel_controller()->HasDataChannels()) {
+ if (!pc_->GetDataMid() && data_channel_controller()->HasDataChannels()) {
// Attempt to recycle a stopped m-line.
// TODO(crbug.com/1442604): GetDataMid() should return the mid if one was
// ever created but rejected.
