Bug 1304165: More complete rollback of transport objects. draft
authorByron Campen [:bwc] <docfaraday@gmail.com>
Wed, 28 Sep 2016 11:48:33 -0500
changeset 418532 d3f9325a1181ae7aed962cc76cde8a1e923f688f
parent 417914 66a77b9bfe5dcacd50eccf85de7c0e7e15ce0ffd
child 532365 c855d8a48c5d02880b51bcb624eacf4b34efe124
push id30701
push userbcampen@mozilla.com
push dateWed, 28 Sep 2016 17:19:58 +0000
bugs1304165
milestone52.0a1
Bug 1304165: More complete rollback of transport objects. MozReview-Commit-ID: Gu1iIuyFI4g
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/src/jsep/JsepSessionImpl.h
media/webrtc/signaling/test/jsep_session_unittest.cpp
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -1110,22 +1110,20 @@ JsepSessionImpl::SetLocalDescription(Jse
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Check that content hasn't done anything unsupported with the SDP
   rv = ValidateLocalDescription(*parsed);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Create transport objects.
   mOldTransports = mTransports; // Save in case we need to rollback
+  mTransports.clear();
   for (size_t t = 0; t < parsed->GetMediaSectionCount(); ++t) {
-    if (t >= mTransports.size()) {
-      mTransports.push_back(RefPtr<JsepTransport>(new JsepTransport));
-    }
-
-    UpdateTransport(parsed->GetMediaSection(t), mTransports[t].get());
+    mTransports.push_back(RefPtr<JsepTransport>(new JsepTransport));
+    InitTransport(parsed->GetMediaSection(t), mTransports[t].get());
   }
 
   switch (type) {
     case kJsepSdpOffer:
       rv = SetLocalDescriptionOffer(Move(parsed));
       break;
     case kJsepSdpAnswer:
     case kJsepSdpPranswer:
@@ -1449,18 +1447,18 @@ JsepSessionImpl::MakeNegotiatedTrackPair
     MOZ_MTLOG(ML_DEBUG, "RTCP-MUX is off");
     trackPairOut->mRtcpTransport = transport;
   }
 
   return NS_OK;
 }
 
 void
