Move ready to send logic from BaseChannel to RtpTransport.

webrtc/pc/channel_unittest.cc

Index: webrtc/pc/channel_unittest.cc
diff --git a/webrtc/pc/channel_unittest.cc b/webrtc/pc/channel_unittest.cc
index 99cca88637a877f018c6f7be8c22ee09a42c8023..572825943d63fbaa094181877206b5a162be0a22 100644
--- a/webrtc/pc/channel_unittest.cc
+++ b/webrtc/pc/channel_unittest.cc
@@ -1878,52 +1878,20 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
     EXPECT_TRUE(Terminate());
   }
 
-  void TestOnReadyToSend() {

[Taylor Brandstetter, 2017/04/19 05:56:33]

Why does this test need to change? It seems correct before, and the new version
calls a "for testing" method which we should avoid when possible.

[Zach Stein, 2017/04/20 19:59:10]

This test called SetTransportChannelReadyToSend, which this change removes.
OnTransportReadyToSend is public just for testing, but so was
SetTransportChannelReadyToSend.

The new version of this test is just checking that OnTranpsortReadyToSend is
hooked up to the media channel (pretty low value). The testing of the
aggregation logic moves to the RtpTransport tests.

[Taylor Brandstetter, 2017/04/21 09:12:38]

Yeah, I see that now. So, with the TODO to not use OnTransportReadyToSend, I
think it's fine to land this.

+  void TestOnTransportReadyToSend() {
     CreateChannels(0, 0);
-    cricket::FakeDtlsTransport* rtp = fake_rtp_dtls_transport1_.get();
-    cricket::FakeDtlsTransport* rtcp = fake_rtcp_dtls_transport1_.get();
-    EXPECT_FALSE(media_channel1_->ready_to_send());
-
-    network_thread_->Invoke<void>(RTC_FROM_HERE,
-                                  [rtp] { rtp->SignalReadyToSend(rtp); });
-    WaitForThreads();
-    EXPECT_FALSE(media_channel1_->ready_to_send());
-
-    network_thread_->Invoke<void>(RTC_FROM_HERE,
-                                  [rtcp] { rtcp->SignalReadyToSend(rtcp); });
-    WaitForThreads();
-    // MediaChannel::OnReadyToSend only be called when both rtp and rtcp
-    // channel are ready to send.
-    EXPECT_TRUE(media_channel1_->ready_to_send());
-
-    // rtp channel becomes not ready to send will be propagated to mediachannel
-    network_thread_->Invoke<void>(RTC_FROM_HERE, [this] {
-      channel1_->SetTransportChannelReadyToSend(false, false);
-    });
-    WaitForThreads();
     EXPECT_FALSE(media_channel1_->ready_to_send());
 
-    network_thread_->Invoke<void>(RTC_FROM_HERE, [this] {
-      channel1_->SetTransportChannelReadyToSend(false, true);
-    });
+    channel1_->OnTransportReadyToSend(true);
     WaitForThreads();
     EXPECT_TRUE(media_channel1_->ready_to_send());
 
-    // rtcp channel becomes not ready to send will be propagated to mediachannel
-    network_thread_->Invoke<void>(RTC_FROM_HERE, [this] {
-      channel1_->SetTransportChannelReadyToSend(true, false);
-    });
+    channel1_->OnTransportReadyToSend(false);
     WaitForThreads();
     EXPECT_FALSE(media_channel1_->ready_to_send());
-
-    network_thread_->Invoke<void>(RTC_FROM_HERE, [this] {
-      channel1_->SetTransportChannelReadyToSend(true, true);
-    });
-    WaitForThreads();
-    EXPECT_TRUE(media_channel1_->ready_to_send());
   }
 
-  void TestOnReadyToSendWithRtcpMux() {
+  void TestOnTransportReadyToSendWithRtcpMux() {
     CreateChannels(0, 0);
     typename T::Content content;
     CreateContent(0, kPcmuCodec, kH264Codec, &content);
@@ -1942,9 +1910,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
     WaitForThreads();
     EXPECT_TRUE(media_channel1_->ready_to_send());
 
-    network_thread_->Invoke<void>(RTC_FROM_HERE, [this] {
-      channel1_->SetTransportChannelReadyToSend(false, false);
-    });
+    network_thread_->Invoke<void>(
+        RTC_FROM_HERE, [this] { channel1_->OnTransportReadyToSend(false); });

[Taylor Brandstetter, 2017/04/19 05:56:33]

Can you add a TODO to not call a "for testing" method, and instead make the
transport become "not ready to send" by having it simulate ENOTCONN?

[Zach Stein, 2017/04/20 19:59:10]

Done.

     WaitForThreads();
     EXPECT_FALSE(media_channel1_->ready_to_send());
   }
@@ -2439,12 +2406,12 @@ TEST_F(VoiceChannelSingleThreadTest, TestSrtpError) {
   Base::TestSrtpError(kAudioPts[0]);
 }
 
-TEST_F(VoiceChannelSingleThreadTest, TestOnReadyToSend) {
-  Base::TestOnReadyToSend();
+TEST_F(VoiceChannelSingleThreadTest, TestOnTransportReadyToSend) {
+  Base::TestOnTransportReadyToSend();
 }
 
-TEST_F(VoiceChannelSingleThreadTest, TestOnReadyToSendWithRtcpMux) {
-  Base::TestOnReadyToSendWithRtcpMux();
+TEST_F(VoiceChannelSingleThreadTest, TestOnTransportReadyToSendWithRtcpMux) {
+  Base::TestOnTransportReadyToSendWithRtcpMux();
 }
 
 // Test that we can scale the output volume properly for 1:1 calls.
@@ -2772,12 +2739,12 @@ TEST_F(VoiceChannelDoubleThreadTest, TestSrtpError) {
   Base::TestSrtpError(kAudioPts[0]);
 }
 
-TEST_F(VoiceChannelDoubleThreadTest, TestOnReadyToSend) {
-  Base::TestOnReadyToSend();
+TEST_F(VoiceChannelDoubleThreadTest, TestOnTransportReadyToSend) {
+  Base::TestOnTransportReadyToSend();
 }
 
-TEST_F(VoiceChannelDoubleThreadTest, TestOnReadyToSendWithRtcpMux) {
-  Base::TestOnReadyToSendWithRtcpMux();
+TEST_F(VoiceChannelDoubleThreadTest, TestOnTransportReadyToSendWithRtcpMux) {
+  Base::TestOnTransportReadyToSendWithRtcpMux();
 }
 
 // Test that we can scale the output volume properly for 1:1 calls.
@@ -3085,12 +3052,12 @@ TEST_F(VideoChannelSingleThreadTest, TestSrtpError) {
   Base::TestSrtpError(kVideoPts[0]);
 }
 
-TEST_F(VideoChannelSingleThreadTest, TestOnReadyToSend) {
-  Base::TestOnReadyToSend();
+TEST_F(VideoChannelSingleThreadTest, TestOnTransportReadyToSend) {
+  Base::TestOnTransportReadyToSend();
 }
 
-TEST_F(VideoChannelSingleThreadTest, TestOnReadyToSendWithRtcpMux) {
-  Base::TestOnReadyToSendWithRtcpMux();
+TEST_F(VideoChannelSingleThreadTest, TestOnTransportReadyToSendWithRtcpMux) {
+  Base::TestOnTransportReadyToSendWithRtcpMux();
 }
 
 TEST_F(VideoChannelSingleThreadTest, DefaultMaxBitrateIsUnlimited) {
@@ -3320,12 +3287,12 @@ TEST_F(VideoChannelDoubleThreadTest, TestSrtpError) {
   Base::TestSrtpError(kVideoPts[0]);
 }
 
-TEST_F(VideoChannelDoubleThreadTest, TestOnReadyToSend) {
-  Base::TestOnReadyToSend();
+TEST_F(VideoChannelDoubleThreadTest, TestOnTransportReadyToSend) {
+  Base::TestOnTransportReadyToSend();
 }
 
-TEST_F(VideoChannelDoubleThreadTest, TestOnReadyToSendWithRtcpMux) {
-  Base::TestOnReadyToSendWithRtcpMux();
+TEST_F(VideoChannelDoubleThreadTest, TestOnTransportReadyToSendWithRtcpMux) {
+  Base::TestOnTransportReadyToSendWithRtcpMux();
 }
 
 TEST_F(VideoChannelDoubleThreadTest, DefaultMaxBitrateIsUnlimited) {
@@ -3475,12 +3442,12 @@ TEST_F(RtpDataChannelSingleThreadTest, TestCallTeardownRtcpMux) {
   Base::TestCallTeardownRtcpMux();
 }
 
-TEST_F(RtpDataChannelSingleThreadTest, TestOnReadyToSend) {
-  Base::TestOnReadyToSend();
+TEST_F(RtpDataChannelSingleThreadTest, TestOnTransportReadyToSend) {
+  Base::TestOnTransportReadyToSend();
 }
 
-TEST_F(RtpDataChannelSingleThreadTest, TestOnReadyToSendWithRtcpMux) {
-  Base::TestOnReadyToSendWithRtcpMux();
+TEST_F(RtpDataChannelSingleThreadTest, TestOnTransportReadyToSendWithRtcpMux) {
+  Base::TestOnTransportReadyToSendWithRtcpMux();
 }
 
 TEST_F(RtpDataChannelSingleThreadTest, SendRtpToRtp) {
@@ -3607,12 +3574,12 @@ TEST_F(RtpDataChannelDoubleThreadTest, TestCallTeardownRtcpMux) {
   Base::TestCallTeardownRtcpMux();
 }
 
-TEST_F(RtpDataChannelDoubleThreadTest, TestOnReadyToSend) {
-  Base::TestOnReadyToSend();
+TEST_F(RtpDataChannelDoubleThreadTest, TestOnTransportReadyToSend) {
+  Base::TestOnTransportReadyToSend();
 }
 
-TEST_F(RtpDataChannelDoubleThreadTest, TestOnReadyToSendWithRtcpMux) {
-  Base::TestOnReadyToSendWithRtcpMux();
+TEST_F(RtpDataChannelDoubleThreadTest, TestOnTransportReadyToSendWithRtcpMux) {
+  Base::TestOnTransportReadyToSendWithRtcpMux();
 }
 
 TEST_F(RtpDataChannelDoubleThreadTest, SendRtpToRtp) {