addIceCandidate should not throw if passed null or undefined
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Mar 2017 23:36:36 +0000 (23:36 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Mar 2017 23:36:36 +0000 (23:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170118

Patch by Youenn Fablet <youenn@apple.com> on 2017-03-27
Reviewed by Eric Carlson.

LayoutTests/imported/w3c:

* web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl-expected.txt:

Source/WebCore:

Covered by updated test.

A null/undefined candidate passed to addIceCandidate means end of Ice candidate..

* Modules/mediastream/PeerConnectionBackend.cpp:
(WebCore::PeerConnectionBackend::addIceCandidate):
* Modules/mediastream/PeerConnectionBackend.h:
(WebCore::PeerConnectionBackend::endOfIceCandidates):
* Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::queuedAddIceCandidate):
* Modules/mediastream/RTCPeerConnection.h:
* Modules/mediastream/RTCPeerConnection.idl:
* Modules/mediastream/RTCPeerConnection.js:
(addIceCandidate):

LayoutTests:

Updating test to log addIceCandidate rejection.

* webrtc/datachannel/basic.html:
* webrtc/routines.js:
(iceCallback1):
(iceCallback2):
(onAddIceCandidateError):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@214441 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl-expected.txt
LayoutTests/imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl.html
LayoutTests/webrtc/datachannel/basic.html
LayoutTests/webrtc/routines.js
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp
Source/WebCore/Modules/mediastream/PeerConnectionBackend.h
Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
Source/WebCore/Modules/mediastream/RTCPeerConnection.h
Source/WebCore/Modules/mediastream/RTCPeerConnection.idl
Source/WebCore/Modules/mediastream/RTCPeerConnection.js

index 8c052e6..72bc347 100644 (file)
@@ -1,3 +1,18 @@
+2017-03-27  Youenn Fablet  <youenn@apple.com>
+
+        addIceCandidate should not throw if passed null or undefined
+        https://bugs.webkit.org/show_bug.cgi?id=170118
+
+        Reviewed by Eric Carlson.
+
+        Updating test to log addIceCandidate rejection.
+
+        * webrtc/datachannel/basic.html:
+        * webrtc/routines.js:
+        (iceCallback1):
+        (iceCallback2):
+        (onAddIceCandidateError):
+
 2017-03-27  Ryan Haddad  <ryanhaddad@apple.com>
 
         Rebaseline svg/css/getComputedStyle-basic.xhtml for macOS.
index 6965696..d299941 100644 (file)
@@ -1,3 +1,12 @@
+2017-03-27  Youenn Fablet  <youenn@apple.com>
+
+        addIceCandidate should not throw if passed null or undefined
+        https://bugs.webkit.org/show_bug.cgi?id=170118
+
+        Reviewed by Eric Carlson.
+
+        * web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl-expected.txt:
+
 2017-03-22  Youenn Fablet  <youenn@apple.com>
 
         Support RTCPeerConnectionState
index e406e60..32950dc 100644 (file)
@@ -23,7 +23,7 @@ FAIL RTCPeerConnection interface: operation setRemoteDescription(RTCSessionDescr
 PASS RTCPeerConnection interface: attribute remoteDescription 
 PASS RTCPeerConnection interface: attribute currentRemoteDescription 
 PASS RTCPeerConnection interface: attribute pendingRemoteDescription 
-FAIL RTCPeerConnection interface: operation addIceCandidate(RTCIceCandidate) assert_equals: property has wrong .length expected 1 but got 0
+FAIL RTCPeerConnection interface: operation addIceCandidate([object Object],[object Object]) assert_equals: property has wrong .length expected 1 but got 0
 PASS RTCPeerConnection interface: attribute signalingState 
 PASS RTCPeerConnection interface: attribute iceGatheringState 
 PASS RTCPeerConnection interface: attribute iceConnectionState 
@@ -79,7 +79,7 @@ PASS RTCPeerConnection interface: pc must inherit property "remoteDescription" w
 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(RTCIceCandidate) on pc with too few arguments must throw TypeError 
+FAIL RTCPeerConnection interface: calling addIceCandidate([object Object],[object Object]) on pc with too few arguments must throw TypeError assert_unreached: Should have rejected: undefined Reached unreachable code
 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
index 7234c54..167b2fd 100644 (file)
@@ -33,7 +33,7 @@ interface RTCPeerConnection : EventTarget  {
     readonly    attribute RTCSessionDescription? remoteDescription;
     readonly    attribute RTCSessionDescription? currentRemoteDescription;
     readonly    attribute RTCSessionDescription? pendingRemoteDescription;
-    Promise<void>                  addIceCandidate (RTCIceCandidate candidate);
+    Promise<void>                  addIceCandidate ((RTCIceCandidateInit or RTCIceCandidate) candidate);
     readonly    attribute RTCSignalingState      signalingState;
     readonly    attribute RTCIceGatheringState   iceGatheringState;
     readonly    attribute RTCIceConnectionState  iceConnectionState;
index 41ea96c..8845f76 100644 (file)
@@ -95,7 +95,7 @@ promise_test((test) => {
                 remoteChannel = event.channel;
                 remoteChannel.onmessage = receiveMessages;
             };
-        }, (candidate) => { return candidate.candidate.toLowerCase().indexOf("udp") == -1; });
+        }, (candidate) => { return candidate && candidate.candidate.toLowerCase().indexOf("udp") == -1; });
     });
 }, "Basic data channel exchange from offerer to receiver using UDP only");
 
