WebKit Bugzilla
Attachment 342919 Details for
Bug 184911
: RTCRtpSender.replaceTrack(null) ends current track
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-184911-20180618112851.patch (text/plain), 13.98 KB, created by
youenn fablet
on 2018-06-18 02:28:53 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
youenn fablet
Created:
2018-06-18 02:28:53 PDT
Size:
13.98 KB
patch
obsolete
>Subversion Revision: 232923 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index d7be2d4937bab03e6e595fd06e31852505c7a076..2a524966c105654577ea0f058379e7b44c3104af 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,29 @@ >+2018-06-18 Youenn Fablet <youenn@apple.com> >+ >+ RTCRtpSender.replaceTrack(null) ends current track >+ https://bugs.webkit.org/show_bug.cgi?id=184911 >+ <rdar://problem/40758138> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Before the patch, when replacing the sender track by null, the previous track was stopped. >+ Instead of doing that, the track now stays alive and it is the realtime source that is stopped. >+ This ensures that the data is no longer sent while the track can still be used elsewhere. >+ >+ Covered by updated test. >+ >+ * Modules/mediastream/PeerConnectionBackend.h: >+ * Modules/mediastream/RTCPeerConnection.cpp: >+ (WebCore::RTCPeerConnection::enqueueReplaceTrackTask): >+ (WebCore::RTCPeerConnection::replaceTrack): >+ * Modules/mediastream/RTCPeerConnection.h: >+ * Modules/mediastream/RTCRtpSender.cpp: >+ (WebCore::RTCRtpSender::replaceTrack): >+ * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp: >+ (WebCore::updateTrackSource): >+ (WebCore::LibWebRTCPeerConnectionBackend::replaceTrack): >+ * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h: >+ > 2018-06-17 Zalan Bujtas <zalan@apple.com> > > Anonymous block collapsing can destroy the renderer's parent. >diff --git a/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h b/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h >index 0fa48cdab99c908a9c5159bad5aa7ef583465edb..08ca435ecbe4054e7ac991b89e4b60f659fbf51f 100644 >--- a/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h >+++ b/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h >@@ -101,7 +101,7 @@ public: > virtual Vector<RefPtr<MediaStream>> getRemoteStreams() const = 0; > > virtual Ref<RTCRtpReceiver> createReceiver(const String& transceiverMid, const String& trackKind, const String& trackId) = 0; >- virtual void replaceTrack(RTCRtpSender&, Ref<MediaStreamTrack>&&, DOMPromiseDeferred<void>&&) = 0; >+ virtual void replaceTrack(RTCRtpSender&, RefPtr<MediaStreamTrack>&&, DOMPromiseDeferred<void>&&) = 0; > virtual void notifyAddedTrack(RTCRtpSender&) { } > virtual void notifyRemovedTrack(RTCRtpSender&) { } > >diff --git a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp >index 165fa5dfd12b2c2dcdef73e4297107c32d8bb566..afe59fa15ebd8b019472c9a7aa561e39fd5f7b21 100644 >--- a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp >+++ b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp >@@ -566,13 +566,20 @@ void RTCPeerConnection::fireEvent(Event& event) > dispatchEvent(event); > } > >-void RTCPeerConnection::enqueueReplaceTrackTask(RTCRtpSender& sender, Ref<MediaStreamTrack>&& withTrack, DOMPromiseDeferred<void>&& promise) >+void RTCPeerConnection::enqueueReplaceTrackTask(RTCRtpSender& sender, RefPtr<MediaStreamTrack>&& withTrack, DOMPromiseDeferred<void>&& promise) > { > scriptExecutionContext()->postTask([protectedThis = makeRef(*this), protectedSender = makeRef(sender), promise = WTFMove(promise), withTrack = WTFMove(withTrack)](ScriptExecutionContext&) mutable { > if (protectedThis->isClosed()) > return; >+ >+ if (!withTrack) { >+ protectedSender->setTrackToNull(); >+ promise.resolve(); >+ return; >+ } >+ > bool hasTrack = protectedSender->track(); >- protectedSender->setTrack(WTFMove(withTrack)); >+ protectedSender->setTrack(withTrack.releaseNonNull()); > if (!hasTrack) > protectedThis->m_backend->notifyAddedTrack(protectedSender.get()); > promise.resolve(); >@@ -583,20 +590,12 @@ void RTCPeerConnection::replaceTrack(RTCRtpSender& sender, RefPtr<MediaStreamTra > { > INFO_LOG(LOGIDENTIFIER); > >- if (!withTrack) { >- scriptExecutionContext()->postTask([protectedSender = makeRef(sender), promise = WTFMove(promise)](ScriptExecutionContext&) mutable { >- protectedSender->setTrackToNull(); >- promise.resolve(); >- }); >- return; >- } >- >- if (!sender.track()) { >+ if (!sender.track() && withTrack) { > enqueueReplaceTrackTask(sender, withTrack.releaseNonNull(), WTFMove(promise)); > return; > } > >- m_backend->replaceTrack(sender, withTrack.releaseNonNull(), WTFMove(promise)); >+ m_backend->replaceTrack(sender, WTFMove(withTrack), WTFMove(promise)); > } > > RTCRtpParameters RTCPeerConnection::getParameters(RTCRtpSender& sender) const >diff --git a/Source/WebCore/Modules/mediastream/RTCPeerConnection.h b/Source/WebCore/Modules/mediastream/RTCPeerConnection.h >index e99751d6ba136ab413670f9c3058da4e27fb6cf5..f46ac9a6036dfcbbef8b6956c6b62dfaaf71e5ee 100644 >--- a/Source/WebCore/Modules/mediastream/RTCPeerConnection.h >+++ b/Source/WebCore/Modules/mediastream/RTCPeerConnection.h >@@ -153,7 +153,7 @@ public: > void disableICECandidateFiltering() { m_backend->disableICECandidateFiltering(); } > void enableICECandidateFiltering() { m_backend->enableICECandidateFiltering(); } > >- void enqueueReplaceTrackTask(RTCRtpSender&, Ref<MediaStreamTrack>&&, DOMPromiseDeferred<void>&&); >+ void enqueueReplaceTrackTask(RTCRtpSender&, RefPtr<MediaStreamTrack>&&, DOMPromiseDeferred<void>&&); > > void clearController() { m_controller = nullptr; } > >diff --git a/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp b/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp >index 93db3dbc2d46e90086054360c24c95d4382ae84b..18dc9bd64da3b1ea89c2c06cf3c99a99a02e0711 100644 >--- a/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp >+++ b/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp >@@ -89,9 +89,6 @@ void RTCRtpSender::replaceTrack(RefPtr<MediaStreamTrack>&& withTrack, DOMPromise > return; > } > >- if (!withTrack && m_track) >- m_track->stopTrack(); >- > m_backend->replaceTrack(*this, WTFMove(withTrack), WTFMove(promise)); > } > >diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp >index c0b988695612bca28533474a20f535f3d5d354d0..aaf2441c5ebed3c44d5eaab7c54e080b690d6caf 100644 >--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp >+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp >@@ -372,12 +372,23 @@ void LibWebRTCPeerConnectionBackend::addRemoteStream(Ref<MediaStream>&& mediaStr > m_remoteStreams.append(WTFMove(mediaStream)); > } > >-void LibWebRTCPeerConnectionBackend::replaceTrack(RTCRtpSender& sender, Ref<MediaStreamTrack>&& track, DOMPromiseDeferred<void>&& promise) >+template<typename Source> >+static inline bool updateTrackSource(Source& source, MediaStreamTrack* track) >+{ >+ if (!track) { >+ source->stop(); >+ return true; >+ } >+ return source->setSource(track->privateTrack()); >+} >+ >+void LibWebRTCPeerConnectionBackend::replaceTrack(RTCRtpSender& sender, RefPtr<MediaStreamTrack>&& track, DOMPromiseDeferred<void>&& promise) > { > ASSERT(sender.track()); >+ > auto* currentTrack = sender.track(); > >- ASSERT(currentTrack->source().type() == track->source().type()); >+ ASSERT(!track || currentTrack->source().type() == track->source().type()); > switch (currentTrack->source().type()) { > case RealtimeMediaSource::Type::None: > ASSERT_NOT_REACHED(); >@@ -386,7 +397,7 @@ void LibWebRTCPeerConnectionBackend::replaceTrack(RTCRtpSender& sender, Ref<Medi > case RealtimeMediaSource::Type::Audio: { > for (auto& audioSource : m_audioSources) { > if (&audioSource->source() == ¤tTrack->privateTrack()) { >- if (!audioSource->setSource(track->privateTrack())) { >+ if (!updateTrackSource(audioSource, track.get())) { > promise.reject(InvalidModificationError); > return; > } >@@ -400,7 +411,7 @@ void LibWebRTCPeerConnectionBackend::replaceTrack(RTCRtpSender& sender, Ref<Medi > case RealtimeMediaSource::Type::Video: { > for (auto& videoSource : m_videoSources) { > if (&videoSource->source() == ¤tTrack->privateTrack()) { >- if (!videoSource->setSource(track->privateTrack())) { >+ if (!updateTrackSource(videoSource, track.get())) { > promise.reject(InvalidModificationError); > return; > } >diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h >index 1b70675fb034bcdc8cb247f1b0d4e8fa12394451..83a0e20c92f372969ee3cb1a00723f0adb4c42f4 100644 >--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h >+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h >@@ -73,7 +73,7 @@ private: > RefPtr<RTCSessionDescription> currentRemoteDescription() const final; > RefPtr<RTCSessionDescription> pendingRemoteDescription() const final; > >- void replaceTrack(RTCRtpSender&, Ref<MediaStreamTrack>&&, DOMPromiseDeferred<void>&&) final; >+ void replaceTrack(RTCRtpSender&, RefPtr<MediaStreamTrack>&&, DOMPromiseDeferred<void>&&) final; > RTCRtpParameters getParameters(RTCRtpSender&) const final; > > void emulatePlatformEvent(const String&) final { } >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 410491dc411e29eb372e6437c5c6208b6b1636ce..a5a2d123feff7c894491d88cdba931286db3516c 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,17 @@ >+2018-06-18 Youenn Fablet <youenn@apple.com> >+ >+ RTCRtpSender.replaceTrack(null) ends current track >+ https://bugs.webkit.org/show_bug.cgi?id=184911 >+ <rdar://problem/40758138> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Added checks for readyState to ensure the track remains live. >+ Split the main test into several tests to ease readability. >+ >+ * webrtc/video-replace-track-to-null-expected.txt: >+ * webrtc/video-replace-track-to-null.html: >+ > 2018-06-18 Zan Dobersek <zdobersek@igalia.com> > > Unreviewed WPE gardening. Adding platform-specific baselines for >diff --git a/LayoutTests/webrtc/video-replace-track-to-null-expected.txt b/LayoutTests/webrtc/video-replace-track-to-null-expected.txt >index e8cad01ccd336ae82b495aefc5778e524af5061e..5c60c54d1387c4117b06579778d0ff54c2fa63d7 100644 >--- a/LayoutTests/webrtc/video-replace-track-to-null-expected.txt >+++ b/LayoutTests/webrtc/video-replace-track-to-null-expected.txt >@@ -1,3 +1,6 @@ > >-PASS Stopping sending video using replaceTrack >+PASS Set-up test >+PASS Test that frame counter increases >+PASS Test replaceTrack with null track >+PASS Test that frame counter no longer increases > >diff --git a/LayoutTests/webrtc/video-replace-track-to-null.html b/LayoutTests/webrtc/video-replace-track-to-null.html >index d0a7bd5dad484ad40a444b27777aa886112d99aa..b5e36b59afa624c5aac437c84085ab6eea4ad4ac 100644 >--- a/LayoutTests/webrtc/video-replace-track-to-null.html >+++ b/LayoutTests/webrtc/video-replace-track-to-null.html >@@ -59,41 +59,49 @@ async function testFrameEncodedStopped(connection, count, previousFramesNumber) > return testFrameEncodedStopped(connection, ++count, framesEncodedNumber); > } > >-promise_test((test) => { >+var sender; >+var sendingTrack; >+var connection; >+promise_test(async (test) => { > if (window.testRunner) > testRunner.setUserMediaPermission(true); > >- var sender; >- var frontStream; >- var connection; >- return navigator.mediaDevices.getUserMedia({ video: true }).then((stream) => { >- frontStream = stream; >- }).then(() => { >- return new Promise((resolve, reject) => { >- createConnections((firstConnection) => { >- connection = firstConnection; >- sender = firstConnection.addTrack(frontStream.getVideoTracks()[0], frontStream); >- }, (secondConnection) => { >- secondConnection.ontrack = (trackEvent) => { >- resolve(trackEvent.streams[0]); >- }; >- }); >- setTimeout(() => reject("Test timed out"), 5000); >+ const frontStream = await navigator.mediaDevices.getUserMedia({ video: true }); >+ >+ const remoteStream = await new Promise((resolve, reject) => { >+ createConnections((firstConnection) => { >+ connection = firstConnection; >+ sender = firstConnection.addTrack(frontStream.getVideoTracks()[0], frontStream); >+ }, (secondConnection) => { >+ secondConnection.ontrack = (trackEvent) => { >+ resolve(trackEvent.streams[0]); >+ }; > }); >- }).then((remoteStream) => { >- video.srcObject = remoteStream; >- return video.play(); >- }).then(() => { >- return testFrameEncodedStarted(connection); >- }).then(() => { >- }).then(() => { >- promise = sender.replaceTrack(null); >- assert_true(!!sender.track); >- return promise; >- }).then(() => { >- return testFrameEncodedStopped(connection); >+ setTimeout(() => reject("Test timed out"), 5000); > }); >-}, "Stopping sending video using replaceTrack"); >+ >+ video.srcObject = remoteStream; >+ await video.play(); >+}, "Set-up test"); >+ >+promise_test(async (test) => { >+ await testFrameEncodedStarted(connection); >+ >+ sendingTrack = sender.track; >+ assert_equals(sendingTrack.readyState, "live"); >+}, "Test that frame counter increases"); >+ >+promise_test(async (test) => { >+ promise = sender.replaceTrack(null); >+ assert_true(!!sender.track); >+ await promise; >+ assert_equals(sender.track, null); >+}, "Test replaceTrack with null track"); >+ >+promise_test(async (test) => { >+ await testFrameEncodedStopped(connection); >+ assert_equals(sendingTrack.readyState, "live"); >+}, "Test that frame counter no longer increases"); > </script> > </body> > </html>
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 184911
:
342919
|
342920
|
342923
|
342927
|
342999