WebKit Bugzilla
Attachment 368392 Details for
Bug 196808
: RTCTrackEvent should be delayed until the whole remote description is set
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-196808-20190427003019.patch (text/plain), 12.05 KB, created by
youenn fablet
on 2019-04-27 00:30:19 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
youenn fablet
Created:
2019-04-27 00:30:19 PDT
Size:
12.05 KB
patch
obsolete
>Subversion Revision: 244694 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index fa95d1668c4aea6a0b3fc93d41ba1850c81917be..d35e87b3777b700f063673b5e3de8aa9702fde77 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,31 @@ >+2019-04-27 Youenn Fablet <youenn@apple.com> >+ >+ RTCTrackEvent should be delayed until the whole remote description is set >+ https://bugs.webkit.org/show_bug.cgi?id=196808 >+ <rdar://problem/49802649> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ As per https://w3c.github.io/webrtc-pc/#set-description, >+ fire events just before resolving the setRemoteDescription promise. >+ This ensures that the exposed stream has all necessary tracks from the beginning. >+ Pending track events are created in LibWebRTCMediaEndpoint and stored in PeerConnectionBackend. >+ >+ Covered by updated test. >+ >+ * Modules/mediastream/PeerConnectionBackend.cpp: >+ (WebCore::PeerConnectionBackend::setRemoteDescriptionSucceeded): >+ (WebCore::PeerConnectionBackend::setRemoteDescriptionFailed): >+ (WebCore::PeerConnectionBackend::addPendingTrackEvent): >+ (WebCore::PeerConnectionBackend::stop): >+ * Modules/mediastream/PeerConnectionBackend.h: >+ * Modules/mediastream/RTCPeerConnection.cpp: >+ * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp: >+ (WebCore::LibWebRTCMediaEndpoint::addRemoteTrack): >+ (WebCore::LibWebRTCMediaEndpoint::addPendingTrackEvent): >+ (WebCore::LibWebRTCMediaEndpoint::newTransceiver): >+ * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h: >+ > 2019-04-26 Youenn Fablet <youenn@apple.com> > > Use normal loading path for ping loads >diff --git a/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp b/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp >index 04ec535064a7c89591afbba2620577165814fc4d..21adefe597ac920070d7f13a9290c42bbdc4687b 100644 >--- a/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp >+++ b/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp >@@ -43,6 +43,7 @@ > #include "RTCPeerConnection.h" > #include "RTCPeerConnectionIceEvent.h" > #include "RTCRtpCapabilities.h" >+#include "RTCTrackEvent.h" > #include "RuntimeEnabledFeatures.h" > #include <wtf/text/StringBuilder.h> > #include <wtf/text/StringConcatenateNumbers.h> >@@ -248,6 +249,21 @@ void PeerConnectionBackend::setRemoteDescriptionSucceeded() > ASSERT(isMainThread()); > ALWAYS_LOG(LOGIDENTIFIER, "Set remote description succeeded"); > >+ ASSERT(!m_peerConnection.isClosed()); >+ >+ auto events = WTFMove(m_pendingTrackEvents); >+ for (auto& event : events) { >+ auto& track = event.track.get(); >+ >+ m_peerConnection.fireEvent(RTCTrackEvent::create(eventNames().trackEvent, Event::CanBubble::No, Event::IsCancelable::No, WTFMove(event.receiver), WTFMove(event.track), WTFMove(event.streams), WTFMove(event.transceiver))); >+ >+ if (m_peerConnection.isClosed()) >+ return; >+ >+ // FIXME: As per spec, we should set muted to 'false' when starting to receive the content from network. >+ track.source().setMuted(false); >+ } >+ > if (m_peerConnection.isClosed()) > return; > >@@ -262,15 +278,22 @@ void PeerConnectionBackend::setRemoteDescriptionFailed(Exception&& exception) > ASSERT(isMainThread()); > ALWAYS_LOG(LOGIDENTIFIER, "Set remote description failed:", exception.message()); > >- if (m_peerConnection.isClosed()) >- return; >+ ASSERT(m_pendingTrackEvents.isEmpty()); >+ m_pendingTrackEvents.clear(); > >+ ASSERT(!m_peerConnection.isClosed()); > ASSERT(m_setDescriptionPromise); > > m_setDescriptionPromise->reject(WTFMove(exception)); > m_setDescriptionPromise = WTF::nullopt; > } > >+void PeerConnectionBackend::addPendingTrackEvent(PendingTrackEvent&& event) >+{ >+ ASSERT(!m_peerConnection.isClosed()); >+ m_pendingTrackEvents.append(WTFMove(event)); >+} >+ > static String extractIPAddres(const String& sdp) > { > ASSERT(sdp.contains(" host ")); >@@ -491,6 +514,8 @@ void PeerConnectionBackend::stop() > m_setDescriptionPromise = WTF::nullopt; > m_addIceCandidatePromise = WTF::nullopt; > >+ m_pendingTrackEvents.clear(); >+ > doStop(); > } > >diff --git a/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h b/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h >index dbbed4df65c1a7426d405f6af948f34ffb2cf9a7..e1bdabc537ae75109594af653eb96fb8e90a7e2a 100644 >--- a/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h >+++ b/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h >@@ -192,6 +192,14 @@ protected: > > String filterSDP(String&&) const; > >+ struct PendingTrackEvent { >+ Ref<RTCRtpReceiver> receiver; >+ Ref<MediaStreamTrack> track; >+ Vector<RefPtr<MediaStream>> streams; >+ RefPtr<RTCRtpTransceiver> transceiver; >+ }; >+ void addPendingTrackEvent(PendingTrackEvent&&); >+ > private: > virtual void doCreateOffer(RTCOfferOptions&&) = 0; > virtual void doCreateAnswer(RTCAnswerOptions&&) = 0; >@@ -221,6 +229,8 @@ private: > }; > Vector<PendingICECandidate> m_pendingICECandidates; > >+ Vector<PendingTrackEvent> m_pendingTrackEvents; >+ > #if !RELEASE_LOG_DISABLED > Ref<const Logger> m_logger; > const void* m_logIdentifier; >diff --git a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp >index af2b7c7dcb9c52125b257cf108332413aec9e2be..8713fdded1711cb80b8b22fe78cd4fec10dde7a6 100644 >--- a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp >+++ b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp >@@ -52,7 +52,6 @@ > #include "RTCIceCandidate.h" > #include "RTCPeerConnectionIceEvent.h" > #include "RTCSessionDescription.h" >-#include "RTCTrackEvent.h" > #include <wtf/CryptographicallyRandomNumber.h> > #include <wtf/IsoMallocInlines.h> > #include <wtf/MainThread.h> >diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp >index d50fb0930721e257d35af537311801125361fedf..c7749db54fa8142332e90ef372fdc0aedc33f8ea 100644 >--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp >+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp >@@ -47,7 +47,6 @@ > #include "RTCPeerConnection.h" > #include "RTCSessionDescription.h" > #include "RTCStatsReport.h" >-#include "RTCTrackEvent.h" > #include "RealtimeIncomingAudioSource.h" > #include "RealtimeIncomingVideoSource.h" > #include "RealtimeOutgoingAudioSource.h" >@@ -395,10 +394,10 @@ void LibWebRTCMediaEndpoint::addRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiv > > receiver->setBackend(std::make_unique<LibWebRTCRtpReceiverBackend>(WTFMove(rtcReceiver))); > auto& track = receiver->track(); >- fireTrackEvent(receiver.releaseNonNull(), track, rtcStreams, nullptr); >+ addPendingTrackEvent(receiver.releaseNonNull(), track, rtcStreams, nullptr); > } > >-void LibWebRTCMediaEndpoint::fireTrackEvent(Ref<RTCRtpReceiver>&& receiver, MediaStreamTrack& track, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>& rtcStreams, RefPtr<RTCRtpTransceiver>&& transceiver) >+void LibWebRTCMediaEndpoint::addPendingTrackEvent(Ref<RTCRtpReceiver>&& receiver, MediaStreamTrack& track, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>& rtcStreams, RefPtr<RTCRtpTransceiver>&& transceiver) > { > Vector<RefPtr<MediaStream>> streams; > for (auto& rtcStream : rtcStreams) { >@@ -411,11 +410,7 @@ void LibWebRTCMediaEndpoint::fireTrackEvent(Ref<RTCRtpReceiver>&& receiver, Medi > }); > m_remoteStreamsFromRemoteTrack.add(&track, WTFMove(streamIds)); > >- m_peerConnectionBackend.connection().fireEvent(RTCTrackEvent::create(eventNames().trackEvent, >- Event::CanBubble::No, Event::IsCancelable::No, WTFMove(receiver), &track, WTFMove(streams), WTFMove(transceiver))); >- >- // FIXME: As per spec, we should set muted to 'false' when starting to receive the content from network. >- track.source().setMuted(false); >+ m_peerConnectionBackend.addPendingTrackEvent({ WTFMove(receiver), makeRef(track), WTFMove(streams), WTFMove(transceiver) }); > } > > static inline void setExistingReceiverSourceTrack(RealtimeMediaSource& existingSource, webrtc::RtpReceiverInterface& rtcReceiver) >@@ -490,7 +485,7 @@ void LibWebRTCMediaEndpoint::newTransceiver(rtc::scoped_refptr<webrtc::RtpTransc > if (transceiver) { > auto rtcReceiver = rtcTransceiver->receiver(); > setExistingReceiverSourceTrack(transceiver->receiver().track().source(), *rtcReceiver); >- fireTrackEvent(makeRef(transceiver->receiver()), transceiver->receiver().track(), rtcReceiver->streams(), makeRef(*transceiver)); >+ addPendingTrackEvent(makeRef(transceiver->receiver()), transceiver->receiver().track(), rtcReceiver->streams(), makeRef(*transceiver)); > return; > } > >@@ -501,7 +496,7 @@ void LibWebRTCMediaEndpoint::newTransceiver(rtc::scoped_refptr<webrtc::RtpTransc > > auto& newTransceiver = m_peerConnectionBackend.newRemoteTransceiver(std::make_unique<LibWebRTCRtpTransceiverBackend>(WTFMove(rtcTransceiver)), source.releaseNonNull()); > >- fireTrackEvent(makeRef(newTransceiver.receiver()), newTransceiver.receiver().track(), rtcReceiver->streams(), makeRef(newTransceiver)); >+ addPendingTrackEvent(makeRef(newTransceiver.receiver()), newTransceiver.receiver().track(), rtcReceiver->streams(), makeRef(newTransceiver)); > } > > void LibWebRTCMediaEndpoint::removeRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface>&& receiver) >diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h >index 6ec94cd03b6f4921b692aee252e6f1da8eabbef5..62bb476a0e3b21f6b39e2daf2b26bbe1fcfedf4f 100644 >--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h >+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h >@@ -143,7 +143,7 @@ private: > void newTransceiver(rtc::scoped_refptr<webrtc::RtpTransceiverInterface>&&); > void removeRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface>&&); > >- void fireTrackEvent(Ref<RTCRtpReceiver>&&, MediaStreamTrack&, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>&, RefPtr<RTCRtpTransceiver>&&); >+ void addPendingTrackEvent(Ref<RTCRtpReceiver>&&, MediaStreamTrack&, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>&, RefPtr<RTCRtpTransceiver>&&); > > template<typename T> > Optional<Backends> createTransceiverBackends(T&&, const RTCRtpTransceiverInit&, LibWebRTCRtpSenderBackend::Source&&); >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 2c644967d530ba8b5428671a0e9967f4deb633ad..1e124e286b6786dbbd6ff62d6bdd2ef5437e9769 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,13 @@ >+2019-04-27 Youenn Fablet <youenn@apple.com> >+ >+ RTCTrackEvent should be delayed until the whole remote description is set >+ https://bugs.webkit.org/show_bug.cgi?id=196808 >+ <rdar://problem/49802649> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * webrtc/video-addTrack.html: >+ > 2019-04-26 Youenn Fablet <youenn@apple.com> > > Enable Fetch Keep Alive by default >diff --git a/LayoutTests/webrtc/video-addTrack.html b/LayoutTests/webrtc/video-addTrack.html >index 45a1baa00870259c3b62ccb3d80c6e6a9e4326ef..88c523f7c388598b1d2cc2331ec877f4b8457191 100644 >--- a/LayoutTests/webrtc/video-addTrack.html >+++ b/LayoutTests/webrtc/video-addTrack.html >@@ -60,6 +60,8 @@ function testBasicVideoExchangeWithAddTrack(waitForSecondTrack) > assert_equals(trackEvent.track.id, stream.getVideoTracks()[0].id); > else > assert_equals(trackEvent.track.id, stream.getAudioTracks()[0].id); >+ assert_equals(trackEvent.streams.length, 1); >+ assert_equals(trackEvent.streams[0].getTracks().length, 2); > }, " track " + count + ", wait = " + waitForSecondTrack); > if (count++ === (waitForSecondTrack ? 1 : 0)) > resolve(trackEvent.streams[0]);
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 196808
: 368392