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

Reviewed by Eric Carlson.

Source/WebCore:

Implement MediaEndpointPeerConnection::replaceTrack() that is the MediaEndpoint implementation
of RTCRtpSender.replaceTrack() [1].

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

Updated fast/mediastream/RTCRtpSender-replaceTrack.html

* Modules/mediastream/MediaEndpointPeerConnection.cpp:
(WebCore::MediaEndpointPeerConnection::replaceTrack):
(WebCore::MediaEndpointPeerConnection::replaceTrackTask):
Implemented.
* Modules/mediastream/MediaEndpointPeerConnection.h:
* Modules/mediastream/PeerConnectionBackend.h:
* Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::replaceTrack):
* Modules/mediastream/RTCPeerConnection.h:
Move the MediaStreamTrack instance of sending a reference to it. This change is the main
reason many files are touched by this change.
* Modules/mediastream/RTCRtpSender.h:
* Modules/mediastream/RTCRtpSender.idl:
* platform/mediastream/MediaEndpoint.h:
Use mid instead of mdescIndex to identify the media description in the backend.
* platform/mock/MockMediaEndpoint.cpp:
(WebCore::MockMediaEndpoint::replaceSendSource):
* platform/mock/MockMediaEndpoint.h:

LayoutTests:

Update existing test for RTCRtpSender.replaceTrack.

* fast/mediastream/RTCRtpSender-replaceTrack-expected.txt:
* fast/mediastream/RTCRtpSender-replaceTrack.html:
Add test cases where a "not yet negotiated track" is directly replaced. Also check that the
old track id is used in later offers.
* fast/mediastream/resources/promise-utils.js:
(promiseShouldResolve):
Added utility method to test promise expressions that are expected to resolve.

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/mediastream/RTCRtpSender-replaceTrack-expected.txt
LayoutTests/fast/mediastream/RTCRtpSender-replaceTrack.html
LayoutTests/fast/mediastream/resources/promise-utils.js
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp
Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.h
Source/WebCore/Modules/mediastream/PeerConnectionBackend.h
Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
Source/WebCore/Modules/mediastream/RTCPeerConnection.h
Source/WebCore/Modules/mediastream/RTCRtpSender.h
Source/WebCore/Modules/mediastream/RTCRtpSender.idl
Source/WebCore/platform/mediastream/MediaEndpoint.h
Source/WebCore/platform/mock/MockMediaEndpoint.cpp
Source/WebCore/platform/mock/MockMediaEndpoint.h

index fb93d2f..e00b617 100644 (file)
@@ -1,3 +1,20 @@
+2016-06-13  Adam Bergkvist  <adam.bergkvist@ericsson.com>
+
+        WebRTC: Imlement MediaEndpointPeerConnection::replaceTrack()
+        https://bugs.webkit.org/show_bug.cgi?id=158688
+
+        Reviewed by Eric Carlson.
+
+        Update existing test for RTCRtpSender.replaceTrack.
+
+        * fast/mediastream/RTCRtpSender-replaceTrack-expected.txt:
+        * fast/mediastream/RTCRtpSender-replaceTrack.html:
+        Add test cases where a "not yet negotiated track" is directly replaced. Also check that the
+        old track id is used in later offers.
+        * fast/mediastream/resources/promise-utils.js:
+        (promiseShouldResolve):
+        Added utility method to test promise expressions that are expected to resolve.
+
 2016-06-13  Joseph Pecoraro  <pecoraro@apple.com>
 
         window.onerror should pass the ErrorEvent's 'error' property as the 5th argument to the event handler
