- Revision
- 219428
- Author
- commit-qu...@webkit.org
- Date
- 2017-07-12 15:31:45 -0700 (Wed, 12 Jul 2017)
Log Message
Accessing localDescription, remoteDescription, etc. after setTimeout raises EXC_BAD_ACCESS
https://bugs.webkit.org/show_bug.cgi?id=174323
<rdar://problem/33267876>
Patch by Youenn Fablet <you...@apple.com> on 2017-07-12
Reviewed by Eric Carlson.
Test: webrtc/calling-peerconnection-once-closed.html
In case the libwebrtc backend is null, we should not use it to get description from it.
Return null in that case.
Adding ASSERT to other calls where the layer above LibWebRTCMediaEndpoint should protect
from calling a function on a null libwebrtc backend.
* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::LibWebRTCMediaEndpoint::currentLocalDescription):
(WebCore::LibWebRTCMediaEndpoint::currentRemoteDescription):
(WebCore::LibWebRTCMediaEndpoint::pendingLocalDescription):
(WebCore::LibWebRTCMediaEndpoint::pendingRemoteDescription):
(WebCore::LibWebRTCMediaEndpoint::localDescription):
(WebCore::LibWebRTCMediaEndpoint::remoteDescription):
(WebCore::LibWebRTCMediaEndpoint::doSetLocalDescription):
(WebCore::LibWebRTCMediaEndpoint::doSetRemoteDescription):
(WebCore::LibWebRTCMediaEndpoint::addTrack):
(WebCore::LibWebRTCMediaEndpoint::removeTrack):
(WebCore::LibWebRTCMediaEndpoint::doCreateOffer):
(WebCore::LibWebRTCMediaEndpoint::doCreateAnswer):
(WebCore::LibWebRTCMediaEndpoint::createDataChannel):
Modified Paths
Added Paths
Diff
Added: trunk/LayoutTests/webrtc/calling-peerconnection-once-closed-expected.txt (0 => 219428)
--- trunk/LayoutTests/webrtc/calling-peerconnection-once-closed-expected.txt (rev 0)
+++ trunk/LayoutTests/webrtc/calling-peerconnection-once-closed-expected.txt 2017-07-12 22:31:45 UTC (rev 219428)
@@ -0,0 +1,10 @@
+
+PASS Ensuring closed connection getters do not crash
+PASS Ensuring closed connection createOffer does not crash
+PASS Ensuring closed connection createDataChannel does not crash
+PASS Ensuring closed connection addTransceiver does not crash
+PASS Ensuring closed connection addTrack does not crash
+PASS Ensuring closed connection setRemoteDescription does not crash
+PASS Ensuring closed connection setLocalDescription does not crash
+PASS Ensuring closed connection createAnswer does not crash
+
Added: trunk/LayoutTests/webrtc/calling-peerconnection-once-closed.html (0 => 219428)
--- trunk/LayoutTests/webrtc/calling-peerconnection-once-closed.html (rev 0)
+++ trunk/LayoutTests/webrtc/calling-peerconnection-once-closed.html 2017-07-12 22:31:45 UTC (rev 219428)
@@ -0,0 +1,96 @@
+<!doctype html>
+<html>
+ <head>
+ <meta charset="utf-8">
+ <title>Testing description getters after connection is closed</title>
+ <script src=""
+ <script src=""
+ <script src=""
+ </head>
+ <body>
+ <script>
+function closedConnection()
+{
+ return new Promise((resolve, reject) => {
+ var localConnection = new RTCPeerConnection();
+ localConnection.close();
+ setTimeout(() => resolve(localConnection), 100);
+ });
+}
+
+promise_test(() => {
+ return closedConnection().then((localConnection) => {
+ assert_equals(localConnection.currentLocalDescription, localConnection.localDescription);
+ assert_equals(localConnection.pendingLocalDescription, localConnection.localDescription);
+
+ assert_equals(localConnection.currentRemoteDescription, localConnection.remoteDescription);
+ assert_equals(localConnection.pendingRemoteDescription, localConnection.remoteDescription);
+ });
+}, "Ensuring closed connection getters do not crash");
+
+promise_test((test) => {
+ return closedConnection().then((connection) => {
+ return promise_rejects(test, 'InvalidStateError', connection.createOffer());
+ });
+}, "Ensuring closed connection createOffer does not crash");
+
+promise_test(() => {
+ return closedConnection().then((connection) => {
+ assert_throws("InvalidStateError", () => { connection.createDataChannel("test"); });
+ });
+}, "Ensuring closed connection createDataChannel does not crash");
+
+promise_test(() => {
+ return closedConnection().then((connection) => {
+ connection.addTransceiver("video");
+ });
+}, "Ensuring closed connection addTransceiver does not crash");
+
+promise_test(() => {
+ return closedConnection().then((connection) => {
+ return navigator.mediaDevices.getUserMedia({video: true}).then((stream) => {
+ assert_throws("InvalidStateError", () => { connection.addTrack(stream.getVideoTracks()[0], stream); });
+ });
+ });
+}, "Ensuring closed connection addTrack does not crash");
+
+promise_test((test) => {
+ var connection;
+ return closedConnection().then((pc) => {
+ connection = pc;
+ var pc2 = new RTCPeerConnection();
+ pc2.createDataChannel("test");
+ return pc2.createOffer();
+ }).then((sdp) => {
+ return promise_rejects(test, 'InvalidStateError', connection.setRemoteDescription(sdp));
+ });
+}, "Ensuring closed connection setRemoteDescription does not crash");
+
+promise_test((test) => {
+ var localConnection = new RTCPeerConnection();
+ return localConnection.createOffer().then((sdp) => {
+ localConnection.close();
+ return new Promise((resolve, reject) => {
+ setTimeout(() => resolve(sdp), 100);
+ });
+ }).then((sdp) => {
+ return promise_rejects(test, 'InvalidStateError', localConnection.setLocalDescription(sdp));
+ });
+}, "Ensuring closed connection setLocalDescription does not crash");
+
+promise_test((test) => {
+ var localConnection = new RTCPeerConnection();
+ var remoteConnection = new RTCPeerConnection();
+ remoteConnection.createDataChannel("test");
+ return remoteConnection.createOffer().then((sdp) => {
+ return localConnection.setRemoteDescription(sdp);
+ }).then(() => {
+ localConnection.close();
+ }).then(() => {
+ return promise_rejects(test, 'InvalidStateError', localConnection.createAnswer());
+ });
+}, "Ensuring closed connection createAnswer does not crash");
+
+ </script>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (219427 => 219428)
--- trunk/Source/WebCore/ChangeLog 2017-07-12 22:26:08 UTC (rev 219427)
+++ trunk/Source/WebCore/ChangeLog 2017-07-12 22:31:45 UTC (rev 219428)
@@ -1,3 +1,34 @@
+2017-07-12 Youenn Fablet <you...@apple.com>
+
+ Accessing localDescription, remoteDescription, etc. after setTimeout raises EXC_BAD_ACCESS
+ https://bugs.webkit.org/show_bug.cgi?id=174323
+ <rdar://problem/33267876>
+
+ Reviewed by Eric Carlson.
+
+ Test: webrtc/calling-peerconnection-once-closed.html
+
+ In case the libwebrtc backend is null, we should not use it to get description from it.
+ Return null in that case.
+
+ Adding ASSERT to other calls where the layer above LibWebRTCMediaEndpoint should protect
+ from calling a function on a null libwebrtc backend.
+
+ * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
+ (WebCore::LibWebRTCMediaEndpoint::currentLocalDescription):
+ (WebCore::LibWebRTCMediaEndpoint::currentRemoteDescription):
+ (WebCore::LibWebRTCMediaEndpoint::pendingLocalDescription):
+ (WebCore::LibWebRTCMediaEndpoint::pendingRemoteDescription):
+ (WebCore::LibWebRTCMediaEndpoint::localDescription):
+ (WebCore::LibWebRTCMediaEndpoint::remoteDescription):
+ (WebCore::LibWebRTCMediaEndpoint::doSetLocalDescription):
+ (WebCore::LibWebRTCMediaEndpoint::doSetRemoteDescription):
+ (WebCore::LibWebRTCMediaEndpoint::addTrack):
+ (WebCore::LibWebRTCMediaEndpoint::removeTrack):
+ (WebCore::LibWebRTCMediaEndpoint::doCreateOffer):
+ (WebCore::LibWebRTCMediaEndpoint::doCreateAnswer):
+ (WebCore::LibWebRTCMediaEndpoint::createDataChannel):
+
2017-07-12 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r219176.
Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp (219427 => 219428)
--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp 2017-07-12 22:26:08 UTC (rev 219427)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp 2017-07-12 22:31:45 UTC (rev 219428)
@@ -117,37 +117,38 @@
// FIXME: We might want to create a new object only if the session actually changed for all description getters.
RefPtr<RTCSessionDescription> LibWebRTCMediaEndpoint::currentLocalDescription() const
{
- return fromSessionDescription(m_backend->current_local_description());
+ return m_backend ? fromSessionDescription(m_backend->current_local_description()) : nullptr;
}
RefPtr<RTCSessionDescription> LibWebRTCMediaEndpoint::currentRemoteDescription() const
{
- return fromSessionDescription(m_backend->current_remote_description());
+ return m_backend ? fromSessionDescription(m_backend->current_remote_description()) : nullptr;
}
RefPtr<RTCSessionDescription> LibWebRTCMediaEndpoint::pendingLocalDescription() const
{
- return fromSessionDescription(m_backend->pending_local_description());
+ return m_backend ? fromSessionDescription(m_backend->pending_local_description()) : nullptr;
}
RefPtr<RTCSessionDescription> LibWebRTCMediaEndpoint::pendingRemoteDescription() const
{
- return fromSessionDescription(m_backend->pending_remote_description());
+ return m_backend ? fromSessionDescription(m_backend->pending_remote_description()) : nullptr;
}
RefPtr<RTCSessionDescription> LibWebRTCMediaEndpoint::localDescription() const
{
- return fromSessionDescription(m_backend->local_description());
+ return m_backend ? fromSessionDescription(m_backend->local_description()) : nullptr;
}
RefPtr<RTCSessionDescription> LibWebRTCMediaEndpoint::remoteDescription() const
{
- // FIXME: We might want to create a new object only if the session actually changed.
- return fromSessionDescription(m_backend->remote_description());
+ return m_backend ? fromSessionDescription(m_backend->remote_description()) : nullptr;
}
void LibWebRTCMediaEndpoint::doSetLocalDescription(RTCSessionDescription& description)
{
+ ASSERT(m_backend);
+
webrtc::SdpParseError error;
std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription(webrtc::CreateSessionDescription(sessionDescriptionType(description.type()), description.sdp().utf8().data(), &error));
@@ -168,6 +169,8 @@
void LibWebRTCMediaEndpoint::doSetRemoteDescription(RTCSessionDescription& description)
{
+ ASSERT(m_backend);
+
webrtc::SdpParseError error;
std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription(webrtc::CreateSessionDescription(sessionDescriptionType(description.type()), description.sdp().utf8().data(), &error));
if (!sessionDescription) {
@@ -182,6 +185,8 @@
void LibWebRTCMediaEndpoint::addTrack(RTCRtpSender& sender, MediaStreamTrack& track, const Vector<String>& mediaStreamIds)
{
+ ASSERT(m_backend);
+
std::vector<webrtc::MediaStreamInterface*> mediaStreams;
rtc::scoped_refptr<webrtc::MediaStreamInterface> mediaStream = nullptr;
if (mediaStreamIds.size()) {
@@ -212,6 +217,8 @@
void LibWebRTCMediaEndpoint::removeTrack(RTCRtpSender& sender)
{
+ ASSERT(m_backend);
+
auto rtcSender = m_senders.get(&sender);
if (!rtcSender)
return;
@@ -250,6 +257,8 @@
void LibWebRTCMediaEndpoint::doCreateOffer(const RTCOfferOptions& options)
{
+ ASSERT(m_backend);
+
m_isInitiator = true;
webrtc::PeerConnectionInterface::RTCOfferAnswerOptions rtcOptions;
rtcOptions.ice_restart = options.iceRestart;
@@ -264,6 +273,8 @@
void LibWebRTCMediaEndpoint::doCreateAnswer()
{
+ ASSERT(m_backend);
+
m_isInitiator = false;
m_backend->CreateAnswer(&m_createSessionDescriptionObserver, nullptr);
}
@@ -714,6 +725,8 @@
std::unique_ptr<RTCDataChannelHandler> LibWebRTCMediaEndpoint::createDataChannel(const String& label, const RTCDataChannelInit& options)
{
+ ASSERT(m_backend);
+
webrtc::DataChannelInit init;
if (options.ordered)
init.ordered = *options.ordered;
