Bug 1318167 - Part 3: Add some testing for a=end-of-candidates in remote SDP. r=jib
authorByron Campen [:bwc] <docfaraday@gmail.com>
Tue, 19 Mar 2019 16:48:25 +0000
changeset 465095 b425ca25edc52da94ae2fc65a571ea054c46ac4f
parent 465094 6d0193a41ab49b89b9e31462c7d1f3581d6da0d2
child 465096 151f1392f61900a67514478a8e48024cf5166462
push id35732
push useropoprus@mozilla.com
push dateWed, 20 Mar 2019 10:52:37 +0000
treeherdermozilla-central@708979f9c3f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersend-of-candidates, jib
bugs1318167
milestone68.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 1318167 - Part 3: Add some testing for a=end-of-candidates in remote SDP. r=jib Differential Revision: https://phabricator.services.mozilla.com/D23591
testing/web-platform/meta/webrtc/RTCPeerConnection-addIceCandidate.html.ini
testing/web-platform/tests/webrtc/RTCPeerConnection-addIceCandidate.html
--- a/testing/web-platform/meta/webrtc/RTCPeerConnection-addIceCandidate.html.ini
+++ b/testing/web-platform/meta/webrtc/RTCPeerConnection-addIceCandidate.html.ini
@@ -8,14 +8,26 @@
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1535410
 
   [Add candidate with invalid candidate string and both sdpMid and sdpMLineIndex null should reject with TypeError]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1535410
 
   [Add candidate with invalid usernameFragment should reject with OperationError]
     expected: FAIL
-    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1490658
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1535442
 
   [Add candidate with sdpMid belonging to different usernameFragment should reject with OperationError]
     expected: FAIL
-    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1490658
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1535442
+
+  [addIceCandidate({usernameFragment: usernameFragment1}) should work, and add a=end-of-candidates to the first m-section]
+    expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1535442
 
+  [addIceCandidate({usernameFragment: usernameFragment2}) should work, and add a=end-of-candidates to the first m-section]
+    expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1535442
+
+  [addIceCandidate({usernameFragment: "no such ufrag"}) should work, but not add a=end-of-candidates]
+    expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1535442
+
--- a/testing/web-platform/tests/webrtc/RTCPeerConnection-addIceCandidate.html
+++ b/testing/web-platform/tests/webrtc/RTCPeerConnection-addIceCandidate.html
@@ -80,38 +80,45 @@ a=rtcp-rsize
   const candidateLine2 = `a=${candidateStr2}`;
   const endOfCandidateLine = 'a=end-of-candidates';
 
   // Copied from MDN
   function escapeRegExp(string) {
     return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
   }
 
