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 Eric Carlson.

Source/WebCore:

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:

LayoutTests:

* webrtc/video-addTrack.html:


Canonical link: https://commits.webkit.org/211568@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244736 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,13 @@
+2019-04-29  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 Eric Carlson.
+
+        * webrtc/video-addTrack.html:
+
 2019-04-29  Javier Fernandez  <jfernandez@igalia.com>
 
         Update the CSS Text WPT test suite

LayoutTests/webrtc/video-addTrack.html

@@ -60,6 +60,8 @@
                             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]);

Source/WebCore/ChangeLog

@@ -1,3 +1,31 @@
+2019-04-29  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 Eric Carlson.
+
+        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-25  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         REGRESSION(r244635): [GTK] Wrong background color used in non-dark mode

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();
 }
 

Source/WebCore/Modules/mediastream/PeerConnectionBackend.h

@@ -192,6 +192,14 @@ class PeerConnectionBackend
 
     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 @@ class PeerConnectionBackend
     };
     Vector<PendingICECandidate> m_pendingICECandidates;
 
+    Vector<PendingTrackEvent> m_pendingTrackEvents;
+
 #if !RELEASE_LOG_DISABLED
     Ref<const Logger> m_logger;
     const void* m_logIdentifier;

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>

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)

Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h

@@ -143,7 +143,7 @@ class LibWebRTCMediaEndpoint
     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&&);