RTCTrackEvent should be delayed until the whole remote description is set
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Apr 2019 16:24:31 +0000 (16:24 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Apr 2019 16:24:31 +0000 (16:24 +0000)
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:

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

LayoutTests/ChangeLog
LayoutTests/webrtc/video-addTrack.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp
Source/WebCore/Modules/mediastream/PeerConnectionBackend.h
Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h

index 218f6d5..ec96697 100644 (file)
@@ -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
index 45a1baa..88c523f 100644 (file)
@@ -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]);
index d3c9459..7c35cb1 100644 (file)
@@ -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
index 04ec535..21adefe 100644 (file)
@@ -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();
 }
 
index dbbed4d..e1bdabc 100644 (file)
@@ -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;
index af2b7c7..8713fdd 100644 (file)
@@ -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>
index d50fb09..c7749db 100644 (file)
@@ -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)
index 6ec94cd..62bb476 100644 (file)
@@ -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&&);