-JsepSessionImpl::UpdateTransport(const SdpMediaSection& msection,
-                                 JsepTransport* transport)
+JsepSessionImpl::InitTransport(const SdpMediaSection& msection,
+                               JsepTransport* transport)
 {
   if (mSdpHelper.MsectionIsDisabled(msection)) {
     transport->Close();
     return;
   }
 
   if (mSdpHelper.HasRtcp(msection.GetProtocol())) {
     transport->mComponents = 2;
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
@@ -276,18 +276,17 @@ private:
   nsresult DetermineAnswererSetupRole(const SdpMediaSection& remoteMsection,
                                       SdpSetupAttribute::Role* rolep);
   nsresult MakeNegotiatedTrackPair(const SdpMediaSection& remote,
                                    const SdpMediaSection& local,
                                    const RefPtr<JsepTransport>& transport,
                                    bool usingBundle,
                                    size_t transportLevel,
                                    JsepTrackPair* trackPairOut);
-  void UpdateTransport(const SdpMediaSection& msection,
-                       JsepTransport* transport);
+  void InitTransport(const SdpMediaSection& msection, JsepTransport* transport);
 
   nsresult FinalizeTransport(const SdpAttributeList& remote,
                              const SdpAttributeList& answer,
                              const RefPtr<JsepTransport>& transport);
 
   nsresult GetNegotiatedBundledMids(SdpHelper::BundledMids* bundledMids);
 
   nsresult EnableOfferMsection(SdpMediaSection* msection);
--- a/media/webrtc/signaling/test/jsep_session_unittest.cpp
+++ b/media/webrtc/signaling/test/jsep_session_unittest.cpp
@@ -308,16 +308,112 @@ protected:
   };
 
   std::vector<JsepTrackPair> GetTrackPairsByLevel(JsepSessionImpl& side) const {
     auto pairs = side.GetNegotiatedTrackPairs();
     std::sort(pairs.begin(), pairs.end(), ComparePairsByLevel());
     return pairs;
   }
 
+  bool Equals(const SdpFingerprintAttributeList::Fingerprint& f1,
+              const SdpFingerprintAttributeList::Fingerprint& f2) const {
+    if (f1.hashFunc != f2.hashFunc) {
+      return false;
+    }
+
+    if (f1.fingerprint != f2.fingerprint) {
+      return false;
+    }
+
+    return true;
+  }
+
+  bool Equals(const SdpFingerprintAttributeList& f1,
+              const SdpFingerprintAttributeList& f2) const {
+    if (f1.mFingerprints.size() != f2.mFingerprints.size()) {
+      return false;
+    }
+
+    for (size_t i=0; i<f1.mFingerprints.size(); ++i) {
+      if (!Equals(f1.mFingerprints[i], f2.mFingerprints[i])) {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
+  bool Equals(const UniquePtr<JsepDtlsTransport>& t1,
+              const UniquePtr<JsepDtlsTransport>& t2) const {
+    if (!t1 && !t2) {
+      return true;
+    }
+
+    if (!t1 || !t2) {
+      return false;
+    }
+
+    if (!Equals(t1->GetFingerprints(),  t2->GetFingerprints())) {
+      return false;
+    }
+
+    if (t1->GetRole() != t2->GetRole()) {
+      return false;
+    }
+
+    return true;
+  }
+
+
+  bool Equals(const UniquePtr<JsepIceTransport>& t1,
+              const UniquePtr<JsepIceTransport>& t2) const {
+    if (!t1 && !t2) {
+      return true;
+    }
+
+    if (!t1 || !t2) {
+      return false;
+    }
+
+    if (t1->GetUfrag() != t2->GetUfrag()) {
+      return false;
+    }
+
+    if (t1->GetPassword() != t2->GetPassword()) {
+      return false;
+    }
+
+    return true;
+  }
+
+  bool Equals(const RefPtr<JsepTransport>& t1,
+              const RefPtr<JsepTransport>& t2) const {
+    if (!t1 && !t2) {
+      return true;
+    }
+
+    if (!t1 || !t2) {
+      return false;
+    }
+
+    if (t1->mTransportId != t2->mTransportId) {
+      return false;
+    }
+
+    if (t1->mComponents != t2->mComponents) {
+      return false;
+    }
+
+    if (!Equals(t1->mIce, t2->mIce)) {
+      return false;
+    }
+
+    return true;
+  }
+
   bool Equals(const JsepTrackPair& p1,
               const JsepTrackPair& p2) const {
     if (p1.mLevel != p2.mLevel) {
       return false;
     }
 
     // We don't check things like mBundleLevel, since that can change without
     // any changes to the transport, which is what we're really interested in.
@@ -325,21 +421,21 @@ protected:
     if (p1.mSending.get() != p2.mSending.get()) {
       return false;
     }
 
     if (p1.mReceiving.get() != p2.mReceiving.get()) {
       return false;
     }
 
-    if (p1.mRtpTransport.get() != p2.mRtpTransport.get()) {
+    if (!Equals(p1.mRtpTransport, p2.mRtpTransport)) {
       return false;
     }
 
-    if (p1.mRtcpTransport.get() != p2.mRtcpTransport.get()) {
+    if (!Equals(p1.mRtcpTransport, p2.mRtcpTransport)) {
       return false;
     }
 
     return true;
   }
 
   size_t GetTrackCount(JsepSessionImpl& side,
                        SdpMediaSection::MediaType type) const {
@@ -1880,18 +1976,20 @@ TEST_P(JsepSessionTest, RenegotiationAns
   auto newOffererPairs = mSessionOff.GetNegotiatedTrackPairs();
 
   ASSERT_EQ(offererPairs.size(), newOffererPairs.size());
   for (size_t i = 0; i < offererPairs.size(); ++i) {
     ASSERT_EQ(offererPairs[i].mReceiving->GetMediaType(),
               newOffererPairs[i].mReceiving->GetMediaType());
 
     ASSERT_EQ(offererPairs[i].mSending, newOffererPairs[i].mSending);
-    ASSERT_EQ(offererPairs[i].mRtpTransport, newOffererPairs[i].mRtpTransport);
-    ASSERT_EQ(offererPairs[i].mRtcpTransport, newOffererPairs[i].mRtcpTransport);
+    ASSERT_TRUE(Equals(offererPairs[i].mRtpTransport,
+                       newOffererPairs[i].mRtpTransport));
+    ASSERT_TRUE(Equals(offererPairs[i].mRtcpTransport,
+                       newOffererPairs[i].mRtcpTransport));
 
     if (offererPairs[i].mReceiving->GetMediaType() ==
         SdpMediaSection::kApplication) {
       ASSERT_EQ(offererPairs[i].mReceiving, newOffererPairs[i].mReceiving);
     } else {
       // This should be the only difference
       ASSERT_NE(offererPairs[i].mReceiving, newOffererPairs[i].mReceiving);
     }
@@ -1925,18 +2023,20 @@ TEST_P(JsepSessionTest, RenegotiationAns
   auto newOffererPairs = mSessionOff.GetNegotiatedTrackPairs();
 
   ASSERT_EQ(offererPairs.size(), newOffererPairs.size());
   for (size_t i = 0; i < offererPairs.size(); ++i) {
     ASSERT_EQ(offererPairs[i].mReceiving->GetMediaType(),
               newOffererPairs[i].mReceiving->GetMediaType());
 
     ASSERT_EQ(offererPairs[i].mSending, newOffererPairs[i].mSending);
-    ASSERT_EQ(offererPairs[i].mRtpTransport, newOffererPairs[i].mRtpTransport);
-    ASSERT_EQ(offererPairs[i].mRtcpTransport, newOffererPairs[i].mRtcpTransport);
+    ASSERT_TRUE(Equals(offererPairs[i].mRtpTransport,
+                       newOffererPairs[i].mRtpTransport));
+    ASSERT_TRUE(Equals(offererPairs[i].mRtcpTransport,
+                       newOffererPairs[i].mRtcpTransport));
 
     if (offererPairs[i].mReceiving->GetMediaType() ==
         SdpMediaSection::kApplication) {
       ASSERT_EQ(offererPairs[i].mReceiving, newOffererPairs[i].mReceiving);
     } else {
       // This should be the only difference
       ASSERT_NE(offererPairs[i].mReceiving, newOffererPairs[i].mReceiving);
     }