-  // Check that a candidate line is found after the first media line
-  // but before the second, i.e. it belongs to the first media stream
-  function assert_candidate_line_between(sdp, beforeMediaLine, candidateLine, afterMediaLine) {
+  function is_candidate_line_between(sdp, beforeMediaLine, candidateLine, afterMediaLine) {
     const line1 = escapeRegExp(beforeMediaLine);
     const line2 = escapeRegExp(candidateLine);
     const line3 = escapeRegExp(afterMediaLine);
 
     const regex = new RegExp(`${line1}[^]+${line2}[^]+${line3}`);
+    return regex.test(sdp);
+  }
 
-    assert_true(regex.test(sdp),
+  // Check that a candidate line is found after the first media line
+  // but before the second, i.e. it belongs to the first media stream
+  function assert_candidate_line_between(sdp, beforeMediaLine, candidateLine, afterMediaLine) {
+    assert_true(is_candidate_line_between(sdp, beforeMediaLine, candidateLine, afterMediaLine),
       `Expect candidate line to be found between media lines ${beforeMediaLine} and ${afterMediaLine}`);
   }
 
   // Check that a candidate line is found after the second media line
   // i.e. it belongs to the second media stream
-  function assert_candidate_line_after(sdp, beforeMediaLine, candidateLine) {
+  function is_candidate_line_after(sdp, beforeMediaLine, candidateLine) {
     const line1 = escapeRegExp(beforeMediaLine);
     const line2 = escapeRegExp(candidateLine);
 
     const regex = new RegExp(`${line1}[^]+${line2}`);
 
-    assert_true(regex.test(sdp),
+    return regex.test(sdp);
+  }
+
+  function assert_candidate_line_after(sdp, beforeMediaLine, candidateLine) {
+    assert_true(is_candidate_line_after(sdp, beforeMediaLine, candidateLine),
       `Expect candidate line to be found after media line ${beforeMediaLine}`);
   }
 
   /*
     4.4.2.  addIceCandidate
       4.  Return the result of enqueuing the following steps:
         1.  If remoteDescription is null return a promise rejected with a
             newly created InvalidStateError.
@@ -128,63 +135,84 @@ a=rtcp-rsize
         sdpMLineIndex: sdpMLineIndex1,
         usernameFragment: usernameFragment1
       }));
   }, 'Add ICE candidate before setting remote description should reject with InvalidStateError');
 
   /*
     Success cases
    */
-  // The RTCIceCandidateInit arg to addIceCandidate is optional, and is converted
-  // to the default RTCIceCandidateInit. Then, because the default candidate is
-  // "", addIceCandidate will allow both sdpMid and sdpMLineIndex to be absent.
-  promise_test(async t => {
+
+  // All of these should work, because all of these end up being equivalent to the
+  // same thing; an end-of-candidates signal for all levels/mids/ufrags.
+  [
+    // This is just the default. Everything else here is equivalent to this.
+    {
+      candidate: '',
+      sdpMid: null,
+      sdpMLineIndex: null,
+      usernameFragment: undefined
+    },
+    // The arg is optional, so when passing undefined we'll just get the default
+    undefined,
+    // The arg is optional, but not nullable, so we get the default again
+    null,
+    // Members in the dictionary take their default values
+    {}
+  ].forEach(init => promise_test(async t => {
     const pc = new RTCPeerConnection();
 
     t.add_cleanup(() => pc.close());
 
     await pc.setRemoteDescription(sessionDesc);
     await pc.addIceCandidate();
-  }, 'addIceCandidate() should work');
-
-  // The RTCIceCandidateInit arg to addIceCandidate is optional, and not
-  // nullable, so null is converted to the default RTCIceCandidateInit. Then,
-  // because the default candidate is "", addIceCandidate will allow both
-  // sdpMid and sdpMLineIndex to be absent.
-  promise_test(async t => {
-    const pc = new RTCPeerConnection();
-
-    t.add_cleanup(() => pc.close());
-
-    await pc.setRemoteDescription(sessionDesc);
-    await pc.addIceCandidate(null);
-  }, 'Add null candidate should work');
+    assert_candidate_line_between(pc.remoteDescription.sdp,
+      mediaLine1, endOfCandidateLine, mediaLine2);
+    assert_candidate_line_after(pc.remoteDescription.sdp,
+      mediaLine2, endOfCandidateLine);
+  }, `addIceCandidate(${JSON.stringify(init)}) should work, and add a=end-of-candidates to both m-sections`));
 
   promise_test(async t => {
     const pc = new RTCPeerConnection();
 
     t.add_cleanup(() => pc.close());
 
     await pc.setRemoteDescription(sessionDesc);
-    await pc.addIceCandidate({});
-  }, 'Add candidate with empty dict should work');
+    await pc.addIceCandidate({usernameFragment: usernameFragment1});
+    assert_candidate_line_between(pc.remoteDescription.sdp,
+      mediaLine1, endOfCandidateLine, mediaLine2);
+    assert_false(is_candidate_line_after(pc.remoteDescription.sdp,
+      mediaLine2, endOfCandidateLine));
+  }, 'addIceCandidate({usernameFragment: usernameFragment1}) should work, and add a=end-of-candidates to the first m-section');
 
   promise_test(async t => {
     const pc = new RTCPeerConnection();
 
     t.add_cleanup(() => pc.close());
 
     await pc.setRemoteDescription(sessionDesc);
-    await pc.addIceCandidate({
-          candidate: '',
-          sdpMid: null,
-          sdpMLineIndex: null,
-          usernameFragment: undefined
-    });
-  }, 'Add candidate with manually filled default values should work');
+    await pc.addIceCandidate({usernameFragment: usernameFragment2});
+    assert_false(is_candidate_line_between(pc.remoteDescription.sdp,
+      mediaLine1, endOfCandidateLine, mediaLine2));
+    assert_true(is_candidate_line_after(pc.remoteDescription.sdp,
+      mediaLine2, endOfCandidateLine));
+  }, 'addIceCandidate({usernameFragment: usernameFragment2}) should work, and add a=end-of-candidates to the first m-section');
+
+  promise_test(async t => {
+    const pc = new RTCPeerConnection();
+
+    t.add_cleanup(() => pc.close());
+
+    await pc.setRemoteDescription(sessionDesc);
+    await pc.addIceCandidate({usernameFragment: "no such ufrag"});
+    assert_false(is_candidate_line_between(pc.remoteDescription.sdp,
+      mediaLine1, endOfCandidateLine, mediaLine2));
+    assert_false(is_candidate_line_after(pc.remoteDescription.sdp,
+      mediaLine2, endOfCandidateLine));
+  }, 'addIceCandidate({usernameFragment: "no such ufrag"}) should work, but not add a=end-of-candidates');
 
   promise_test(t => {
     const pc = new RTCPeerConnection();
 
     t.add_cleanup(() => pc.close());
 
     return pc.setRemoteDescription(sessionDesc)
     .then(() => pc.addIceCandidate({