Bug 1095793 - use mid if provided to place candidate in msection. r=bwc
authorMichael Froman <mfroman@mozilla.com>
Thu, 10 Sep 2015 13:20:09 -0500
changeset 295616 372b1c30dc09b14aeb78d1dd61fbcdb48811c767
parent 295615 3560ae11e7eb40732be6f2b5e9c0b1eb69c84849
child 295617 81a35d84fb974c3932071b28bbe9c1c738a1a9f0
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1095793
milestone43.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 1095793 - use mid if provided to place candidate in msection. r=bwc
dom/media/tests/mochitest/test_peerConnection_addIceCandidate.html
media/webrtc/signaling/src/sdp/SdpHelper.cpp
media/webrtc/signaling/test/jsep_session_unittest.cpp
--- a/dom/media/tests/mochitest/test_peerConnection_addIceCandidate.html
+++ b/dom/media/tests/mochitest/test_peerConnection_addIceCandidate.html
@@ -3,17 +3,17 @@
 <head>
   <script type="application/javascript" src="pc.js"></script>
 </head>
 <body>
 <pre id="test">
 <script type="application/javascript">
   createHTML({
     bug: "1087551",
-    title: "addCandidate behavior in different states"
+    title: "addIceCandidate behavior (local and remote) including invalid data"
   });
 
   var test;
   runNetworkTest(function () {
     test = new PeerConnectionTest();
     test.setMediaConstraints([{audio: true}], [{audio: true}]);
     test.chain.removeAfter("PC_LOCAL_GET_ANSWER");
 
@@ -66,16 +66,38 @@
         );
       },
       function PC_REMOTE_ADD_VALID_CANDIDATE(test) {
         var candidate = new mozRTCIceCandidate(
           {candidate:"candidate:1 1 UDP 2130706431 192.168.2.1 50005 typ host",
            sdpMLineIndex: 0});
         return test.pcRemote._pc.addIceCandidate(candidate)
         .then(ok(true, "Successfully added valid ICE candidate"));
+      },
+      // bug 1095793
+      function PC_REMOTE_ADD_MISMATCHED_MID_AND_LEVEL_CANDIDATE(test) {
+        var bogus = new mozRTCIceCandidate(
+          {candidate:"candidate:1 1 UDP 2130706431 192.168.2.1 50005 typ host",
+           sdpMLineIndex: 0,
+           sdpMid: "sdparta_1"});
+        return test.pcRemote._pc.addIceCandidate(bogus)
+        .then(
+          generateErrorCallback("addIceCandidate should have failed."),
+          err => {
+            is(err.name, "InvalidCandidateError", "Error is InvalidCandidateError");
+          }
+        );
+      },
+      function PC_REMOTE_ADD_MATCHING_MID_AND_LEVEL_CANDIDATE(test) {
+        var candidate = new mozRTCIceCandidate(
+          {candidate:"candidate:1 1 UDP 2130706431 192.168.2.1 50005 typ host",
+           sdpMLineIndex: 0,
+           sdpMid: "sdparta_0"});
+        return test.pcRemote._pc.addIceCandidate(candidate)
+        .then(ok(true, "Successfully added valid ICE candidate with matching mid and level"));
       }
     ]);
     test.run();
   });
 </script>
 </pre>
 </body>
 </html>