index bcc63e6..ed7c3d2 100644 (file)
@@ -12,8 +12,13 @@ PASS promise sender.replaceTrack(null) rejected with TypeError: Argument 1 ('wit
 PASS promise sender.replaceTrack({}) rejected with TypeError: Argument 1 ('withTrack') to RTCRtpSender.replaceTrack must be an instance of MediaStreamTrack
 Test mismatching track kind
 PASS promise sender.replaceTrack(videoTrack) rejected with TypeError: Type error
-This is a valid call but will be rejected until the PeerConnectionBackend implementation has proper support
-PASS promise sender.replaceTrack(audioTrack2) rejected with Error: NotSupportedError: DOM Exception 9
+PASS promise sender.replaceTrack(audioTrack2) fulfilled with undefined
+PASS Track successfully replaced
+PASS sender.track is audioTrack2
+Sender should still use old track (audioTrack) id in negotiation.
+PASS offer created
+PASS offer.sdp.indexOf(audioTrack.id) is not -1
+PASS offer.sdp.indexOf(audioTrack2.id) is -1
 Stop sender, and try replacing the track
 PASS promise sender.replaceTrack(audioTrack2) rejected with Error: InvalidStateError: DOM Exception 11
 Create a new sender
index b0b24db..77d19e9 100644 (file)
@@ -2,6 +2,7 @@
 <html>
     <head>
         <script src="../../resources/js-test-pre.js"></script>
+        <script src="resources/promise-utils.js"></script>
     </head>
     <body>
         <script>
@@ -10,6 +11,7 @@
             var audioTrack2;
             var videoTrack;
             var sender;
+            var offer;
 
             description("Test basic behavior of RTCRtpSender.replaceTrack()");
 
                 shouldBe("sender.track", "audioTrack");
 
                 promiseShouldReject("sender.replaceTrack()")
-                .catch(function () {
+                .then(function () {
                     return promiseShouldReject("sender.replaceTrack(null)");
                 })
-                .catch(function () {
+                .then(function () {
                     return promiseShouldReject("sender.replaceTrack({})");
                 })
-                .catch(function () {
+                .then(function () {
                     debug("Test mismatching track kind");
                     return promiseShouldReject("sender.replaceTrack(videoTrack)");
                 })
-                .catch(function () {
-                    debug("This is a valid call but will be rejected until the PeerConnectionBackend implementation has proper support");
-                    return promiseShouldReject("sender.replaceTrack(audioTrack2)");
+                .then(function () {
+                    return promiseShouldResolve("sender.replaceTrack(audioTrack2)");
+                })
+                .then(function () {
+                    testPassed("Track successfully replaced");
+                    shouldBe("sender.track", "audioTrack2");
+
+                    debug("Sender should still use old track (audioTrack) id in negotiation.");
+                    return pc.createOffer();
                 })
-                .catch(function () {
+                .then(function (o) {
+                    offer = o;
+                    testPassed("offer created");
+                    shouldNotBe("offer.sdp.indexOf(audioTrack.id)", "-1");
+                    shouldBe("offer.sdp.indexOf(audioTrack2.id)", "-1");
+
                     debug("Stop sender, and try replacing the track");
                     pc.removeTrack(sender);
                     return promiseShouldReject("sender.replaceTrack(audioTrack2)");
                 })
-                .catch(function () {
+                .then(function () {
                     debug("Create a new sender");
                     shouldNotThrow("sender = pc.addTrack(audioTrack2, stream)");
                     debug("Close pc and try replacing the track");
                     pc.close();
                     return promiseShouldReject("sender.replaceTrack(audioTrack3)");
                 })
-                .catch(function () {
+                .then(function () {
                     debug("End of promise chain");
                     finishJSTest();
+                })
+                .catch(function (error) {
+                    testFailed("Error in promise chain: " + error);
+                    finishJSTest();
                 });
             })
             .catch(function (error) {
                 finishJSTest();
             });
 
-            function promiseShouldReject(expr) {
-                var p;
-                try {
-                    p = eval(expr);
-                } catch (e) {
-                    testFailed("evaluating " + expr + " threw exception " + e);
-                    return Promise.reject();
-                }
-
-                if (!(p instanceof Promise)) {
-                    testFailed(expr + " does not evaluate to a promise.");
-                    return Promise.reject();
-                }
-
-                p.then(function () {
-                    testFailed("promise " + expr + " fulfilled unexpectedly.");
-                });
-
-                p.catch(function (reason) {
-                    testPassed("promise " + expr + " rejected with " + reason);
-                });
-
-                return p;
-            }
-
             window.jsTestIsAsync = true;
             window.successfullyParsed = true;
 
index 20e078d..fd0c396 100644 (file)
@@ -15,6 +15,25 @@ function ensurePromise(expr) {
     return p;
 }
 
+function promiseShouldResolve(expr) {
+    return new Promise(function (done) {
+        var p = ensurePromise(expr);
+        if (!p) {
+            done();
+            return;
+        }
+
+        p.then(function (value) {
+            testPassed("promise " + expr + " fulfilled with " + value);
+            done();
+        })
+        .catch(function (reason) {
+            testFailed("promise " + expr + " rejected unexpectedly.");
+            done();
+        });
+    });
+}
+
 function promiseShouldReject(expr, reasonArg) {
     return new Promise(function (done) {
         var p = ensurePromise(expr);
index d645c85..df32ca9 100644 (file)
@@ -1,3 +1,36 @@
+2016-06-13  Adam Bergkvist  <adam.bergkvist@ericsson.com>
+
+        WebRTC: Imlement MediaEndpointPeerConnection::replaceTrack()
+        https://bugs.webkit.org/show_bug.cgi?id=158688
+
+        Reviewed by Eric Carlson.
+
+        Implement MediaEndpointPeerConnection::replaceTrack() that is the MediaEndpoint implementation
+        of RTCRtpSender.replaceTrack() [1].
+
+        [1] https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#dom-rtcrtpsender-replacetrack
+
+        Updated fast/mediastream/RTCRtpSender-replaceTrack.html
+
+        * Modules/mediastream/MediaEndpointPeerConnection.cpp:
+        (WebCore::MediaEndpointPeerConnection::replaceTrack):
+        (WebCore::MediaEndpointPeerConnection::replaceTrackTask):
+        Implemented.
+        * Modules/mediastream/MediaEndpointPeerConnection.h:
+        * Modules/mediastream/PeerConnectionBackend.h:
+        * Modules/mediastream/RTCPeerConnection.cpp:
+        (WebCore::RTCPeerConnection::replaceTrack):
+        * Modules/mediastream/RTCPeerConnection.h:
+        Move the MediaStreamTrack instance of sending a reference to it. This change is the main
+        reason many files are touched by this change.
+        * Modules/mediastream/RTCRtpSender.h:
+        * Modules/mediastream/RTCRtpSender.idl:
+        * platform/mediastream/MediaEndpoint.h:
+        Use mid instead of mdescIndex to identify the media description in the backend.
+        * platform/mock/MockMediaEndpoint.cpp:
+        (WebCore::MockMediaEndpoint::replaceSendSource):
+        * platform/mock/MockMediaEndpoint.h:
+
 2016-06-13  Joseph Pecoraro  <pecoraro@apple.com>
 
         window.onerror should pass the ErrorEvent's 'error' property as the 5th argument to the event handler
index 00431b6..b8ca48d 100644 (file)
@@ -625,15 +625,35 @@ RefPtr<RTCRtpReceiver> MediaEndpointPeerConnection::createReceiver(const String&
     return RTCRtpReceiver::create(WTFMove(remoteTrack));
 }
 
-void MediaEndpointPeerConnection::replaceTrack(RTCRtpSender& sender, MediaStreamTrack& withTrack, PeerConnection::VoidPromise&& promise)
+void MediaEndpointPeerConnection::replaceTrack(RTCRtpSender& sender, RefPtr<MediaStreamTrack>&& withTrack, PeerConnection::VoidPromise&& promise)
 {
-    UNUSED_PARAM(sender);
-    UNUSED_PARAM(withTrack);
-    UNUSED_PARAM(promise);
+    RTCRtpTransceiver* transceiver = matchTransceiver(m_client->getTransceivers(), [&sender] (RTCRtpTransceiver& current) {
+        return current.sender() == &sender;
+    });
+    ASSERT(transceiver);
 
-    notImplemented();
+    const String& mid = transceiver->mid();
+    if (mid.isNull()) {
+        // Transceiver is not associated with a media description yet.
+        sender.setTrack(WTFMove(withTrack));
+        promise.resolve(nullptr);
+        return;
+    }
 
-    promise.reject(NOT_SUPPORTED_ERR);
+    runTask([this, protectedSender = RefPtr<RTCRtpSender>(&sender), mid, protectedTrack = WTFMove(withTrack), protectedPromise = WTFMove(promise)]() mutable {
+        replaceTrackTask(*protectedSender, mid, WTFMove(protectedTrack), protectedPromise);
+    });
+}
+
+void MediaEndpointPeerConnection::replaceTrackTask(RTCRtpSender& sender, const String& mid, RefPtr<MediaStreamTrack>&& withTrack, PeerConnection::VoidPromise& promise)
+{
+    if (m_client->internalSignalingState() == SignalingState::Closed)
+        return;
+
+    m_mediaEndpoint->replaceSendSource(withTrack->source(), mid);
+
+    sender.setTrack(WTFMove(withTrack));
+    promise.resolve(nullptr);
 }
 
 void MediaEndpointPeerConnection::stop()
index f017b2e..df1e9d2 100644 (file)
@@ -74,7 +74,7 @@ public:
     void getStats(MediaStreamTrack*, PeerConnection::StatsPromise&&) override;
 
     RefPtr<RTCRtpReceiver> createReceiver(const String& transceiverMid, const String& trackKind, const String& trackId) override;
-    void replaceTrack(RTCRtpSender&, MediaStreamTrack&, PeerConnection::VoidPromise&&) override;
+    void replaceTrack(RTCRtpSender&, RefPtr<MediaStreamTrack>&&, PeerConnection::VoidPromise&&) override;
 
     void stop() override;
 
@@ -92,6 +92,8 @@ private:
     void setLocalDescriptionTask(RefPtr<RTCSessionDescription>&&, PeerConnection::VoidPromise&);
     void setRemoteDescriptionTask(RefPtr<RTCSessionDescription>&&, PeerConnection::VoidPromise&);
 
+    void replaceTrackTask(RTCRtpSender&, const String& mid, RefPtr<MediaStreamTrack>&&, PeerConnection::VoidPromise&);
+
     bool localDescriptionTypeValidForState(RTCSessionDescription::SdpType) const;
     bool remoteDescriptionTypeValidForState(RTCSessionDescription::SdpType) const;
 
index 4053b36..7552b2c 100644 (file)
@@ -107,7 +107,7 @@ public:
     virtual void getStats(MediaStreamTrack*, PeerConnection::StatsPromise&&) = 0;
 
     virtual RefPtr<RTCRtpReceiver> createReceiver(const String& transceiverMid, const String& trackKind, const String& trackId) = 0;
-    virtual void replaceTrack(RTCRtpSender&, MediaStreamTrack&, PeerConnection::VoidPromise&&) = 0;
+    virtual void replaceTrack(RTCRtpSender&, RefPtr<MediaStreamTrack>&&, PeerConnection::VoidPromise&&) = 0;
 
     virtual void stop() = 0;
 
index f33bdb1..0c18681 100644 (file)
@@ -495,9 +495,9 @@ void RTCPeerConnection::fireEvent(Event& event)
     dispatchEvent(event);
 }
 
-void RTCPeerConnection::replaceTrack(RTCRtpSender& sender, MediaStreamTrack& withTrack, PeerConnection::VoidPromise&& promise)
+void RTCPeerConnection::replaceTrack(RTCRtpSender& sender, RefPtr<MediaStreamTrack>&& withTrack, PeerConnection::VoidPromise&& promise)
 {
-    m_backend->replaceTrack(sender, withTrack, WTFMove(promise));
+    m_backend->replaceTrack(sender, WTFMove(withTrack), WTFMove(promise));
 }
 
 } // namespace WebCore
index 09b2a45..ef1587d 100644 (file)
@@ -145,7 +145,7 @@ private:
     PeerConnectionStates::IceConnectionState internalIceConnectionState() const override { return m_iceConnectionState; }
 
     // RTCRtpSenderClient
-    void replaceTrack(RTCRtpSender&, MediaStreamTrack&, PeerConnection::VoidPromise&&) override;
+    void replaceTrack(RTCRtpSender&, RefPtr<MediaStreamTrack>&&, PeerConnection::VoidPromise&&) override;
 
     PeerConnectionStates::SignalingState m_signalingState;
     PeerConnectionStates::IceGatheringState m_iceGatheringState;
index 912d0b1..f35501c 100644 (file)
@@ -41,7 +41,7 @@ namespace WebCore {
 
 class RTCRtpSenderClient {
 public:
-    virtual void replaceTrack(RTCRtpSender&, MediaStreamTrack&, PeerConnection::VoidPromise&&) = 0;
+    virtual void replaceTrack(RTCRtpSender&, RefPtr<MediaStreamTrack>&&, PeerConnection::VoidPromise&&) = 0;
 
     virtual ~RTCRtpSenderClient() { }
 };
index de41275..434d7a6 100644 (file)
@@ -31,7 +31,7 @@
 [
     Conditional=WEB_RTC,
 ] interface RTCRtpSender {
-    readonly attribute MediaStreamTrack track;
+    readonly attribute MediaStreamTrack? track;
 
     [StrictTypeChecking, RaisesException] Promise replaceTrack(MediaStreamTrack withTrack);
 };
index ad96e93..3dbf00a 100644 (file)
@@ -83,7 +83,7 @@ public:
     virtual void addRemoteCandidate(IceCandidate&, unsigned mdescIndex, const String& ufrag, const String& password) = 0;
 
     virtual Ref<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) = 0;
-    virtual void replaceSendSource(RealtimeMediaSource&, unsigned mdescIndex) = 0;
+    virtual void replaceSendSource(RealtimeMediaSource&, const String& mid) = 0;
 
     virtual void stop() = 0;
 };
index 7949f7e..89e84ba 100644 (file)
@@ -190,10 +190,10 @@ Ref<RealtimeMediaSource> MockMediaEndpoint::createMutedRemoteSource(const String
     return MockRealtimeVideoSource::createMuted("remote video");
 }
 
-void MockMediaEndpoint::replaceSendSource(RealtimeMediaSource& newSource, unsigned mdescIndex)
+void MockMediaEndpoint::replaceSendSource(RealtimeMediaSource& newSource, const String& mid)
 {
     UNUSED_PARAM(newSource);
-    UNUSED_PARAM(mdescIndex);
+    UNUSED_PARAM(mid);
 }
 
 void MockMediaEndpoint::stop()
index 6d14ac0..a1b8e9e 100644 (file)
@@ -57,7 +57,7 @@ public:
     void addRemoteCandidate(IceCandidate&, unsigned mdescIndex, const String& ufrag, const String& password) override;
 
     Ref<RealtimeMediaSource> createMutedRemoteSource(const String& mid, RealtimeMediaSource::Type) override;
-    void replaceSendSource(RealtimeMediaSource&, unsigned mdescIndex) override;
+    void replaceSendSource(RealtimeMediaSource&, const String& mid) override;
 
     void stop() override;