Move ready to send logic from BaseChannel to RtpTransport.

webrtc/pc/channel.cc

Index: webrtc/pc/channel.cc
diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc
index d45e220d5c7843b9eab9099d7ccdd58d6b4e9dc6..a871888fe2d8d9b930d9524c20d64bf97a762e15 100644
--- a/webrtc/pc/channel.cc
+++ b/webrtc/pc/channel.cc
@@ -176,6 +176,8 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread,
 #if defined(ENABLE_EXTERNAL_AUTH)
   srtp_filter_.EnableExternalAuth();
 #endif
+  rtp_transport_.SignalReadyToSend.connect(
+      this, &BaseChannel::OnTransportReadyToSend);
   LOG(LS_INFO) << "Created channel for " << content_name;
 }
 
@@ -377,8 +379,10 @@ void BaseChannel::SetTransport_n(
   RTC_DCHECK(old_packet_transport != new_packet_transport);
   if (old_dtls_transport) {
     DisconnectFromDtlsTransport(old_dtls_transport);
+    rtp_transport_.Disconnect(rtcp);
   } else if (old_packet_transport) {
     DisconnectFromPacketTransport(old_packet_transport);
+    rtp_transport_.Disconnect(rtcp);

[Taylor Brandstetter, 2017/04/14 18:05:33]

Could the "disconnect"/"connect" happen implicitly in
"set_rt[c]p_packet_transport"? If you do this, they should be changed to use
camel case naming, since they'd no longer be trivial setters.

[Zach Stein, 2017/04/18 23:39:29]

Done.

   }
 
   if (rtcp) {
@@ -398,6 +402,8 @@ void BaseChannel::SetTransport_n(
         << "Setting RTCP for DTLS/SRTP after SrtpFilter is active "
         << "should never happen.";
   }
+
+  rtp_transport_.Connect(rtcp);
   if (new_dtls_transport) {
     ConnectToDtlsTransport(new_dtls_transport);
   } else {
@@ -412,9 +418,9 @@ void BaseChannel::SetTransport_n(
 void BaseChannel::ConnectToDtlsTransport(DtlsTransportInternal* transport) {
   RTC_DCHECK(network_thread_->IsCurrent());
 
+  // TODO(zstein): de-dup with ConnectToPacketTransport
   transport->SignalWritableState.connect(this, &BaseChannel::OnWritableState);
   transport->SignalReadPacket.connect(this, &BaseChannel::OnPacketRead);
-  transport->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend);
   transport->SignalDtlsState.connect(this, &BaseChannel::OnDtlsState);
   transport->SignalSentPacket.connect(this, &BaseChannel::SignalSentPacket_n);
   transport->ice_transport()->SignalSelectedCandidatePairChanged.connect(
@@ -429,7 +435,6 @@ void BaseChannel::DisconnectFromDtlsTransport(
 
   transport->SignalWritableState.disconnect(this);
   transport->SignalReadPacket.disconnect(this);
-  transport->SignalReadyToSend.disconnect(this);
   transport->SignalDtlsState.disconnect(this);
   transport->SignalSentPacket.disconnect(this);
   transport->ice_transport()->SignalSelectedCandidatePairChanged.disconnect(
@@ -441,7 +446,6 @@ void BaseChannel::ConnectToPacketTransport(
   RTC_DCHECK_RUN_ON(network_thread_);
   transport->SignalWritableState.connect(this, &BaseChannel::OnWritableState);
   transport->SignalReadPacket.connect(this, &BaseChannel::OnPacketRead);
-  transport->SignalReadyToSend.connect(this, &BaseChannel::OnReadyToSend);
   transport->SignalSentPacket.connect(this, &BaseChannel::SignalSentPacket_n);
 }
 
@@ -450,7 +454,6 @@ void BaseChannel::DisconnectFromPacketTransport(
   RTC_DCHECK_RUN_ON(network_thread_);
   transport->SignalWritableState.disconnect(this);
   transport->SignalReadPacket.disconnect(this);
-  transport->SignalReadyToSend.disconnect(this);
   transport->SignalSentPacket.disconnect(this);
 }
 
@@ -618,13 +621,6 @@ void BaseChannel::OnPacketRead(rtc::PacketTransportInternal* transport,
   HandlePacket(rtcp, &packet, packet_time);
 }
 
-void BaseChannel::OnReadyToSend(rtc::PacketTransportInternal* transport) {
-  RTC_DCHECK(transport == rtp_transport_.rtp_packet_transport() ||
-             transport == rtp_transport_.rtcp_packet_transport());
-  SetTransportChannelReadyToSend(
-      transport == rtp_transport_.rtcp_packet_transport(), true);
-}
-
 void BaseChannel::OnDtlsState(DtlsTransportInternal* transport,
                               DtlsTransportState state) {
   if (!ShouldSetupDtlsSrtp_n()) {
@@ -670,20 +666,13 @@ void BaseChannel::OnSelectedCandidatePairChanged(
 
 void BaseChannel::SetTransportChannelReadyToSend(bool rtcp, bool ready) {

[Taylor Brandstetter, 2017/04/14 18:05:33]

Can this method be removed, and the necessary logic moved to RtpTransport?
Currently this is called either:

1. When the transport channel changes
2. When sending a packet fails

RtpTransport already knows about #1. So you'd just need to move the
responsibility of sending packets to RtpTransport to handle #2. Meaning, add
"SendRtpPacket"/"SendRtcpPacket" methods that take an RTP or SRTP packet as
input and just send it on the right transport.

[Zach Stein, 2017/04/18 23:39:29]

Done.

   RTC_DCHECK(network_thread_->IsCurrent());
-  if (rtcp) {
-    rtcp_ready_to_send_ = ready;
-  } else {
-    rtp_ready_to_send_ = ready;
-  }
-
-  bool ready_to_send =
-      (rtp_ready_to_send_ &&
-       // In the case of rtcp mux |rtcp_packet_transport_| will be null.
-       (rtcp_ready_to_send_ || !rtp_transport_.rtcp_packet_transport()));
+  rtp_transport_.SetReadyToSend(rtcp, ready);
+}
 
+void BaseChannel::OnTransportReadyToSend(bool ready) {
   invoker_.AsyncInvoke<void>(
       RTC_FROM_HERE, worker_thread_,
-      Bind(&MediaChannel::OnReadyToSend, media_channel_, ready_to_send));
+      Bind(&MediaChannel::OnReadyToSend, media_channel_, ready));
 }
 
 bool BaseChannel::PacketIsRtcp(const rtc::PacketTransportInternal* transport,