WebRTC: Imlement MediaEndpointPeerConnection::addIceCandidate()
authoradam.bergkvist@ericsson.com <adam.bergkvist@ericsson.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 16:41:18 +0000 (16:41 +0000)
committeradam.bergkvist@ericsson.com <adam.bergkvist@ericsson.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 16:41:18 +0000 (16:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158690

Reviewed by Eric Carlson.

Source/WebCore:

Implement MediaEndpointPeerConnection::addIceCandidate() that is the MediaEndpoint
implementation of RTCPeerConnection.addIceCandidate() [1].

[1] https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#dom-peerconnection-addicecandidate

Test: fast/mediastream/RTCPeerConnection-addIceCandidate.html

* Modules/mediastream/MediaEndpointPeerConnection.cpp:
(WebCore::MediaEndpointPeerConnection::addIceCandidate):
(WebCore::MediaEndpointPeerConnection::addIceCandidateTask):
Implemented.
* Modules/mediastream/MediaEndpointPeerConnection.h:
* platform/mediastream/MediaEndpoint.h:
Use mid instead of mdescIndex to identify the target media description in the backend.
* platform/mock/MockMediaEndpoint.cpp:
Update mock method signature accordingly.
(WebCore::MockMediaEndpoint::addRemoteCandidate):
* platform/mock/MockMediaEndpoint.h:

LayoutTests:

Add test for RTCPeerConnection.addIceCandidate() that verifies:
- Candidate line parsing
- That a underlying media description can be identified using either sdpMid or sdpMLineIndex
- That sdpMid takes precedence over sdpMLineIndex

* fast/mediastream/RTCPeerConnection-addIceCandidate-expected.txt: Added.
* fast/mediastream/RTCPeerConnection-addIceCandidate.html: Added.
* platform/mac/TestExpectations:
The mac port is not building with WEB_RTC yet.

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

LayoutTests/ChangeLog
LayoutTests/fast/mediastream/RTCPeerConnection-addIceCandidate-expected.txt [new file with mode: 0644]
LayoutTests/fast/mediastream/RTCPeerConnection-addIceCandidate.html [new file with mode: 0644]
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp
Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.h
Source/WebCore/platform/mediastream/MediaEndpoint.h
Source/WebCore/platform/mock/MockMediaEndpoint.cpp
Source/WebCore/platform/mock/MockMediaEndpoint.h

index b8d33ce..b8e9bb0 100644 (file)
@@ -1,5 +1,22 @@
 2016-06-14  Adam Bergkvist  <adam.bergkvist@ericsson.com>
 
+        WebRTC: Imlement MediaEndpointPeerConnection::addIceCandidate()
+        https://bugs.webkit.org/show_bug.cgi?id=158690
+
+        Reviewed by Eric Carlson.
+
+        Add test for RTCPeerConnection.addIceCandidate() that verifies:
+        - Candidate line parsing
+        - That a underlying media description can be identified using either sdpMid or sdpMLineIndex
+        - That sdpMid takes precedence over sdpMLineIndex
+
+        * fast/mediastream/RTCPeerConnection-addIceCandidate-expected.txt: Added.
+        * fast/mediastream/RTCPeerConnection-addIceCandidate.html: Added.
+        * platform/mac/TestExpectations:
+        The mac port is not building with WEB_RTC yet.
+
+2016-06-14  Adam Bergkvist  <adam.bergkvist@ericsson.com>
+
         WebRTC: Add media setup test where media is set up in one direction at a time
         https://bugs.webkit.org/show_bug.cgi?id=158691
 
diff --git a/LayoutTests/fast/mediastream/RTCPeerConnection-addIceCandidate-expected.txt b/LayoutTests/fast/mediastream/RTCPeerConnection-addIceCandidate-expected.txt
new file mode 100644 (file)
index 0000000..ecc3cb7
--- /dev/null
@@ -0,0 +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 Error: InvalidStateError: DOM Exception 11
+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 Error: OperationError: DOM Exception 34
+PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMLineIndex: badSdpMLineIndex})) rejected with Error: OperationError: DOM Exception 34
+*** A (bad) sdpMid takes precedesce over valid sdpMLineIndex
+PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMid: badSdpMid, sdpMLineIndex: sdpMLineIndex})) rejected with Error: OperationError: DOM Exception 34
+*** Test bad candidate content with valid sdpMid
+PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: 'bad content', sdpMid: sdpMid})) rejected with Error: OperationError: DOM Exception 34
+*** Test bad candidate content with valid sdpMLineIndex
+PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: 'bad content', sdpMLineIndex: sdpMLineIndex})) rejected with Error: OperationError: DOM Exception 34
+
+*** Test some OK input
+PASS promise pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMid: sdpMid})) fulfilled with undefined
+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
+
diff --git a/LayoutTests/fast/mediastream/RTCPeerConnection-addIceCandidate.html b/LayoutTests/fast/mediastream/RTCPeerConnection-addIceCandidate.html
new file mode 100644 (file)
index 0000000..6bd0540
--- /dev/null
@@ -0,0 +1,101 @@
+<!DOCTYPE html>
+
+<html>
+    <head>
+        <script src="../../resources/js-test-pre.js"></script>
+        <script src="resources/promise-utils.js"></script>
+    </head>
+    <body>
+        <script>
+            let stream;
+            let track;
+
+            let sdpMLineIndex = 0;
+            let badSdpMLineIndex;
+            let sdpMid = null;
+            let badSdpMid;
+            const validCandidate = "candidate:1 1 UDP 2013266431 172.17.0.1 44474 typ host";
+
+            description("Test behavior of RTCPeerConnection.addIceCandidate");
+
+            if (window.testRunner)
+                testRunner.setUserMediaPermission(true);
+            else {
+                debug("This test can not be run without the testRunner");
+                finishJSTest();
+            }
+
+            const pc = new webkitRTCPeerConnection({iceServers:[{urls:'stun:foo.com'}]});
+            const remotePc = new webkitRTCPeerConnection({iceServers:[{urls:'stun:foo.com'}]});
+
+            debug("<br>*** A remote description is needed before a candidate can be added");
+            shouldBe("pc.remoteDescription", "null");
+            promiseShouldReject("pc.addIceCandidate(new RTCIceCandidate({candidate: 'foo', sdpMid: 0}))").then(function () {
+                return navigator.mediaDevices.getUserMedia({ "audio": true, "video": true });
+            })
+            .then(function (s) {
+                stream = s;
+                track = stream.getTracks()[0];
+
+                remotePc.addTrack(track, stream);
+                return remotePc.createOffer();
+            })
+            .then(function (remoteOffer) {
+                return pc.setRemoteDescription(remoteOffer);
+            })
+            .then(function () {
+                testPassed("Remote description set");
+
+                debug("<br>*** Define sdpMid, badSdpMid, sdpMLineIndex and badSdpMLineIndex for testing");
+                badSdpMLineIndex = sdpMLineIndex + pc.getTransceivers().length + 10;
+                shouldNotBe("sdpMLineIndex", "badSdpMLineIndex");
+
+                sdpMid = pc.getTransceivers()[0].mid;
+                shouldNotBe("sdpMid", "null");
+                badSdpMid = sdpMid + "_foo";
+                shouldNotBe("sdpMid", "badSdpMid");
+
+                return promiseShouldReject("pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMid: badSdpMid}))");
+            })
+            .then(function () {
+                return promiseShouldReject("pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMLineIndex: badSdpMLineIndex}))");
+            })
+            .then(function () {
+                debug("*** A (bad) sdpMid takes precedesce over valid sdpMLineIndex");
+                return promiseShouldReject("pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMid: badSdpMid, sdpMLineIndex: sdpMLineIndex}))");
+            })
+            .then(function () {
+                debug("*** Test bad candidate content with valid sdpMid");
+                return promiseShouldReject("pc.addIceCandidate(new RTCIceCandidate({candidate: 'bad content', sdpMid: sdpMid}))");
+            })
+            .then(function () {
+                debug("*** Test bad candidate content with valid sdpMLineIndex");
+                return promiseShouldReject("pc.addIceCandidate(new RTCIceCandidate({candidate: 'bad content', sdpMLineIndex: sdpMLineIndex}))");
+            })
+            .then(function () {
+                debug("<br>*** Test some OK input");
+                return promiseShouldResolve("pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMid: sdpMid}))");
+            })
+            .then(function () {
+                return promiseShouldResolve("pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMLineIndex: sdpMLineIndex}))");
+            })
+            .then(function () {
+                debug("*** A valid sdpMid takes precedesce over a bad sdpMLineIndex");
+                return promiseShouldResolve("pc.addIceCandidate(new RTCIceCandidate({candidate: validCandidate, sdpMid: sdpMid, sdpMLineIndex: badSdpMLineIndex}))");
+            })
+            .then(function () {
+                testPassed("End of test promise chain");
+                finishJSTest();
+            })
+            .catch(function (error) {
+                testFailed("Error in promise chain: " + error);
+                finishJSTest();
+            });
+
+            window.jsTestIsAsync = true;
+            window.successfullyParsed = true;
+
+        </script>
+        <script src="../../resources/js-test-post.js"></script>
+    </body>
+</html>
index bff1f54..7d2e37c 100644 (file)
@@ -175,6 +175,7 @@ fast/mediastream/RTCPeerConnection-have-local-pranswer.html
 fast/mediastream/RTCPeerConnection-have-remote-offer.html
 fast/mediastream/RTCPeerConnection-have-remote-pranswer.html
 fast/mediastream/RTCPeerConnection-ice.html
