We should throw if no parameter is passed.
Created attachment 305526 [details] Patch
Comment on attachment 305526 [details] Patch r=me if the bots are happy.
(In reply to Chris Dumez from comment #2) > Comment on attachment 305526 [details] > Patch > > r=me if the bots are happy. Right, I updated the test expectation by hand...
(In reply to youenn fablet from comment #3) > (In reply to Chris Dumez from comment #2) > > Comment on attachment 305526 [details] > > Patch > > > > r=me if the bots are happy. > > Right, I updated the test expectation by hand... lol
Comment on attachment 305526 [details] Patch Attachment 305526 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3420597 New failing tests: imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl.html
Created attachment 305536 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
--- /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/retries/imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl-expected.txt +++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/retries/imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl-actual.txt @@ -23,7 +23,7 @@ PASS RTCPeerConnection interface: attribute remoteDescription PASS RTCPeerConnection interface: attribute currentRemoteDescription PASS RTCPeerConnection interface: attribute pendingRemoteDescription -PASS RTCPeerConnection interface: operation addIceCandidate([object Object],[object Object]) +PASS RTCPeerConnection interface: operation addIceCandidate([object Object],[object Object]) PASS RTCPeerConnection interface: attribute signalingState PASS RTCPeerConnection interface: attribute iceGatheringState PASS RTCPeerConnection interface: attribute iceConnectionState @@ -44,7 +44,9 @@ fn.apply(obj, args); }" did not throw FAIL RTCPeerConnection interface: operation setRemoteDescription(RTCSessionDescription,VoidFunction,RTCPeerConnectionErrorCallback) assert_equals: property has wrong .length expected 1 but got 0 -PASS RTCPeerConnection interface: operation addIceCandidate(RTCIceCandidate,VoidFunction,RTCPeerConnectionErrorCallback) +FAIL RTCPeerConnection interface: operation addIceCandidate(RTCIceCandidate,VoidFunction,RTCPeerConnectionErrorCallback) assert_throws: calling operation with this = null didn't throw TypeError function "function () { + fn.apply(obj, args); + }" did not throw FAIL RTCPeerConnection interface: operation getStats(MediaStreamTrack,RTCStatsCallback,RTCPeerConnectionErrorCallback) assert_throws: calling operation with this = null didn't throw TypeError function "function () { fn.apply(obj, args); }" did not throw @@ -79,7 +81,7 @@ PASS RTCPeerConnection interface: pc must inherit property "currentRemoteDescription" with the proper type (8) PASS RTCPeerConnection interface: pc must inherit property "pendingRemoteDescription" with the proper type (9) PASS RTCPeerConnection interface: pc must inherit property "addIceCandidate" with the proper type (10) -PASS RTCPeerConnection interface: calling addIceCandidate([object Object],[object Object]) on pc with too few arguments must throw TypeError +PASS RTCPeerConnection interface: calling addIceCandidate([object Object],[object Object]) on pc with too few arguments must throw TypeError FAIL RTCPeerConnection interface: pc must inherit property "signalingState" with the proper type (11) Unrecognized type RTCSignalingState FAIL RTCPeerConnection interface: pc must inherit property "iceGatheringState" with the proper type (12) Unrecognized type RTCIceGatheringState FAIL RTCPeerConnection interface: pc must inherit property "iceConnectionState" with the proper type (13) Unrecognized type RTCIceConnectionState @@ -106,7 +108,7 @@ fn.apply(obj, args); }" did not throw PASS RTCPeerConnection interface: pc must inherit property "addIceCandidate" with the proper type (27) -PASS RTCPeerConnection interface: calling addIceCandidate(RTCIceCandidate,VoidFunction,RTCPeerConnectionErrorCallback) on pc with too few arguments must throw TypeError +FAIL RTCPeerConnection interface: calling addIceCandidate(RTCIceCandidate,VoidFunction,RTCPeerConnectionErrorCallback) on pc with too few arguments must throw TypeError assert_throws: Called with 0 arguments function "function () { fn.apply(obj, args); }" did not throw PASS RTCPeerConnection interface: pc must inherit property "getStats" with the proper type (28)
Created attachment 305538 [details] Patch
(In reply to youenn fablet from comment #8) > Created attachment 305538 [details] > Patch This one should have a better expectation...
Created attachment 305626 [details] Patch for landing
(In reply to youenn fablet from comment #10) > Created attachment 305626 [details] > Patch for landing Retrying EWS since it was submitted at a bad time apparently.
r214441 <https://trac.webkit.org/r214441> Also caused this failure on the WebKitGTK port (OpenWebRTC) on the test fast/mediastream/RTCPeerConnection-overloaded-operations-params.html --- /home/clopez/webkit/webkit/layout-test-results/fast/mediastream/RTCPeerConnection-overloaded-operations-params-expected.txt +++ /home/clopez/webkit/webkit/layout-test-results/fast/mediastream/RTCPeerConnection-overloaded-operations-params-actual.txt @@ -80,10 +80,10 @@ PASS promise pc.addIceCandidate(candidate) did not reject with TypeError. PASS promise pc.addIceCandidate(candidate, emptyFunc, emptyFunc) did not reject with TypeError. *** candidate is not optional -PASS promise pc.addIceCandidate() rejected with TypeError: Not enough arguments +FAIL promise pc.addIceCandidate() fulfilled unexpectedly. *** candidate is not nullable -PASS promise pc.addIceCandidate(null) rejected with TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of RTCIceCandidate -PASS promise pc.addIceCandidate(null, emptyFunc, emptyFunc) rejected with TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of RTCIceCandidate +FAIL promise pc.addIceCandidate(null) fulfilled unexpectedly. +FAIL promise pc.addIceCandidate(null, emptyFunc, emptyFunc) fulfilled unexpectedly. PASS promise pc.addIceCandidate(1) rejected with TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of RTCIceCandidate *** Bad input as candidate PASS promise pc.addIceCandidate('foo') rejected with TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of RTCIceCandidate Note that the test fast/mediastream/RTCPeerConnection-overloaded-operations-params.html is Skipped on all ports but the WebKitGTK+ one.
Comment on attachment 305626 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=305626&action=review > LayoutTests/imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl-expected.txt:49 > +FAIL RTCPeerConnection interface: operation addIceCandidate(RTCIceCandidate,VoidFunction,RTCPeerConnectionErrorCallback) assert_throws: calling operation with this = null didn't throw TypeError function "function () { > + fn.apply(obj, args); > + }" did not throw Does the test need to be updated or is this correct?
The patch that is about to land reduces the test diff to this: $ cat layout-test-results/fast/mediastream/RTCPeerConnection-overloaded-operations-params-diff.txt --- /home/clopez/webkit/webkit/layout-test-results/fast/mediastream/RTCPeerConnection-overloaded-operations-params-expected.txt +++ /home/clopez/webkit/webkit/layout-test-results/fast/mediastream/RTCPeerConnection-overloaded-operations-params-actual.txt @@ -82,8 +82,8 @@ *** candidate is not optional PASS promise pc.addIceCandidate() rejected with TypeError: Not enough arguments *** candidate is not nullable -PASS promise pc.addIceCandidate(null) rejected with TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of RTCIceCandidate -PASS promise pc.addIceCandidate(null, emptyFunc, emptyFunc) rejected with TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of RTCIceCandidate +FAIL promise pc.addIceCandidate(null) fulfilled unexpectedly. +FAIL promise pc.addIceCandidate(null, emptyFunc, emptyFunc) fulfilled unexpectedly. PASS promise pc.addIceCandidate(1) rejected with TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of RTCIceCandidate *** Bad input as candidate PASS promise pc.addIceCandidate('foo') rejected with TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of RTCIceCandidate
Comment on attachment 305626 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=305626&action=review >> LayoutTests/imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl-expected.txt:49 >> + }" did not throw > > Does the test need to be updated or is this correct? I think the test is wrong here. The spec says: Promise<void> addIceCandidate((RTCIceCandidateInit or RTCIceCandidate) candidate, VoidFunction successCallback, RTCPeerConnectionErrorCallback failureCallback); But the test's IDL returns a void, not a Promise<void>.
(In reply to Eric Carlson from comment #13) > Comment on attachment 305626 [details] > Patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=305626&action=review > > > LayoutTests/imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl-expected.txt:49 > > +FAIL RTCPeerConnection interface: operation addIceCandidate(RTCIceCandidate,VoidFunction,RTCPeerConnectionErrorCallback) assert_throws: calling operation with this = null didn't throw TypeError function "function () { > > + fn.apply(obj, args); > > + }" did not throw > > Does the test need to be updated or is this correct? As discussed with Chris, it needs to be updated since it is returning a Promise. In the end, I hope we will be able to remove support for these legacies
> -PASS promise pc.addIceCandidate(null) rejected with TypeError: Argument 1 > ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of > RTCIceCandidate > -PASS promise pc.addIceCandidate(null, emptyFunc, emptyFunc) rejected with > TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate > must be an instance of RTCIceCandidate > +FAIL promise pc.addIceCandidate(null) fulfilled unexpectedly. > +FAIL promise pc.addIceCandidate(null, emptyFunc, emptyFunc) fulfilled > unexpectedly. We might want to check with other browsers, but I think it is ok to do addIceCandidate(null). It will mean that there is no more ice candidate, just as would be done with addIceCandidate({ })
(In reply to youenn fablet from comment #17) > > -PASS promise pc.addIceCandidate(null) rejected with TypeError: Argument 1 > > ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of > > RTCIceCandidate > > -PASS promise pc.addIceCandidate(null, emptyFunc, emptyFunc) rejected with > > TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate > > must be an instance of RTCIceCandidate > > +FAIL promise pc.addIceCandidate(null) fulfilled unexpectedly. > > +FAIL promise pc.addIceCandidate(null, emptyFunc, emptyFunc) fulfilled > > unexpectedly. > > We might want to check with other browsers, but I think it is ok to do > addIceCandidate(null). It will mean that there is no more ice candidate, > just as would be done with addIceCandidate({ }) That seems right: https://github.com/w3c/webrtc-pc/issues/569 So I guess this test just needs to be rebaselined. I will do that.
Comment on attachment 305626 [details] Patch for landing Clearing flags on attachment: 305626 Committed r214490: <http://trac.webkit.org/changeset/214490>
All reviewed patches have been landed. Closing bug.
(In reply to Carlos Alberto Lopez Perez from comment #18) > (In reply to youenn fablet from comment #17) > > > -PASS promise pc.addIceCandidate(null) rejected with TypeError: Argument 1 > > > ('candidate') to RTCPeerConnection.addIceCandidate must be an instance of > > > RTCIceCandidate > > > -PASS promise pc.addIceCandidate(null, emptyFunc, emptyFunc) rejected with > > > TypeError: Argument 1 ('candidate') to RTCPeerConnection.addIceCandidate > > > must be an instance of RTCIceCandidate > > > +FAIL promise pc.addIceCandidate(null) fulfilled unexpectedly. > > > +FAIL promise pc.addIceCandidate(null, emptyFunc, emptyFunc) fulfilled > > > unexpectedly. > > > > We might want to check with other browsers, but I think it is ok to do > > addIceCandidate(null). It will mean that there is no more ice candidate, > > just as would be done with addIceCandidate({ }) > > That seems right: https://github.com/w3c/webrtc-pc/issues/569 > > So I guess this test just needs to be rebaselined. I will do that. Updated on bug 170223