Get rid of unnecessary network thread Invoke in BaseChannel.

By changing was_ever_writable_ to be guarded by the worker thread
instead of the network thread.

Gets rid of one network thread invoke per audio/video m= section per
round of negotiation.

NOTRY=True

Bug: webrtc:12266
Change-Id: Ie913a9f96db3fd8351559e916922c82d2d0337f0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/203881
Commit-Queue: Taylor <deadbeef@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33130}

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<bool>(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);
 }

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<StreamParams>& 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<std::pair<rtc::Socket::Option, int> > rtcp_socket_options_
       RTC_GUARDED_BY(network_thread());
-  // TODO(bugs.webrtc.org/12230): writable_ is accessed in tests
+  bool writable_ RTC_GUARDED_BY(network_thread()) = false;
-  // outside of the network thread.
+  bool was_ever_writable_n_ RTC_GUARDED_BY(network_thread()) = false;
-  bool writable_ = false;
+  bool was_ever_writable_ RTC_GUARDED_BY(worker_thread()) = false;
-  bool was_ever_writable_ RTC_GUARDED_BY(network_thread()) = false;
   const bool srtp_required_ = true;
   const webrtc::CryptoOptions crypto_options_;
 

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());