+fast/mediastream/RTCPeerConnection-addIceCandidate.html
 fast/mediastream/RTCPeerConnection-localDescription.html
 fast/mediastream/RTCPeerConnection-onnegotiationneeded.html
 fast/mediastream/RTCPeerConnection-remoteDescription.html
index cd3d68d..4efcd86 100644 (file)
@@ -1,3 +1,29 @@
+2016-06-14  Adam Bergkvist  <adam.bergkvist@ericsson.com>
+
+        WebRTC: Imlement MediaEndpointPeerConnection::addIceCandidate()
+        https://bugs.webkit.org/show_bug.cgi?id=158690
+
+        Reviewed by Eric Carlson.
+
+        Implement MediaEndpointPeerConnection::addIceCandidate() that is the MediaEndpoint
+        implementation of RTCPeerConnection.addIceCandidate() [1].
+
+        [1] https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#dom-peerconnection-addicecandidate
+
+        Test: fast/mediastream/RTCPeerConnection-addIceCandidate.html
+
+        * Modules/mediastream/MediaEndpointPeerConnection.cpp:
+        (WebCore::MediaEndpointPeerConnection::addIceCandidate):
+        (WebCore::MediaEndpointPeerConnection::addIceCandidateTask):
+        Implemented.
+        * Modules/mediastream/MediaEndpointPeerConnection.h:
+        * platform/mediastream/MediaEndpoint.h:
+        Use mid instead of mdescIndex to identify the target media description in the backend.
+        * platform/mock/MockMediaEndpoint.cpp:
+        Update mock method signature accordingly.
+        (WebCore::MockMediaEndpoint::addRemoteCandidate):
+        * platform/mock/MockMediaEndpoint.h:
+
 2016-06-14  Zalan Bujtas  <zalan@apple.com>
 
         Make RenderBlock::insertInto/RemoveFromTrackedRendererMaps functions static.
