Bug 786234 - Part 4: Fix a bug in mediapipeline_unittest where RTP packets were mashed together into a single buffer five at a time, preventing the receive pipeline from successfully decrypting them and causing the packet counts to be wrong. r=ethanhugg
☠☠ backed out by 18849209753a ☠ ☠
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 17 Jan 2014 17:10:17 -0800
changeset 170543 c8083d830fa6b2fa4fa4e549d4942b64b289a7bd
parent 170542 c5334aea64331b9dcc282082cd16a5511917e53d
child 170544 2d1f70b91712b19c9207fd9c35c6a1d0a3fd4bf0
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersethanhugg
bugs786234
milestone30.0a1
Bug 786234 - Part 4: Fix a bug in mediapipeline_unittest where RTP packets were mashed together into a single buffer five at a time, preventing the receive pipeline from successfully decrypting them and causing the packet counts to be wrong. r=ethanhugg
media/webrtc/signaling/test/mediapipeline_unittest.cpp
--- a/media/webrtc/signaling/test/mediapipeline_unittest.cpp
+++ b/media/webrtc/signaling/test/mediapipeline_unittest.cpp
@@ -20,17 +20,17 @@
 #include "FakeMediaStreams.h"
 #include "FakeMediaStreamsImpl.h"
 #include "MediaConduitErrors.h"
 #include "MediaConduitInterface.h"
 #include "MediaPipeline.h"
 #include "MediaPipelineFilter.h"
 #include "runnable_utils.h"
 #include "transportflow.h"
-#include "transportlayerprsock.h"
+#include "transportlayerloopback.h"
 #include "transportlayerdtls.h"
 #include "mozilla/SyncRunnable.h"
 
 
 #include "mtransport_test_utils.h"
 #include "runnable_utils.h"
 
 #include "webrtc/modules/interface/module_common_types.h"