@@ -114,7 +114,7 @@ promise_test((test) => {
                 remoteChannel = event.channel;
                 remoteChannel.onmessage = receiveMessages;
             };
-        }, (candidate) => { return candidate.candidate.toLowerCase().indexOf("tcp") == -1; });
+        }, (candidate) => { return candidate && candidate.candidate.toLowerCase().indexOf("tcp") == -1; });
     });
 }, "Basic data channel exchange from offerer to receiver using TCP only");
 
index b841f11..fce0244 100644 (file)
@@ -42,9 +42,6 @@ function gotDescription2(desc)
 
 function iceCallback1(event, filterOutICECandidate)
 {
-    if (!event.candidate)
-        return;
-
     if (filterOutICECandidate && filterOutICECandidate(event.candidate))
         return;
 
@@ -53,14 +50,10 @@ function iceCallback1(event, filterOutICECandidate)
 
 function iceCallback2(event, filterOutICECandidate)
 {
-    if (!event.candidate)
-        return;
-
     if (filterOutICECandidate && filterOutICECandidate(event.candidate))
         return;
 
-    if (event.candidate)
-        localConnection.addIceCandidate(event.candidate).then(onAddIceCandidateSuccess, onAddIceCandidateError);
+    localConnection.addIceCandidate(event.candidate).then(onAddIceCandidateSuccess, onAddIceCandidateError);
 }
 
 function onAddIceCandidateSuccess()
@@ -69,6 +62,7 @@ function onAddIceCandidateSuccess()
 
 function onAddIceCandidateError(error)
 {
+    console.log("addIceCandidate error: " + error)
     assert_unreached();
 }
 
index a82a877..28a1159 100644 (file)
@@ -1,3 +1,25 @@
+2017-03-27  Youenn Fablet  <youenn@apple.com>
+
+        addIceCandidate should not throw if passed null or undefined
+        https://bugs.webkit.org/show_bug.cgi?id=170118
+
+        Reviewed by Eric Carlson.
+
+        Covered by updated test.
+
+        A null/undefined candidate passed to addIceCandidate means end of Ice candidate..
+
+        * Modules/mediastream/PeerConnectionBackend.cpp:
+        (WebCore::PeerConnectionBackend::addIceCandidate):
+        * Modules/mediastream/PeerConnectionBackend.h:
+        (WebCore::PeerConnectionBackend::endOfIceCandidates):
+        * Modules/mediastream/RTCPeerConnection.cpp:
+        (WebCore::RTCPeerConnection::queuedAddIceCandidate):
+        * Modules/mediastream/RTCPeerConnection.h:
+        * Modules/mediastream/RTCPeerConnection.idl:
+        * Modules/mediastream/RTCPeerConnection.js:
+        (addIceCandidate):
+
 2017-03-27  Antti Koivisto  <antti@apple.com>
 
         Allow the page to render before <link> stylesheet tags in body
index e626336..dd59649 100644 (file)
@@ -225,16 +225,22 @@ void PeerConnectionBackend::setRemoteDescriptionFailed(Exception&& exception)
     m_setDescriptionPromise = std::nullopt;
 }
 