index b8ca48d..9ee0f12 100644 (file)
@@ -38,6 +38,7 @@
 #include "MediaStream.h"
 #include "MediaStreamTrack.h"
 #include "PeerMediaDescription.h"
+#include "RTCIceCandidate.h"
 #include "RTCOfferAnswerOptions.h"
 #include "RTCRtpTransceiver.h"
 #include "RTCTrackEvent.h"
@@ -599,11 +600,67 @@ void MediaEndpointPeerConnection::setConfiguration(RTCConfiguration& configurati
 
 void MediaEndpointPeerConnection::addIceCandidate(RTCIceCandidate& rtcCandidate, PeerConnection::VoidPromise&& promise)
 {
-    UNUSED_PARAM(rtcCandidate);
+    runTask([this, protectedCandidate = RefPtr<RTCIceCandidate>(&rtcCandidate), protectedPromise = WTFMove(promise)]() mutable {
+        addIceCandidateTask(*protectedCandidate, protectedPromise);
+    });
+}
 
-    notImplemented();
+void MediaEndpointPeerConnection::addIceCandidateTask(RTCIceCandidate& rtcCandidate, PeerConnection::VoidPromise& promise)
+{
+    if (m_client->internalSignalingState() == SignalingState::Closed)
+        return;
 
-    promise.reject(NOT_SUPPORTED_ERR);
+    if (!internalRemoteDescription()) {
+        promise.reject(INVALID_STATE_ERR, "No remote description set");
+        return;
+    }
+
+    const MediaDescriptionVector& remoteMediaDescriptions = internalRemoteDescription()->configuration()->mediaDescriptions();
+    PeerMediaDescription* targetMediaDescription = nullptr;
+
+    // When identifying the target media description, sdpMid takes precedence over sdpMLineIndex
+    // if both are present.
+    if (!rtcCandidate.sdpMid().isNull()) {
+        const String& mid = rtcCandidate.sdpMid();
+        for (auto& description : remoteMediaDescriptions) {
+            if (description->mid() == mid) {
+                targetMediaDescription = description.get();
+                break;
+            }
+        }
+
+        if (!targetMediaDescription) {
+            promise.reject(OperationError, "sdpMid did not match any media description");
+            return;
+        }
+    } else if (rtcCandidate.sdpMLineIndex()) {
+        unsigned short sdpMLineIndex = rtcCandidate.sdpMLineIndex().value();
+        if (sdpMLineIndex >= remoteMediaDescriptions.size()) {
+            promise.reject(OperationError, "sdpMLineIndex is out of range");
+            return;
+        }
+        targetMediaDescription = remoteMediaDescriptions[sdpMLineIndex].get();
+    } else {
+        ASSERT_NOT_REACHED();
+        return;
+    }
+
+    RefPtr<IceCandidate> candidate;
+    SDPProcessor::Result result = m_sdpProcessor->parseCandidateLine(rtcCandidate.candidate(), candidate);
+    if (result != SDPProcessor::Result::Success) {
+        if (result == SDPProcessor::Result::ParseError)
+            promise.reject(OperationError, "Invalid candidate content");
+        else
+            LOG_ERROR("SDPProcessor internal error");
+        return;
+    }
+
+    targetMediaDescription->addIceCandidate(candidate.copyRef());
+
+    m_mediaEndpoint->addRemoteCandidate(*candidate, targetMediaDescription->mid(), targetMediaDescription->iceUfrag(),
+        targetMediaDescription->icePassword());
+
+    promise.resolve(nullptr);
 }
 
 void MediaEndpointPeerConnection::getStats(MediaStreamTrack*, PeerConnection::StatsPromise&& promise)
index df1e9d2..cf76b5f 100644 (file)
@@ -92,6 +92,8 @@ private:
     void setLocalDescriptionTask(RefPtr<RTCSessionDescription>&&, PeerConnection::VoidPromise&);
     void setRemoteDescriptionTask(RefPtr<RTCSessionDescription>&&, PeerConnection::VoidPromise&);
 
+    void addIceCandidateTask(RTCIceCandidate&, PeerConnection::VoidPromise&);
+
     void replaceTrackTask(RTCRtpSender&, const String& mid, RefPtr<MediaStreamTrack>&&, PeerConnection::VoidPromise&);
 
     bool localDescriptionTypeValidForState(RTCSessionDescription::SdpType) const;
index 3dbf00a..6d2d61b 100644 (file)
@@ -80,7 +80,7 @@ public:
     virtual UpdateResult updateReceiveConfiguration(MediaEndpointSessionConfiguration*, bool isInitiator) = 0;
     virtual UpdateResult updateSendConfiguration(MediaEndpointSessionConfiguration*, const RealtimeMediaSourceMap&, bool isInitiator) = 0;
 
-    virtual void addRemoteCandidate(IceCandidate&, unsigned mdescIndex, const String& ufrag, const String& password) = 0;
+    virtual void addRemoteCandidate(IceCandidate&, const String& mid, const String& ufrag, const String& password) = 0;
 
     virtual Ref<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) = 0;
     virtual void replaceSendSource(RealtimeMediaSource&, const String& mid) = 0;
