From a7585623fe6e070989d385dfa36c832095046817 Mon Sep 17 00:00:00 2001 From: "youenn@apple.com" Date: Wed, 14 Nov 2018 17:35:02 +0000 Subject: [PATCH] Calling removeTrack on different RTCPeerConnection should throw InvalidAccessError https://bugs.webkit.org/show_bug.cgi?id=191603 Reviewed by Chris Dumez. LayoutTests/imported/w3c: * web-platform-tests/webrtc/RTCPeerConnection-removeTrack.https-expected.txt: Source/WebCore: Make sure to check that the sender peer connection backend is matching. Covered by rebased WPT test. * Modules/mediastream/RTCPeerConnection.cpp: (WebCore::RTCPeerConnection::removeTrack): * Modules/mediastream/RTCRtpSender.cpp: (WebCore::RTCRtpSender::isCreatedBy const): * Modules/mediastream/RTCRtpSender.h: LayoutTests: Removed obsolete test. * fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt: * fast/mediastream/RTCPeerConnection-add-removeTrack.html: git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238180 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 12 ++++++++++++ .../RTCPeerConnection-add-removeTrack-expected.txt | 3 --- .../mediastream/RTCPeerConnection-add-removeTrack.html | 4 ---- LayoutTests/imported/w3c/ChangeLog | 9 +++++++++ .../RTCPeerConnection-removeTrack.https-expected.txt | 4 ++-- Source/WebCore/ChangeLog | 16 ++++++++++++++++ Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp | 5 ++++- Source/WebCore/Modules/mediastream/RTCRtpSender.cpp | 5 +++++ Source/WebCore/Modules/mediastream/RTCRtpSender.h | 2 ++ 9 files changed, 50 insertions(+), 10 deletions(-) diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 7ef9ce9..ca0ef32 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,5 +1,17 @@ 2018-11-14 Youenn Fablet + Calling removeTrack on different RTCPeerConnection should throw InvalidAccessError + https://bugs.webkit.org/show_bug.cgi?id=191603 + + Reviewed by Chris Dumez. + + Removed obsolete test. + + * fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt: + * fast/mediastream/RTCPeerConnection-add-removeTrack.html: + +2018-11-14 Youenn Fablet + Add support for transport and peerConnection stats https://bugs.webkit.org/show_bug.cgi?id=191592 diff --git a/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt b/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt index d81f833..08d9d64 100644 --- a/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt +++ b/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack-expected.txt @@ -40,9 +40,6 @@ PASS pc.getSenders()[1] is sender2 PASS pc.removeTrack(sender) did not throw exception. Sender is still in getSenders() list PASS pc.getSenders().length is 2 -PASS senderFromPc2 = pc2.addTrack(track, stream) did not throw exception. -removeTrack() with 'foreign' sender must be ignored (not throw) -PASS pc.removeTrack(senderFromPc2) did not throw exception. PASS pc.addTrack(null); threw exception TypeError: Argument 1 ('track') to RTCPeerConnection.addTrack must be an instance of MediaStreamTrack. PASS pc.addTrack(undefined); threw exception TypeError: Argument 1 ('track') to RTCPeerConnection.addTrack must be an instance of MediaStreamTrack. PASS pc.removeTrack(null); threw exception TypeError: Argument 1 ('sender') to RTCPeerConnection.removeTrack must be an instance of RTCRtpSender. diff --git a/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack.html b/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack.html index 405909d..202230f 100644 --- a/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack.html +++ b/LayoutTests/fast/mediastream/RTCPeerConnection-add-removeTrack.html @@ -91,10 +91,6 @@ debug("Sender is still in getSenders() list") shouldBe("pc.getSenders().length", "2"); - shouldNotThrow("senderFromPc2 = pc2.addTrack(track, stream)"); - debug("removeTrack() with 'foreign' sender must be ignored (not throw)"); - shouldNotThrow("pc.removeTrack(senderFromPc2)"); - shouldThrow("pc.addTrack(null);"); shouldThrow("pc.addTrack(undefined);"); shouldThrow("pc.removeTrack(null);"); diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog index 2e2e05e..e4b008a 100644 --- a/LayoutTests/imported/w3c/ChangeLog +++ b/LayoutTests/imported/w3c/ChangeLog @@ -1,5 +1,14 @@ 2018-11-14 Youenn Fablet + Calling removeTrack on different RTCPeerConnection should throw InvalidAccessError + https://bugs.webkit.org/show_bug.cgi?id=191603 + + Reviewed by Chris Dumez. + + * web-platform-tests/webrtc/RTCPeerConnection-removeTrack.https-expected.txt: + +2018-11-14 Youenn Fablet + Add support for RTCRtpCodecParameters.sdpFmtpLine https://bugs.webkit.org/show_bug.cgi?id=191591 diff --git a/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-removeTrack.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-removeTrack.https-expected.txt index 3ca4167..15ce3fc 100644 --- a/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-removeTrack.https-expected.txt +++ b/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-removeTrack.https-expected.txt @@ -3,8 +3,8 @@ PASS addTransceiver - Calling removeTrack when connection is closed should throw PASS addTrack - Calling removeTrack when connection is closed should throw InvalidStateError PASS addTransceiver - Calling removeTrack on different connection that is closed should throw InvalidStateError PASS addTrack - Calling removeTrack on different connection that is closed should throw InvalidStateError -FAIL addTransceiver - Calling removeTrack on different connection should throw InvalidAccessError assert_throws: function "() => pc2.removeTrack(sender)" did not throw -FAIL addTrack - Calling removeTrack on different connection should throw InvalidAccessError assert_throws: function "() => pc2.removeTrack(sender)" did not throw +PASS addTransceiver - Calling removeTrack on different connection should throw InvalidAccessError +PASS addTrack - Calling removeTrack on different connection should throw InvalidAccessError PASS addTransceiver - Calling removeTrack with valid sender should set sender.track to null PASS addTrack - Calling removeTrack with valid sender should set sender.track to null PASS Calling removeTrack with currentDirection sendrecv should set direction to recvonly diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index d11f4bc..0cbb16c 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,19 @@ +2018-11-14 Youenn Fablet + + Calling removeTrack on different RTCPeerConnection should throw InvalidAccessError + https://bugs.webkit.org/show_bug.cgi?id=191603 + + Reviewed by Chris Dumez. + + Make sure to check that the sender peer connection backend is matching. + Covered by rebased WPT test. + + * Modules/mediastream/RTCPeerConnection.cpp: + (WebCore::RTCPeerConnection::removeTrack): + * Modules/mediastream/RTCRtpSender.cpp: + (WebCore::RTCRtpSender::isCreatedBy const): + * Modules/mediastream/RTCRtpSender.h: + 2018-11-14 Fujii Hironori [curl] Unify CookieJarCurlDatabase and the abstract class CookieJarCurl diff --git a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp index 1386cac..62840e6 100644 --- a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp +++ b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp @@ -131,7 +131,10 @@ ExceptionOr RTCPeerConnection::removeTrack(RTCRtpSender& sender) INFO_LOG(LOGIDENTIFIER); if (isClosed()) - return Exception { InvalidStateError }; + return Exception { InvalidStateError, "RTCPeerConnection is closed"_s }; + + if (!sender.isCreatedBy(*m_backend)) + return Exception { InvalidAccessError, "RTCPeerConnection did not create the given sender"_s }; bool shouldAbort = true; RTCRtpTransceiver* senderTransceiver = nullptr; diff --git a/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp b/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp index ef39647..2744d06 100644 --- a/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp +++ b/Source/WebCore/Modules/mediastream/RTCRtpSender.cpp @@ -122,6 +122,11 @@ void RTCRtpSender::getStats(Ref&& promise) m_connection->getStats(*this, WTFMove(promise)); } +bool RTCRtpSender::isCreatedBy(const PeerConnectionBackend& connection) const +{ + return &connection == m_connection.get(); +} + std::optional RTCRtpSender::getCapabilities(ScriptExecutionContext& context, const String& kind) { return PeerConnectionBackend::senderCapabilities(context, kind); diff --git a/Source/WebCore/Modules/mediastream/RTCRtpSender.h b/Source/WebCore/Modules/mediastream/RTCRtpSender.h index 46cfa6a..3f4483f 100644 --- a/Source/WebCore/Modules/mediastream/RTCRtpSender.h +++ b/Source/WebCore/Modules/mediastream/RTCRtpSender.h @@ -71,6 +71,8 @@ public: void getStats(Ref&&); + bool isCreatedBy(const PeerConnectionBackend&) const; + private: RTCRtpSender(PeerConnectionBackend&, String&& trackKind, Vector&& mediaStreamIds, std::unique_ptr&&); -- 1.8.3.1