Move ready to send logic from BaseChannel to RtpTransport.

webrtc/pc/channel_unittest.cc

 /*
  *  Copyright 2009 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 1860 matching lines @@
           fake_rtp_dtls_transport2_.get(),
           reinterpret_cast<const char*>(kBadPacket), sizeof(kBadPacket),
           rtc::PacketTime(), 0);
     });
     EXPECT_EQ(cricket::SrtpFilter::ERROR_FAIL, error_handler.error_);
     EXPECT_EQ(cricket::SrtpFilter::UNPROTECT, error_handler.mode_);
     // Terminate channels/threads before the fake clock is destroyed.
     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);
     // Both sides agree on mux. Should signal that RTCP mux is fully active.
     content.set_rtcp_mux(true);
     EXPECT_TRUE(channel1_->SetLocalContent(&content, CA_OFFER, NULL));
     EXPECT_EQ(0, rtcp_mux_activated_callbacks1_);
     EXPECT_TRUE(channel1_->SetRemoteContent(&content, CA_ANSWER, NULL));
     EXPECT_EQ(1, rtcp_mux_activated_callbacks1_);
     cricket::FakeDtlsTransport* rtp = fake_rtp_dtls_transport1_.get();
     EXPECT_FALSE(media_channel1_->ready_to_send());
     // In the case of rtcp mux, the SignalReadyToSend() from rtp channel
     // should trigger the MediaChannel's OnReadyToSend.
     network_thread_->Invoke<void>(RTC_FROM_HERE,
                                   [rtp] { rtp->SignalReadyToSend(rtp); });
     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());
   }
 
   bool SetRemoteContentWithBitrateLimit(int remote_limit) {
     typename T::Content content;
     CreateContent(0, kPcmuCodec, kH264Codec, &content);
     content.set_bandwidth(remote_limit);
     return channel1_->SetRemoteContent(&content, CA_OFFER, NULL);
   }
@@ skipping 474 matching lines @@
 }
 
 TEST_F(VoiceChannelSingleThreadTest, TestFlushRtcp) {
   Base::TestFlushRtcp();
 }
 
 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.
 TEST_F(VoiceChannelSingleThreadTest, TestScaleVolume1to1Call) {
   CreateChannels(0, 0);
   EXPECT_TRUE(SendInitiate());
   EXPECT_TRUE(SendAccept());
   double volume;
 
   // Default is (1.0).
@@ skipping 307 matching lines @@
 }
 
 TEST_F(VoiceChannelDoubleThreadTest, TestFlushRtcp) {
   Base::TestFlushRtcp();
 }
 
 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.
 TEST_F(VoiceChannelDoubleThreadTest, TestScaleVolume1to1Call) {
   CreateChannels(0, 0);
   EXPECT_TRUE(SendInitiate());
   EXPECT_TRUE(SendAccept());
   double volume;
 
   // Default is (1.0).
@@ skipping 287 matching lines @@
 }
 
 TEST_F(VideoChannelSingleThreadTest, SendBundleToBundleWithRtcpMuxSecure) {
   Base::SendBundleToBundle(kVideoPts, arraysize(kVideoPts), true, true);
 }
 
 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) {
   Base::DefaultMaxBitrateIsUnlimited();
 }
 
 TEST_F(VideoChannelSingleThreadTest, CanChangeMaxBitrate) {
   Base::CanChangeMaxBitrate();
 }
 
@@ skipping 209 matching lines @@
 }
 
 TEST_F(VideoChannelDoubleThreadTest, SendBundleToBundleWithRtcpMuxSecure) {
   Base::SendBundleToBundle(kVideoPts, arraysize(kVideoPts), true, true);
 }
 
 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) {
   Base::DefaultMaxBitrateIsUnlimited();
 }
 
 TEST_F(VideoChannelDoubleThreadTest, CanChangeMaxBitrate) {
   Base::CanChangeMaxBitrate();
 }
 
@@ skipping 129 matching lines @@
 }
 
 TEST_F(RtpDataChannelSingleThreadTest, TestCallSetup) {
   Base::TestCallSetup();
 }
 
 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) {
   Base::SendRtpToRtp();
 }
 
 TEST_F(RtpDataChannelSingleThreadTest, SendRtcpToRtcp) {
   Base::SendRtcpToRtcp();
 }
 
@@ skipping 106 matching lines @@
 }
 
 TEST_F(RtpDataChannelDoubleThreadTest, TestCallSetup) {
   Base::TestCallSetup();
 }
 
 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) {
   Base::SendRtpToRtp();
 }
 
 TEST_F(RtpDataChannelDoubleThreadTest, SendRtcpToRtcp) {
   Base::SendRtcpToRtcp();
 }
 
@@ skipping 159 matching lines @@
   ASSERT_TRUE(voice_channel_.Init_w(nullptr, nullptr, &fake_rtp_dtls_transport_,
                                     &fake_rtcp_dtls_transport_));
   EXPECT_DEATH(voice_channel_.SetTransports(&fake_rtp_dtls_transport_,
                                             &fake_rtp_dtls_transport_),
                "");
 }
 
 #endif  // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
 
 // TODO(pthatcher): TestSetReceiver?