Bug 1325991 - sections with bundle-only should have port set to 0. r=drno,jesup
authorMichael Froman <mfroman@mozilla.com>
Fri, 30 Dec 2016 13:29:51 -0600
changeset 327727 e4e55af56102c89813ae8e2ba7fdcf7050036bdf
parent 327726 a20283c2bbea3011b9f54f0efef6ea5aca0dea7e
child 327728 88ab9fde6a047163cbe3c5051f639e747a038844
push id35554
push userryanvm@gmail.com
push dateSat, 31 Dec 2016 02:56:08 +0000
treeherderautoland@e4e55af56102 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdrno, jesup
bugs1325991
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1325991 - sections with bundle-only should have port set to 0. r=drno,jesup MozReview-Commit-ID: 6O7X1MWZhZI
media/webrtc/signaling/gtest/jsep_session_unittest.cpp
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/src/sdp/SdpHelper.cpp
--- a/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
+++ b/media/webrtc/signaling/gtest/jsep_session_unittest.cpp
@@ -819,31 +819,36 @@ protected:
         }
       }
 
       void CheckDefaultRtpCandidate(bool expectDefault,
                                     const SdpMediaSection& msection,
                                     size_t transportLevel,
                                     const std::string& context) const
       {
+        Address expectedAddress = "0.0.0.0";
+        Port expectedPort = 9U;
+
         if (expectDefault) {
           // Copy so we can be terse and use []
           auto defaultCandidates = mDefaultCandidates;
-          ASSERT_EQ(defaultCandidates[transportLevel][RTP].first,
-                    msection.GetConnection().GetAddress())
-            << context << " (level " << msection.GetLevel() << ")";
-          ASSERT_EQ(defaultCandidates[transportLevel][RTP].second,
-                    msection.GetPort())
-            << context << " (level " << msection.GetLevel() << ")";
-        } else {
-          ASSERT_EQ("0.0.0.0", msection.GetConnection().GetAddress())
-            << context << " (level " << msection.GetLevel() << ")";
-          ASSERT_EQ(9U, msection.GetPort())
-            << context << " (level " << msection.GetLevel() << ")";
+          expectedAddress = defaultCandidates[transportLevel][RTP].first;
+          expectedPort = defaultCandidates[transportLevel][RTP].second;
         }
+
+        // if bundle-only attribute is present, expect port 0
+        const SdpAttributeList& attrs = msection.GetAttributeList();
+        if (attrs.HasAttribute(SdpAttribute::kBundleOnlyAttribute)) {
+          expectedPort = 0U;
+        }
+
+        ASSERT_EQ(expectedAddress, msection.GetConnection().GetAddress())
+          << context << " (level " << msection.GetLevel() << ")";
+        ASSERT_EQ(expectedPort, msection.GetPort())
+          << context << " (level " << msection.GetLevel() << ")";
       }
 
       void CheckDefaultRtcpCandidate(bool expectDefault,
                                      const SdpMediaSection& msection,
                                      size_t transportLevel,
                                      const std::string& context) const
       {
         if (expectDefault) {
@@ -1069,21 +1074,25 @@ private:
       auto& msection = sdp->GetMediaSection(i);
 
       if (msection.GetMediaType() == SdpMediaSection::kApplication) {
         ASSERT_EQ(SdpMediaSection::kDtlsSctp, msection.GetProtocol());
       } else {
         ASSERT_EQ(SdpMediaSection::kUdpTlsRtpSavpf, msection.GetProtocol());
       }
 
-      if (msection.GetPort() == 0) {
+      const SdpAttributeList& attrs = msection.GetAttributeList();
+      bool bundle_only = attrs.HasAttribute(SdpAttribute::kBundleOnlyAttribute);
+
+      // port 0 only means disabled when the bundle-only attribute is missing
+      if (!bundle_only && msection.GetPort() == 0) {
         ValidateDisabledMSection(&msection);
         continue;
       }
-      const SdpAttributeList& attrs = msection.GetAttributeList();
+
       ASSERT_EQ(source.mIceUfrag, attrs.GetIceUfrag());
       ASSERT_EQ(source.mIcePwd, attrs.GetIcePwd());
       const SdpFingerprintAttributeList& fps = attrs.GetFingerprint();
       for (auto fp = fps.mFingerprints.begin(); fp != fps.mFingerprints.end();
            ++fp) {
         std::string alg_str = "None";
 
         if (fp->hashFunc == SdpFingerprintAttributeList::kSha1) {
@@ -4147,20 +4156,22 @@ TEST_P(JsepSessionTest, TestMaxBundle)
   std::string offer = mSessionOff.GetLocalDescription();
   SipccSdpParser parser;
   UniquePtr<Sdp> parsedOffer = parser.Parse(offer);
   ASSERT_TRUE(parsedOffer.get());
 
   ASSERT_FALSE(
       parsedOffer->GetMediaSection(0).GetAttributeList().HasAttribute(
         SdpAttribute::kBundleOnlyAttribute));
+  ASSERT_NE(0U, parsedOffer->GetMediaSection(0).GetPort());
   for (size_t i = 1; i < parsedOffer->GetMediaSectionCount(); ++i) {
     ASSERT_TRUE(
         parsedOffer->GetMediaSection(i).GetAttributeList().HasAttribute(
           SdpAttribute::kBundleOnlyAttribute));
+    ASSERT_EQ(0U, parsedOffer->GetMediaSection(i).GetPort());
   }
 
 
   CheckPairs(mSessionOff, "Offerer pairs");
   CheckPairs(mSessionAns, "Answerer pairs");
   EXPECT_EQ(1U, GetActiveTransportCount(mSessionOff));
   EXPECT_EQ(1U, GetActiveTransportCount(mSessionAns));
 }
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -661,16 +661,18 @@ JsepSessionImpl::SetupBundle(Sdp* sdp) c
           // m-section
           useBundleOnly = !mids.empty();
           break;
       }
 
       if (useBundleOnly) {
         attrs.SetAttribute(
             new SdpFlagAttribute(SdpAttribute::kBundleOnlyAttribute));
+        // Set port to 0 for sections with bundle-only attribute. (mjf)
+        sdp->GetMediaSection(i).SetPort(0);
       }
 
       mids.push_back(attrs.GetMid());
     }
   }
 
   if (mids.size() > 1) {
     UniquePtr<SdpGroupAttributeList> groupAttr(new SdpGroupAttributeList);
--- a/media/webrtc/signaling/src/sdp/SdpHelper.cpp
+++ b/media/webrtc/signaling/src/sdp/SdpHelper.cpp
@@ -419,24 +419,29 @@ SdpHelper::SetDefaultAddresses(const std
 void
 SdpHelper::SetDefaultAddresses(const std::string& defaultCandidateAddr,
                                uint16_t defaultCandidatePort,
                                const std::string& defaultRtcpCandidateAddr,
                                uint16_t defaultRtcpCandidatePort,
                                SdpMediaSection* msection)
 {
   msection->GetConnection().SetAddress(defaultCandidateAddr);
-  msection->SetPort(defaultCandidatePort);
+  SdpAttributeList& attrList = msection->GetAttributeList();
+
+  // only set the port if there is no bundle-only attribute
+  if (!attrList.HasAttribute(SdpAttribute::kBundleOnlyAttribute)) {
+    msection->SetPort(defaultCandidatePort);
+  }
 
   if (!defaultRtcpCandidateAddr.empty()) {
     sdp::AddrType ipVersion = sdp::kIPv4;
     if (defaultRtcpCandidateAddr.find(':') != std::string::npos) {
       ipVersion = sdp::kIPv6;
     }
-    msection->GetAttributeList().SetAttribute(new SdpRtcpAttribute(
+    attrList.SetAttribute(new SdpRtcpAttribute(
           defaultRtcpCandidatePort,
           sdp::kInternet,
           ipVersion,
           defaultRtcpCandidateAddr));
   }
 }
 
 nsresult