Bug 170192 - [WebRTC] After r214441 addIceCandidate not longer accepts an RTCIceCandidateInit dictionary
Summary: [WebRTC] After r214441 addIceCandidate not longer accepts an RTCIceCandidateI...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks: 170118
  Show dependency treegraph
 
Reported: 2017-03-28 12:56 PDT by Carlos Alberto Lopez Perez
Modified: 2017-03-29 08:33 PDT (History)
8 users (show)

See Also:


Attachments
Patch (12.01 KB, patch)
2017-03-28 16:00 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (11.86 KB, patch)
2017-03-28 20:27 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2017-03-28 12:56:54 PDT
I have detected this layout test failure on the WebKitGTK+ (OpenWebRTC) port caused by r214441 <https://trac.webkit.org/r214441>


$ diff -U100 -u LayoutTests/platform/gtk/fast/mediastream/RTCPeerConnection-addIceCandidate-expected.txt layout-test-results/fast/mediastream/RTCPeerConnection-addIceCandidate-actual.txt 
--- LayoutTests/platform/gtk/fast/mediastream/RTCPeerConnection-addIceCandidate-expected.txt	2017-03-16 17:49:20.780620966 +0100
+++ layout-test-results/fast/mediastream/RTCPeerConnection-addIceCandidate-actual.txt	2017-03-28 21:30:45.797590203 +0200
@@ -1,34 +1,34 @@
 Test behavior of RTCPeerConnection.addIceCandidate
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
 
 *** A remote description is needed before a candidate can be added
 PASS pc.remoteDescription is null
 PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: 'foo', sdpMid: 0})) rejected with InvalidStateError (DOM Exception 11): No remote description set
 PASS Remote description set
 
 *** Define sdpMid, badSdpMid, sdpMLineIndex and badSdpMLineIndex for testing
 PASS sdpMLineIndex is not badSdpMLineIndex
 PASS sdpMid is not null
 PASS sdpMid is not badSdpMid
 PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMid: badSdpMid})) rejected with OperationError (DOM Exception 34): sdpMid did not match any media description
 PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMLineIndex: badSdpMLineIndex})) rejected with OperationError (DOM Exception 34): sdpMLineIndex is out of range
 *** A (bad) sdpMid takes precedesce over valid sdpMLineIndex
 PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMid: badSdpMid, sdpMLineIndex: sdpMLineIndex})) rejected with OperationError (DOM Exception 34): sdpMid did not match any media description
 *** Test bad candidate content with valid sdpMid
 PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: 'bad content', sdpMid: sdpMid})) rejected with OperationError (DOM Exception 34): Invalid candidate content
 *** Test bad candidate content with valid sdpMLineIndex
 PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: 'bad content', sdpMLineIndex: sdpMLineIndex})) rejected with OperationError (DOM Exception 34): Invalid candidate content
 
 *** Test some OK input
-PASS promise pc.addIceCandidate({candidate: validCandidate, sdpMid: sdpMid}) fulfilled with undefined
+FAIL promise pc.addIceCandidate({candidate: validCandidate, sdpMid: sdpMid}) rejected unexpectedly.
 PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMLineIndex: sdpMLineIndex})) fulfilled with undefined
 *** A valid sdpMid takes precedesce over a bad sdpMLineIndex
 PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMid: sdpMid, sdpMLineIndex: badSdpMLineIndex})) fulfilled with undefined
 PASS End of test promise chain
 PASS successfullyParsed is true
 
 TEST COMPLETE



Note that the GTK port has a different expectation for this test than the mac port.


The test tries to call RTCPeerConnection.addIceCandidate() passing a Dictionary with some of the values allowed as per the definition of RTCIceCandidateInit here: https://www.w3.org/TR/webrtc/#dom-rtcicecandidateinit

From LayoutTests/fast/mediastream/RTCPeerConnection-addIceCandidate.html:
[...]
            .then(function () {
                debug("<br>*** Test some OK input");
                // Testing passing a RTCIceCandidateInit
                return promiseShouldResolve("pc.addIceCandidate({candidate: validCandidate, sdpMid: sdpMid})");
[...]

My understanding is that the promise should work. And it was working before r214441.
I double-checked it: reverting r214441 locally makes the test pass back again.
Comment 1 youenn fablet 2017-03-28 13:11:27 PDT
>  *** Test some OK input
> -PASS promise pc.addIceCandidate({candidate: validCandidate, sdpMid:
> sdpMid}) fulfilled with undefined
> +FAIL promise pc.addIceCandidate({candidate: validCandidate, sdpMid:
> sdpMid}) rejected unexpectedly.

This should be working. I'll check that.
Comment 2 youenn fablet 2017-03-28 16:00:28 PDT
Created attachment 305658 [details]
Patch
Comment 3 Jon Lee 2017-03-28 17:25:10 PDT
Comment on attachment 305658 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=305658&action=review

> Source/WebCore/ChangeLog:3
> +        [WebRTC] After r214441 addIceCandidate not longer accepts an RTCIceCandidateInit dictionary

Typo: no* longer
Comment 4 youenn fablet 2017-03-28 20:27:57 PDT
Created attachment 305701 [details]
Patch for landing
Comment 5 WebKit Commit Bot 2017-03-28 21:09:18 PDT
Comment on attachment 305701 [details]
Patch for landing

Clearing flags on attachment: 305701

Committed r214527: <http://trac.webkit.org/changeset/214527>
Comment 6 WebKit Commit Bot 2017-03-28 21:09:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Carlos Alberto Lopez Perez 2017-03-29 08:33:25 PDT
Committed r214535: <http://trac.webkit.org/changeset/214535>