index 89e84ba..177654c 100644 (file)
@@ -173,10 +173,10 @@ MediaEndpoint::UpdateResult MockMediaEndpoint::updateSendConfiguration(MediaEndp
     return UpdateResult::Success;
 }
 
-void MockMediaEndpoint::addRemoteCandidate(IceCandidate& candidate, unsigned mdescIndex, const String& ufrag, const String& password)
+void MockMediaEndpoint::addRemoteCandidate(IceCandidate& candidate, const String& mid, const String& ufrag, const String& password)
 {
     UNUSED_PARAM(candidate);
-    UNUSED_PARAM(mdescIndex);
+    UNUSED_PARAM(mid);
     UNUSED_PARAM(ufrag);
     UNUSED_PARAM(password);
 }
index a1b8e9e..a15db38 100644 (file)
@@ -54,7 +54,7 @@ public:
     UpdateResult updateReceiveConfiguration(MediaEndpointSessionConfiguration*, bool isInitiator) override;
     UpdateResult updateSendConfiguration(MediaEndpointSessionConfiguration*, const RealtimeMediaSourceMap&, bool isInitiator) override;
 
-    void addRemoteCandidate(IceCandidate&, unsigned mdescIndex, const String& ufrag, const String& password) override;
+    void addRemoteCandidate(IceCandidate&, const String& mid, const String& ufrag, const String& password) override;
 
     Ref<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) override;
     void replaceSendSource(RealtimeMediaSource&, const String& mid) override;