--- a/media/webrtc/signaling/src/sdp/SdpHelper.cpp
+++ b/media/webrtc/signaling/src/sdp/SdpHelper.cpp
@@ -265,20 +265,44 @@ SdpHelper::AddCandidateToSdp(Sdp* sdp,
   if (begin == std::string::npos) {
     SDP_SET_ERROR("Invalid candidate, no ':' (" << candidateUntrimmed << ")");
     return NS_ERROR_INVALID_ARG;
   }
   ++begin;
 
   std::string candidate = candidateUntrimmed.substr(begin);
 
-  // TODO(bug 1095793): mid
+  // https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-11#section-3.4.2.1
+  // Implementations receiving an ICE Candidate object MUST use the MID if
+  // present, or the m= line index, if not (as it could have come from a
+  // non-JSEP endpoint). (bug 1095793)
+  SdpMediaSection* msection = 0;
+  if (!mid.empty()) {
+    // FindMsectionByMid could return nullptr
+    msection = FindMsectionByMid(*sdp, mid);
 
-  SdpMediaSection& msection = sdp->GetMediaSection(level);
-  SdpAttributeList& attrList = msection.GetAttributeList();
+    // Check to make sure mid matches what we'd get by
+    // looking up the m= line using the level. (mjf)
+    std::string checkMid;
+    nsresult rv = GetMidFromLevel(*sdp, level, &checkMid);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+    if (mid != checkMid) {
+      SDP_SET_ERROR("Mismatch between mid and level - \"" << mid
+                     << "\" is not the mid for level " << level
+                     << "; \"" << checkMid << "\" is");
+      return NS_ERROR_INVALID_ARG;
+    }
+  }
+  if (!msection) {
+    msection = &(sdp->GetMediaSection(level));
+  }
+
+  SdpAttributeList& attrList = msection->GetAttributeList();
 
   UniquePtr<SdpMultiStringAttribute> candidates;
   if (!attrList.HasAttribute(SdpAttribute::kCandidateAttribute)) {
     // Create new
     candidates.reset(
         new SdpMultiStringAttribute(SdpAttribute::kCandidateAttribute));
   } else {
     // Copy existing
--- a/media/webrtc/signaling/test/jsep_session_unittest.cpp
+++ b/media/webrtc/signaling/test/jsep_session_unittest.cpp
@@ -7,16 +7,17 @@
 #include <iostream>
 #include <map>
 
 #include "nspr.h"
 #include "nss.h"
 #include "ssl.h"
 
 #include "mozilla/RefPtr.h"
+#include "mozilla/Tuple.h"
 
 #define GTEST_HAS_RTTI 0
 #include "gtest/gtest.h"
 #include "gtest_utils.h"
 
 #include "FakeMediaStreams.h"
 #include "FakeMediaStreamsImpl.h"
 
@@ -609,20 +610,19 @@ protected:
           std::ostringstream candidate;
           candidate << "0 " << static_cast<uint16_t>(component)
                     << " UDP 9999 192.168.0.1 " << port << " typ host";
           std::string mid;
           bool skipped;
           session.AddLocalIceCandidate(kAEqualsCandidate + candidate.str(),
                                        level, &mid, &skipped);
           if (!skipped) {
-            // TODO (bug 1095793): Need to add mid to mCandidatesToTrickle
             mCandidatesToTrickle.push_back(
-                std::pair<uint16_t, std::string>(
-                  level, kAEqualsCandidate + candidate.str()));
+                Tuple<Level, Mid, Candidate>(
+                  level, mid, kAEqualsCandidate + candidate.str()));
             candidates.push_back(candidate.str());
           }
         }
 
         // Stomp existing candidates
         mCandidates[level][component] = candidates;
 
         // Stomp existing defaults
@@ -643,20 +643,22 @@ protected:
               levelAndCandidates.second[RTCP].first,
               levelAndCandidates.second[RTCP].second,
               levelAndCandidates.first);
         }
       }
 
       void Trickle(JsepSession& session)
       {
-        for (const auto& levelAndCandidate : mCandidatesToTrickle) {
-          session.AddRemoteIceCandidate(levelAndCandidate.second,
-                                        "",
-                                        levelAndCandidate.first);
+        for (const auto& levelMidAndCandidate : mCandidatesToTrickle) {
+          Level level;
+          Mid mid;
+          Candidate candidate;
+          Tie(level, mid, candidate) = levelMidAndCandidate;
+          session.AddRemoteIceCandidate(candidate, mid, level);
         }
         mCandidatesToTrickle.clear();
       }
 
       void CheckRtpCandidates(bool expectRtpCandidates,
                               const SdpMediaSection& msection,
                               size_t transportLevel,
                               const std::string& context) const
@@ -757,29 +759,30 @@ protected:
           ASSERT_FALSE(msection.GetAttributeList().HasAttribute(
                 SdpAttribute::kRtcpAttribute))
             << context << " (level " << msection.GetLevel() << ")";
         }
       }
 
     private:
       typedef size_t Level;
+      typedef std::string Mid;
       typedef std::string Candidate;
       typedef std::string Address;
       typedef uint16_t Port;
       // Default candidates are put into the m-line, c-line, and rtcp
       // attribute for endpoints that don't support ICE.
       std::map<Level,
                std::map<ComponentType,
                         std::pair<Address, Port>>> mDefaultCandidates;
       std::map<Level,
                std::map<ComponentType,
                         std::vector<Candidate>>> mCandidates;
-      // Level/candidate pairs that need to be trickled
-      std::vector<std::pair<Level, Candidate>> mCandidatesToTrickle;
+      // Level/mid/candidate tuples that need to be trickled
+      std::vector<Tuple<Level, Mid, Candidate>> mCandidatesToTrickle;
   };
 
   // For streaming parse errors
   std::string
   GetParseErrors(const SipccSdpParser& parser) const
   {
     std::stringstream output;
     for (auto e = parser.GetParseErrors().begin();