Attachment 368392 Details for Bug 196808 – Patch

[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:

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]);

Actions: View | Formatted Diff | Diff

Attachments on bug 196808: 368392