admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor HasDataChannels

Browse Source 
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}
This commit is contained in:
Tommi 2023-05-15 15:14:10 +02:00 committed by WebRTC LUCI CQ
parent 3ca0aa5e6f
commit 44ebe2abc9
3 changed files with 27 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
pc/data_channel_controller.cc
View File

@ -29,20 +29,13 @@ DataChannelController::~DataChannelController() {
} }
bool DataChannelController::HasDataChannels() const { bool DataChannelController::HasDataChannels() const {
auto has_channels = [&] { RTC_DCHECK_RUN_ON(signaling_thread());
RTC_DCHECK_RUN_ON(network_thread()); return channel_usage_ == DataChannelUsage::kInUse;
return !sctp_data_channels_n_.empty();
};
if (network_thread()->IsCurrent())
return has_channels();
return network_thread()->BlockingCall(std::move(has_channels));
} }
bool DataChannelController::HasUsedDataChannels() const { bool DataChannelController::HasUsedDataChannels() const {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
return has_used_data_channels_; return channel_usage_ != DataChannelUsage::kNeverUsed;
} }
RTCError DataChannelController::SendData( RTCError DataChannelController::SendData(
@ -80,9 +73,14 @@ void DataChannelController::OnChannelStateChanged(
if (state == DataChannelInterface::DataState::kClosed) if (state == DataChannelInterface::DataState::kClosed)
OnSctpDataChannelClosed(channel); OnSctpDataChannelClosed(channel);
signaling_thread()->PostTask( DataChannelUsage channel_usage = sctp_data_channels_n_.empty()
SafeTask(signaling_safety_.flag(), ? DataChannelUsage::kHaveBeenUsed
[this, channel_id = channel->internal_id(), state = state] { : 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); pc_->OnSctpDataChannelStateChanged(channel_id, state);
})); }));
} }
@ -169,6 +167,8 @@ void DataChannelController::SetupDataChannelTransport_n(
void DataChannelController::PrepareForShutdown() { void DataChannelController::PrepareForShutdown() {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
signaling_safety_.reset(PendingTaskSafetyFlag::CreateDetachedInactive()); signaling_safety_.reset(PendingTaskSafetyFlag::CreateDetachedInactive());
if (channel_usage_ != DataChannelUsage::kNeverUsed)
channel_usage_ = DataChannelUsage::kHaveBeenUsed;
} }
void DataChannelController::TeardownDataChannelTransport_n(RTCError error) { void DataChannelController::TeardownDataChannelTransport_n(RTCError error) {
@ -237,7 +237,7 @@ bool DataChannelController::HandleOpenMessage_n(
void DataChannelController::OnDataChannelOpenMessage( void DataChannelController::OnDataChannelOpenMessage(
rtc::scoped_refptr<SctpDataChannel> channel, rtc::scoped_refptr<SctpDataChannel> channel,
bool ready_to_send) { bool ready_to_send) {
has_used_data_channels_ = true; channel_usage_ = DataChannelUsage::kInUse;
auto proxy = SctpDataChannel::CreateProxy(channel, signaling_safety_.flag()); auto proxy = SctpDataChannel::CreateProxy(channel, signaling_safety_.flag());
pc_->Observer()->OnDataChannel(proxy); pc_->Observer()->OnDataChannel(proxy);
@ -342,7 +342,7 @@ DataChannelController::InternalCreateDataChannelWithProxy(
if (!ret.ok()) if (!ret.ok())
return ret.MoveError(); return ret.MoveError();
has_used_data_channels_ = true; channel_usage_ = DataChannelUsage::kInUse;
return SctpDataChannel::CreateProxy(ret.MoveValue(), return SctpDataChannel::CreateProxy(ret.MoveValue(),
signaling_safety_.flag()); signaling_safety_.flag());
} }

12
pc/data_channel_controller.h
View File

@ -94,13 +94,13 @@ class DataChannelController : public SctpDataChannelControllerInterface,
// At some point in time, a data channel has existed. // At some point in time, a data channel has existed.
bool HasUsedDataChannels() const; bool HasUsedDataChannels() const;
void OnSctpDataChannelClosed(SctpDataChannel* channel);
protected: protected:
rtc::Thread* network_thread() const; rtc::Thread* network_thread() const;
rtc::Thread* signaling_thread() const; rtc::Thread* signaling_thread() const;
private: private:
void OnSctpDataChannelClosed(SctpDataChannel* channel);
// Creates a new SctpDataChannel object on the network thread. // Creates a new SctpDataChannel object on the network thread.
RTCErrorOr<rtc::scoped_refptr<SctpDataChannel>> CreateDataChannel( RTCErrorOr<rtc::scoped_refptr<SctpDataChannel>> CreateDataChannel(
const std::string& label, const std::string& label,
@ -143,7 +143,13 @@ class DataChannelController : public SctpDataChannelControllerInterface,
SctpSidAllocator sid_allocator_ RTC_GUARDED_BY(network_thread()); SctpSidAllocator sid_allocator_ RTC_GUARDED_BY(network_thread());
std::vector<rtc::scoped_refptr<SctpDataChannel>> sctp_data_channels_n_ std::vector<rtc::scoped_refptr<SctpDataChannel>> sctp_data_channels_n_
RTC_GUARDED_BY(network_thread()); 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. // Owning PeerConnection.
PeerConnectionInternal* const pc_; PeerConnectionInternal* const pc_;

3
pc/sdp_offer_answer.cc
View File

@ -4275,8 +4275,7 @@ void SdpOfferAnswerHandler::GetOptionsForUnifiedPlanOffer(
} }
// Lastly, add a m-section if we have requested local data channels and an // Lastly, add a m-section if we have requested local data channels and an
// m section does not already exist. // m section does not already exist.
if (!pc_->GetDataMid() && data_channel_controller()->HasUsedDataChannels() && if (!pc_->GetDataMid() && data_channel_controller()->HasDataChannels()) {
data_channel_controller()->HasDataChannels()) {
// Attempt to recycle a stopped m-line. // Attempt to recycle a stopped m-line.
// TODO(crbug.com/1442604): GetDataMid() should return the mid if one was // TODO(crbug.com/1442604): GetDataMid() should return the mid if one was
// ever created but rejected. // ever created but rejected.