-void PeerConnectionBackend::addIceCandidate(RTCIceCandidate& iceCandidate, DOMPromise<void>&& promise)
+void PeerConnectionBackend::addIceCandidate(RTCIceCandidate* iceCandidate, DOMPromise<void>&& promise)
 {
     ASSERT(m_peerConnection.signalingState() != RTCSignalingState::Closed);
 
-    if (iceCandidate.sdpMid().isNull() && !iceCandidate.sdpMLineIndex()) {
+    if (!iceCandidate) {
+        endOfIceCandidates(WTFMove(promise));
+        return;
+    }
+
+    // FIXME: As per https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-addicecandidate(), this check should be done before enqueuing the task.
+    if (iceCandidate->sdpMid().isNull() && !iceCandidate->sdpMLineIndex()) {
         promise.reject(Exception { TypeError, ASCIILiteral("Trying to add a candidate that is missing both sdpMid and sdpMLineIndex") });
         return;
     }
     m_addIceCandidatePromise = WTFMove(promise);
-    doAddIceCandidate(iceCandidate);
+    doAddIceCandidate(*iceCandidate);
 }
 
 void PeerConnectionBackend::addIceCandidateSucceeded()
index 56c4a08..fedf113 100644 (file)
@@ -62,7 +62,6 @@ using StatsPromise = DOMPromise<IDLInterface<RTCStatsReport>>;
 
 using CreatePeerConnectionBackend = std::unique_ptr<PeerConnectionBackend> (*)(RTCPeerConnection&);
 
-// FIXME: What is the value of this abstract class? There is only one concrete class derived from it.
 class PeerConnectionBackend {
 public:
     WEBCORE_EXPORT static CreatePeerConnectionBackend create;
@@ -74,7 +73,7 @@ public:
     void createAnswer(RTCAnswerOptions&&, PeerConnection::SessionDescriptionPromise&&);
     void setLocalDescription(RTCSessionDescription&, DOMPromise<void>&&);
     void setRemoteDescription(RTCSessionDescription&, DOMPromise<void>&&);
-    void addIceCandidate(RTCIceCandidate&, DOMPromise<void>&&);
+    void addIceCandidate(RTCIceCandidate*, DOMPromise<void>&&);
 
     virtual std::unique_ptr<RTCDataChannelHandler> createDataChannelHandler(const String&, const RTCDataChannelInit&) = 0;
 
@@ -137,6 +136,7 @@ private:
     virtual void doSetLocalDescription(RTCSessionDescription&) = 0;
     virtual void doSetRemoteDescription(RTCSessionDescription&) = 0;
     virtual void doAddIceCandidate(RTCIceCandidate&) = 0;
+    virtual void endOfIceCandidates(DOMPromise<void>&& promise) { promise.resolve(); }
     virtual void doStop() = 0;
 
 protected:
index 556e4d9..849b7a2 100644 (file)
@@ -278,7 +278,7 @@ RefPtr<RTCSessionDescription> RTCPeerConnection::pendingRemoteDescription() cons
     return m_backend->pendingRemoteDescription();
 }
 
-void RTCPeerConnection::queuedAddIceCandidate(RTCIceCandidate& rtcCandidate, DOMPromise<void>&& promise)
+void RTCPeerConnection::queuedAddIceCandidate(RTCIceCandidate* rtcCandidate, DOMPromise<void>&& promise)
 {
     if (m_signalingState == RTCSignalingState::Closed) {
         promise.reject(INVALID_STATE_ERR);
index f0e7271..7ce871c 100644 (file)
@@ -86,7 +86,7 @@ public:
     RefPtr<RTCSessionDescription> currentRemoteDescription() const;
     RefPtr<RTCSessionDescription> pendingRemoteDescription() const;
 
-    void queuedAddIceCandidate(RTCIceCandidate&, DOMPromise<void>&&);
+    void queuedAddIceCandidate(RTCIceCandidate*, DOMPromise<void>&&);
 
     RTCSignalingState signalingState() const { return m_signalingState; }
     RTCIceGatheringState iceGatheringState() const { return m_iceGatheringState; }
index 7851792..a83bbe6 100644 (file)
@@ -110,7 +110,7 @@ typedef RTCRtpTransceiverDirection RtpTransceiverDirection;
     [PrivateIdentifier] Promise<RTCSessionDescriptionInit> queuedCreateAnswer(optional RTCAnswerOptions answerOptions);
     [PrivateIdentifier] Promise<void> queuedSetLocalDescription(RTCSessionDescription description);
     [PrivateIdentifier] Promise<void> queuedSetRemoteDescription(RTCSessionDescription description);
-    [PrivateIdentifier] Promise<void> queuedAddIceCandidate(RTCIceCandidate candidate);
+    [PrivateIdentifier] Promise<void> queuedAddIceCandidate(RTCIceCandidate? candidate);
 
 
     // 4.11 Certificate management
index c2cc603..3c8c183 100644 (file)
@@ -250,7 +250,8 @@ function addIceCandidate()
         "constructor": @RTCIceCandidate,
         "argName": "candidate",
         "argType": "RTCIceCandidate",
-        "maybeDictionary": "true"
+        "maybeDictionary": "true",
+        "defaultsToNull" : "true"
     };
     return @objectAndCallbacksOverload(arguments, "addIceCandidate", objectInfo, function (candidate) {
         // Promise mode