@@ -45,27 +45,36 @@ MOZ_MTLOG_MODULE("mediapipeline")
 MtransportTestUtils *test_utils;
 
 namespace {
 
 class TransportInfo {
  public:
   TransportInfo() :
     flow_(nullptr),
-    prsock_(nullptr),
+    loopback_(nullptr),
     dtls_(nullptr) {}
 
+  static void InitAndConnect(TransportInfo &client, TransportInfo &server) {
+    client.Init(true);
+    server.Init(false);
+    client.PushLayers();
+    server.PushLayers();
+    client.Connect(&server);
+    server.Connect(&client);
+  }
+
   void Init(bool client) {
     nsresult res;
 
     flow_ = new TransportFlow();
-    prsock_ = new TransportLayerPrsock();
+    loopback_ = new TransportLayerLoopback();
     dtls_ = new TransportLayerDtls();
 
-    res = prsock_->Init();
+    res = loopback_->Init();
     if (res != NS_OK) {
       FreeLayers();
     }
     ASSERT_EQ((nsresult)NS_OK, res);
 
     std::vector<uint16_t> ciphers;
     ciphers.push_back(SRTP_AES128_CM_HMAC_SHA1_80);
     dtls_->SetSrtpCiphers(ciphers);
@@ -75,76 +84,77 @@ class TransportInfo {
     dtls_->SetVerificationAllowAll();
   }
 
   void PushLayers() {
     nsresult res;
 
     nsAutoPtr<std::queue<TransportLayer *> > layers(
       new std::queue<TransportLayer *>);
-    layers->push(prsock_);
+    layers->push(loopback_);
     layers->push(dtls_);
     res = flow_->PushLayers(layers);
     if (res != NS_OK) {
       FreeLayers();
     }
     ASSERT_EQ((nsresult)NS_OK, res);
   }
 
+  void Connect(TransportInfo* peer) {
+    MOZ_ASSERT(loopback_);
+    MOZ_ASSERT(peer->loopback_);
+
+    loopback_->Connect(peer->loopback_);
+  }
+
   // Free the memory allocated at the beginning of Init
   // if failure occurs before layers setup.
   void FreeLayers() {
-    delete prsock_;
+    delete loopback_;
+    loopback_ = nullptr;
     delete dtls_;
+    dtls_ = nullptr;
   }
 
   void Stop() {
+    if (loopback_) {
+      loopback_->Disconnect();
+    }
+    loopback_ = nullptr;
+    dtls_ = nullptr;
     flow_ = nullptr;
   }
 
   mozilla::RefPtr<TransportFlow> flow_;
-  TransportLayerPrsock *prsock_;
+  TransportLayerLoopback *loopback_;
   TransportLayerDtls *dtls_;
 };
 
 class TestAgent {
  public:
   TestAgent() :
       audio_config_(109, "opus", 48000, 960, 2, 64000),
       audio_conduit_(mozilla::AudioSessionConduit::Create(nullptr)),
       audio_(),
       audio_pipeline_() {
   }
 
-  void ConnectRtpSocket(PRFileDesc *fd, bool client) {
-    ConnectSocket(&audio_rtp_transport_, fd, client);
-  }
-
-  void ConnectRtcpSocket(PRFileDesc *fd, bool client) {
-    ConnectSocket(&audio_rtcp_transport_, fd, client);
-  }
-
-  void ConnectBundleSocket(PRFileDesc *fd, bool client) {
-    ConnectSocket(&bundle_transport_, fd, client);
+  static void ConnectRtp(TestAgent *client, TestAgent *server) {
+    TransportInfo::InitAndConnect(client->audio_rtp_transport_,
+                                  server->audio_rtp_transport_);
   }
 
-  void ConnectSocket(TransportInfo *transport, PRFileDesc *fd, bool client) {
-    nsresult res;
-
-    transport->Init(client);
+  static void ConnectRtcp(TestAgent *client, TestAgent *server) {
+    TransportInfo::InitAndConnect(client->audio_rtcp_transport_,
+                                  server->audio_rtcp_transport_);
+  }
 
-    mozilla::SyncRunnable::DispatchToThread(
-      test_utils->sts_target(),
-      WrapRunnable(transport->prsock_, &TransportLayerPrsock::Import, fd, &res));
-    if (!NS_SUCCEEDED(res)) {
-      transport->FreeLayers();
-    }
-    ASSERT_TRUE(NS_SUCCEEDED(res));
-
-    transport->PushLayers();
+  static void ConnectBundle(TestAgent *client, TestAgent *server) {
+    TransportInfo::InitAndConnect(client->bundle_transport_,
+                                  server->bundle_transport_);
   }
 
   virtual void CreatePipelines_s(bool aIsRtcpMux) = 0;
 
   void Start() {
     nsresult ret;
 
     MOZ_MTLOG(ML_DEBUG, "Starting");
@@ -171,18 +181,16 @@ class TestAgent {
     if (audio_pipeline_)
       audio_pipeline_->ShutdownMedia_m();
 
     mozilla::SyncRunnable::DispatchToThread(
       test_utils->sts_target(),
       WrapRunnable(this, &TestAgent::StopInt));
 
     audio_pipeline_ = nullptr;
-
-    PR_Sleep(1000); // Deal with race condition
   }
 
  protected:
   mozilla::AudioCodecConfig audio_config_;
   mozilla::RefPtr<mozilla::MediaSessionConduit> audio_conduit_;
   nsRefPtr<DOMMediaStream> audio_;
   mozilla::RefPtr<mozilla::MediaPipeline> audio_pipeline_;
   TransportInfo audio_rtp_transport_;
@@ -317,71 +325,40 @@ class TestAgentReceive : public TestAgen
   }
  private:
   nsAutoPtr<MediaPipelineFilter> bundle_filter_;
 };
 
 
 class MediaPipelineTest : public ::testing::Test {
  public:
-  MediaPipelineTest() : p1_() {
-    rtp_fds_[0] = rtp_fds_[1] = nullptr;
-    rtcp_fds_[0] = rtcp_fds_[1] = nullptr;
-    bundle_fds_[0] = bundle_fds_[1] = nullptr;
+  ~MediaPipelineTest() {
+    p1_.Stop();
+    p2_.Stop();
   }
 
   // Setup transport.
   void InitTransports(bool aIsRtcpMux) {
-    // Create RTP related transport.
-    PRStatus status =  PR_NewTCPSocketPair(rtp_fds_);
-    ASSERT_EQ(status, PR_SUCCESS);
-
-    // RTP, DTLS server
+    // RTP, p1_ is server, p2_ is client
     mozilla::SyncRunnable::DispatchToThread(
       test_utils->sts_target(),
-      WrapRunnable(&p1_, &TestAgent::ConnectRtpSocket, rtp_fds_[0], false));
-
-    // RTP, DTLS client
-    mozilla::SyncRunnable::DispatchToThread(
-      test_utils->sts_target(),
-      WrapRunnable(&p2_, &TestAgent::ConnectRtpSocket, rtp_fds_[1], true));
+      WrapRunnableNM(&TestAgent::ConnectRtp, &p2_, &p1_));
 
     // Create RTCP flows separately if we are not muxing them.
     if(!aIsRtcpMux) {
-      status = PR_NewTCPSocketPair(rtcp_fds_);
-      ASSERT_EQ(status, PR_SUCCESS);
-
-      // RTCP, DTLS server
+      // RTCP, p1_ is server, p2_ is client
       mozilla::SyncRunnable::DispatchToThread(
         test_utils->sts_target(),
-        WrapRunnable(&p1_, &TestAgent::ConnectRtcpSocket, rtcp_fds_[0], false));
-
-      // RTCP, DTLS client
-      mozilla::SyncRunnable::DispatchToThread(
-        test_utils->sts_target(),
-        WrapRunnable(&p2_, &TestAgent::ConnectRtcpSocket, rtcp_fds_[1], true));
+        WrapRunnableNM(&TestAgent::ConnectRtcp, &p2_, &p1_));
     }
 
-    status = PR_NewTCPSocketPair(bundle_fds_);
-    // BUNDLE, DTLS server
+    // BUNDLE, p1_ is server, p2_ is client
     mozilla::SyncRunnable::DispatchToThread(
       test_utils->sts_target(),
-      WrapRunnable(&p1_,
-                   &TestAgent::ConnectBundleSocket,
-                   bundle_fds_[0],
-                   false));
-
-    // BUNDLE, DTLS client
-    mozilla::SyncRunnable::DispatchToThread(
-      test_utils->sts_target(),
-      WrapRunnable(&p2_,
-                   &TestAgent::ConnectBundleSocket,
-                   bundle_fds_[1],
-                   true));
-
+      WrapRunnableNM(&TestAgent::ConnectBundle, &p2_, &p1_));
   }
 
   // Verify RTP and RTCP
   void TestAudioSend(bool aIsRtcpMux,
                      bool bundle = false,
                      nsAutoPtr<MediaPipelineFilter> localFilter =
                         nsAutoPtr<MediaPipelineFilter>(nullptr),
                      nsAutoPtr<MediaPipelineFilter> remoteFilter =
@@ -426,38 +403,35 @@ class MediaPipelineTest : public ::testi
                        remoteFilter));
     }
 
 
     // wait for some RTP/RTCP tx and rx to happen
     PR_Sleep(10000);
 
     if (bundle) {
-      // Filter should have eaten everything, so no RTCP
+      // Filter should have eaten everything
+      ASSERT_EQ(0, p2_.GetAudioRtpCount());
     } else {
       ASSERT_GE(p1_.GetAudioRtpCount(), 40);
-// TODO: Fix to not fail or crash (Bug 947663)
-//    ASSERT_GE(p2_.GetAudioRtpCount(), 40);
+      ASSERT_GE(p2_.GetAudioRtpCount(), 40);
       ASSERT_GE(p2_.GetAudioRtcpCount(), 1);
     }
 
     p1_.Stop();
     p2_.Stop();
   }
 
   void TestAudioReceiverOffersBundle(bool bundle_accepted,
       nsAutoPtr<MediaPipelineFilter> localFilter,
       nsAutoPtr<MediaPipelineFilter> remoteFilter =
           nsAutoPtr<MediaPipelineFilter>(nullptr)) {
     TestAudioSend(true, bundle_accepted, localFilter, remoteFilter);
   }
 protected:
-  PRFileDesc *rtp_fds_[2];
-  PRFileDesc *rtcp_fds_[2];
-  PRFileDesc *bundle_fds_[2];
   TestAgentSend p1_;
   TestAgentReceive p2_;
 };
 
 class MediaPipelineFilterTest : public ::testing::Test {
   public:
     bool Filter(MediaPipelineFilter& filter,
                 int32_t correlator,