Attachment 347988 Details for Bug 188914 – Patch

[patch] Patch

bug-188914-20180823222049.patch (text/plain), 11.97 KB, created by youenn fablet on 2018-08-23 22:20:50 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-08-23 22:20:50 PDT

Size: 11.97 KB

patch

obsolete

>Subversion Revision: 235135
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index aceaa79f9b042408503901ee68d0b99c2b80213b..0c9876ff36bd208de2e7d41915dc09143ffbdb66 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-08-23  Youenn Fablet  <youenn@apple.com>
>+
>+        libwebrtc PeerConnection::AddTrack sometimes fail
>+        https://bugs.webkit.org/show_bug.cgi?id=188914
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        AddTrack may fail so test the result and if not successful, make JS addTrack to throw an exception.
>+        This makes some tests to fail now.
>+        These tests should pass again when unified plan will be enabled.
>+        Covered by rebased tests.
>+
>+        * Modules/mediastream/PeerConnectionBackend.h:
>+        (WebCore::PeerConnectionBackend::notifyAddedTrack):
>+        * Modules/mediastream/RTCPeerConnection.cpp:
>+        (WebCore::RTCPeerConnection::addTrack):
>+        (WebCore::RTCPeerConnection::addTransceiver):
>+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
>+        (WebCore::LibWebRTCMediaEndpoint::addTrack):
>+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:
>+        * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
>+        (WebCore::LibWebRTCPeerConnectionBackend::notifyAddedTrack):
>+        * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h:
>+
> 2018-08-20  Youenn Fablet  <youenn@apple.com>
> 
>         Update libwebrtc up to 984f1a80c0
>diff --git a/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h b/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h
>index 29da984376f16f537d3f067b669131c3b4a3ce67..3a660118e8faf4f5d51359380dfc28c4c8fab7b0 100644
>--- a/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h
>+++ b/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h
>@@ -102,7 +102,7 @@ public:
> 
>     virtual Ref<RTCRtpReceiver> createReceiver(const String& transceiverMid, const String& trackKind, const String& trackId) = 0;
>     virtual void replaceTrack(RTCRtpSender&, RefPtr<MediaStreamTrack>&&, DOMPromiseDeferred<void>&&) = 0;
>-    virtual void notifyAddedTrack(RTCRtpSender&) { }
>+    virtual bool notifyAddedTrack(RTCRtpSender&) { return false; }
>     virtual void notifyRemovedTrack(RTCRtpSender&) { }
> 
>     virtual RTCRtpParameters getParameters(RTCRtpSender&) const { return { }; }
>diff --git a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
>index 225edfadc96df0dad7904e36aaad69909aa24ae3..a04f81e1fb12727cffbd508918b23c760a22baef 100644
>--- a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
>+++ b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
>@@ -154,7 +154,9 @@ ExceptionOr<Ref<RTCRtpSender>> RTCPeerConnection::addTrack(Ref<MediaStreamTrack>
>         m_transceiverSet->append(WTFMove(transceiver));
>     }
> 
>-    m_backend->notifyAddedTrack(*sender);
>+    if (!m_backend->notifyAddedTrack(*sender))
>+        return Exception { InvalidAccessError, "Unable to add track"_s };
>+
>     return Ref<RTCRtpSender> { *sender };
> }
> 
>@@ -199,7 +201,9 @@ ExceptionOr<Ref<RTCRtpTransceiver>> RTCPeerConnection::addTransceiver(AddTransce
>     const String& trackKind = track->kind();
> 
>     auto sender = RTCRtpSender::create(WTFMove(track), Vector<String>(), *this);
>-    m_backend->notifyAddedTrack(sender);
>+    if (!m_backend->notifyAddedTrack(sender))
>+        return Exception { InvalidAccessError, "Unable to add track"_s };
>+
>     return completeAddTransceiver(WTFMove(sender), init, trackId, trackKind);
> }
> 
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
>index 743d18f0cb5b13e2288f21208bbbbd2df4492803..0d58b9f8a811636be988e2883c64e4892a6f5701 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
>@@ -193,7 +193,7 @@ void LibWebRTCMediaEndpoint::doSetRemoteDescription(RTCSessionDescription& descr
>     startLoggingStats();
> }
> 
>-void LibWebRTCMediaEndpoint::addTrack(RTCRtpSender& sender, MediaStreamTrack& track, const Vector<String>& mediaStreamIds)
>+bool LibWebRTCMediaEndpoint::addTrack(RTCRtpSender& sender, MediaStreamTrack& track, const Vector<String>& mediaStreamIds)
> {
>     ASSERT(m_backend);
> 
>@@ -214,15 +214,19 @@ void LibWebRTCMediaEndpoint::addTrack(RTCRtpSender& sender, MediaStreamTrack& tr
>         auto trackSource = RealtimeOutgoingAudioSource::create(track.privateTrack());
>         auto audioTrack = m_peerConnectionFactory.CreateAudioTrack(track.id().utf8().data(), trackSource.ptr());
>         m_peerConnectionBackend.addAudioSource(WTFMove(trackSource));
>-        m_senders.add(&sender, m_backend->AddTrack(audioTrack.get(), WTFMove(ids)).MoveValue());
>-        return;
>+        auto rtpSender = m_backend->AddTrack(audioTrack.get(), WTFMove(ids));
>+        if (rtpSender.ok())
>+            m_senders.add(&sender, rtpSender.MoveValue());
>+        return rtpSender.ok();
>     }
>     case RealtimeMediaSource::Type::Video: {
>         auto videoSource = RealtimeOutgoingVideoSource::create(track.privateTrack());
>         auto videoTrack = m_peerConnectionFactory.CreateVideoTrack(track.id().utf8().data(), videoSource.ptr());
>         m_peerConnectionBackend.addVideoSource(WTFMove(videoSource));
>-        m_senders.add(&sender, m_backend->AddTrack(videoTrack.get(), WTFMove(ids)).MoveValue());
>-        return;
>+        auto rtpSender = m_backend->AddTrack(videoTrack.get(), WTFMove(ids));
>+        if (rtpSender.ok())
>+            m_senders.add(&sender, rtpSender.MoveValue());
>+        return rtpSender.ok();
>     }
>     case RealtimeMediaSource::Type::None:
>         ASSERT_NOT_REACHED();
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h
>index d409a1b23dd7fed899cad16ca6b292a4c60f498e..30a2a8ba36bfe58b482a5ed52d8c39ca9c3b2af9 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h
>@@ -96,7 +96,7 @@ public:
>     RefPtr<RTCSessionDescription> pendingLocalDescription() const;
>     RefPtr<RTCSessionDescription> pendingRemoteDescription() const;
> 
>-    void addTrack(RTCRtpSender&, MediaStreamTrack&, const Vector<String>&);
>+    bool addTrack(RTCRtpSender&, MediaStreamTrack&, const Vector<String>&);
>     void removeTrack(RTCRtpSender&);
>     RTCRtpParameters getRTCRtpSenderParameters(RTCRtpSender&);
> 
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp
>index 327bc88fe854598f26de8f9690496912d9ba52ac..b811497dd2db20ca4824be46a439bb92dfcfe020 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp
>@@ -349,10 +349,10 @@ RefPtr<RTCSessionDescription> LibWebRTCPeerConnectionBackend::remoteDescription(
>     return m_endpoint->remoteDescription();
> }
> 
>-void LibWebRTCPeerConnectionBackend::notifyAddedTrack(RTCRtpSender& sender)
>+bool LibWebRTCPeerConnectionBackend::notifyAddedTrack(RTCRtpSender& sender)
> {
>     ASSERT(sender.track());
>-    m_endpoint->addTrack(sender, *sender.track(), sender.mediaStreamIds());
>+    return m_endpoint->addTrack(sender, *sender.track(), sender.mediaStreamIds());
> }
> 
> void LibWebRTCPeerConnectionBackend::notifyRemovedTrack(RTCRtpSender& sender)
>diff --git a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h
>index d5e826f8bb5d16ec3da14f2ba4589fcb057d2d4b..1745b24d1ec703649fedce0737148b035b224e88 100644
>--- a/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h
>+++ b/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.h
>@@ -92,7 +92,7 @@ private:
>     void removeRemoteStream(MediaStream*);
>     void addRemoteStream(Ref<MediaStream>&&);
> 
>-    void notifyAddedTrack(RTCRtpSender&) final;
>+    bool notifyAddedTrack(RTCRtpSender&) final;
>     void notifyRemovedTrack(RTCRtpSender&) final;
> 
>     struct VideoReceiver {
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index c687430a12a483bc53197776f785abf77dfeba42..43e0132a881a2d1cd136d2576de5978cc9c23a6e 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-08-23  Youenn Fablet  <youenn@apple.com>
>+
>+        libwebrtc PeerConnection::AddTrack sometimes fail
>+        https://bugs.webkit.org/show_bug.cgi?id=188914
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt:
>+
> 2018-08-20  Youenn Fablet  <youenn@apple.com>
> 
>         Update libwebrtc up to 984f1a80c0
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 0e4dda7c09e4074dc99e534b7da895f504d9c327..e280badd055379af1000b2cabd15af80aca2f006 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-08-23  Youenn Fablet  <youenn@apple.com>
>+
>+        libwebrtc PeerConnection::AddTrack sometimes fail
>+        https://bugs.webkit.org/show_bug.cgi?id=188914
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * web-platform-tests/webrtc/RTCPeerConnection-addTrack.https-expected.txt:
>+
> 2018-08-21  Youenn Fablet  <youenn@apple.com>
> 
>         Update libwebrtc up to 984f1a80c0
>diff --git a/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt b/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt
>index d81f83330a233d7b7360efc1b9e2f9e6cc3367c4..89bcf5203409ee1ecc6ce961918332765fc4d761 100644
>--- a/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt
>+++ b/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt
>@@ -33,10 +33,10 @@ PASS transceiver.receiver is receiver
> PASS transceiver.stopped is false
> Try to add same track again
> PASS sender = pc.addTrack(track, stream) threw exception InvalidAccessError: The object does not support the operation or argument..
>-PASS sender2 = pc.addTrack(track2, stream, stream2) did not throw exception.
>+FAIL sender2 = pc.addTrack(track2, stream, stream2) should not throw exception. Threw exception InvalidAccessError: Unable to add track.
> PASS pc.getSenders().length is 2
> PASS pc.getSenders()[0] is sender
>-PASS pc.getSenders()[1] is sender2
>+FAIL pc.getSenders()[1] should be undefined (of type undefined). Was [object RTCRtpSender] (of type object).
> PASS pc.removeTrack(sender) did not throw exception.
> Sender is still in getSenders() list
> PASS pc.getSenders().length is 2
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-addTrack.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-addTrack.https-expected.txt
>index 78565279eb53ebda2af9176227842185b4760859..78a29d490eb3a616f0e8aec2c00b4aaa7b8aa635 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-addTrack.https-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-addTrack.https-expected.txt
>@@ -2,7 +2,7 @@
> PASS addTrack when pc is closed should throw InvalidStateError 
> PASS addTrack with single track argument and no mediaStream should succeed 
> PASS addTrack with single track argument and single mediaStream should succeed 
>-PASS addTrack with single track argument and multiple mediaStreams should succeed 
>+FAIL addTrack with single track argument and multiple mediaStreams should succeed promise_test: Unhandled rejection with value: object "InvalidAccessError: Unable to add track"
> PASS Adding the same track multiple times should throw InvalidAccessError 
> PASS addTrack with existing sender with null track, same kind, and recvonly direction should reuse sender 
> PASS addTrack with existing sender with null track, same kind, and sendrecv direction should create new sender

Actions: View | Formatted Diff | Diff

Attachments on bug 188914: 347988 | 348024