diff --git a/pc/channel.cc b/pc/channel.cc index 2247dd36d0..b672a96539 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -314,14 +314,6 @@ bool BaseChannel::IsReadyToReceiveMedia_w() const { } bool BaseChannel::IsReadyToSendMedia_w() const { - // Need to access some state updated on the network thread. - return network_thread_->Invoke(RTC_FROM_HERE, [this] { - RTC_DCHECK_RUN_ON(network_thread()); - return IsReadyToSendMedia_n(); - }); -} - -bool BaseChannel::IsReadyToSendMedia_n() const { // Send outgoing data if we are enabled, have local and remote content, // and we have had some form of connectivity. return enabled() && @@ -580,22 +572,27 @@ void BaseChannel::ChannelWritable_n() { if (writable_) { return; } - - RTC_LOG(LS_INFO) << "Channel writable (" << ToString() << ")" - << (was_ever_writable_ ? "" : " for the first time"); - - was_ever_writable_ = true; writable_ = true; - UpdateMediaSendRecvState(); + RTC_LOG(LS_INFO) << "Channel writable (" << ToString() << ")" + << (was_ever_writable_n_ ? "" : " for the first time"); + // We only have to do this PostTask once, when first transitioning to + // writable. + if (!was_ever_writable_n_) { + worker_thread_->PostTask(ToQueuedTask(alive_, [this] { + RTC_DCHECK_RUN_ON(worker_thread()); + was_ever_writable_ = true; + UpdateMediaSendRecvState_w(); + })); + } + was_ever_writable_n_ = true; } void BaseChannel::ChannelNotWritable_n() { - if (!writable_) + if (!writable_) { return; - - RTC_LOG(LS_INFO) << "Channel not writable (" << ToString() << ")"; + } writable_ = false; - UpdateMediaSendRecvState(); + RTC_LOG(LS_INFO) << "Channel not writable (" << ToString() << ")"; } bool BaseChannel::AddRecvStream_w(const StreamParams& sp) { @@ -893,12 +890,6 @@ VoiceChannel::~VoiceChannel() { Deinit(); } -void BaseChannel::UpdateMediaSendRecvState() { - RTC_DCHECK_RUN_ON(network_thread()); - worker_thread_->PostTask( - ToQueuedTask(alive_, [this] { UpdateMediaSendRecvState_w(); })); -} - void VoiceChannel::Init_w(webrtc::RtpTransportInternal* rtp_transport) { BaseChannel::Init_w(rtp_transport); } diff --git a/pc/channel.h b/pc/channel.h index 113ad20bbd..e795a10529 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -143,8 +143,6 @@ class BaseChannel : public ChannelInterface, return srtp_active(); } - bool writable() const { return writable_; } - // Set an RTP level transport which could be an RtpTransport without // encryption, an SrtpTransport for SDES or a DtlsSrtpTransport for DTLS-SRTP. // This can be called from any thread and it hops to the network thread @@ -221,7 +219,7 @@ class BaseChannel : public ChannelInterface, protected: bool was_ever_writable() const { - RTC_DCHECK_RUN_ON(network_thread()); + RTC_DCHECK_RUN_ON(worker_thread()); return was_ever_writable_; } void set_local_content_direction(webrtc::RtpTransceiverDirection direction) { @@ -287,7 +285,6 @@ class BaseChannel : public ChannelInterface, // Should be called whenever the conditions for // IsReadyToReceiveMedia/IsReadyToSendMedia are satisfied (or unsatisfied). // Updates the send/recv state of the media channel. - void UpdateMediaSendRecvState(); virtual void UpdateMediaSendRecvState_w() = 0; bool UpdateLocalStreams_w(const std::vector& streams, @@ -345,7 +342,6 @@ class BaseChannel : public ChannelInterface, void DisconnectFromRtpTransport(); void SignalSentPacket_n(const rtc::SentPacket& sent_packet) RTC_RUN_ON(network_thread()); - bool IsReadyToSendMedia_n() const RTC_RUN_ON(network_thread()); rtc::Thread* const worker_thread_; rtc::Thread* const network_thread_; @@ -372,10 +368,9 @@ class BaseChannel : public ChannelInterface, RTC_GUARDED_BY(network_thread()); std::vector > rtcp_socket_options_ RTC_GUARDED_BY(network_thread()); - // TODO(bugs.webrtc.org/12230): writable_ is accessed in tests - // outside of the network thread. - bool writable_ = false; - bool was_ever_writable_ RTC_GUARDED_BY(network_thread()) = false; + bool writable_ RTC_GUARDED_BY(network_thread()) = false; + bool was_ever_writable_n_ RTC_GUARDED_BY(network_thread()) = false; + bool was_ever_writable_ RTC_GUARDED_BY(worker_thread()) = false; const bool srtp_required_ = true; const webrtc::CryptoOptions crypto_options_; diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index c4071475d0..f2e93d69ea 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -323,6 +323,10 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { fake_rtcp_packet_transport2_.get(), asymmetric); } }); + // The transport becoming writable will asynchronously update the send state + // on the worker thread; since this test uses the main thread as the worker + // thread, we must process the message queue for this to occur. + WaitForThreads(); } bool SendInitiate() { @@ -915,8 +919,6 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { EXPECT_FALSE(channel2_->SrtpActiveForTesting()); EXPECT_TRUE(SendInitiate()); WaitForThreads(); - EXPECT_TRUE(channel1_->writable()); - EXPECT_TRUE(channel2_->writable()); EXPECT_TRUE(SendAccept()); EXPECT_TRUE(channel1_->SrtpActiveForTesting()); EXPECT_TRUE(channel2_->SrtpActiveForTesting());