Title: [213982] trunk
Revision
213982
Author
commit-qu...@webkit.org
Date
2017-03-15 09:35:18 -0700 (Wed, 15 Mar 2017)

Log Message

Preventive clean-up: ensure RTCPeerConnection stays valid when calling postTask
https://bugs.webkit.org/show_bug.cgi?id=169661

Patch by Youenn Fablet <you...@apple.com> on 2017-03-15
Reviewed by Alex Christensen.

Source/WebCore:

Protecting the RTCPeerConnection object when calling postTask since it might get collected between the task post
and task run. Also do not send negotiationNeeded event if RTCPeerConnection is closed (covered by added test).

* Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::updateIceGatheringState):
(WebCore::RTCPeerConnection::updateIceConnectionState):
(WebCore::RTCPeerConnection::scheduleNegotiationNeededEvent):

LayoutTests:

* webrtc/negotiatedneeded-event-addStream-expected.txt:
* webrtc/negotiatedneeded-event-addStream.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (213981 => 213982)


--- trunk/LayoutTests/ChangeLog	2017-03-15 15:58:18 UTC (rev 213981)
+++ trunk/LayoutTests/ChangeLog	2017-03-15 16:35:18 UTC (rev 213982)
@@ -1,3 +1,13 @@
+2017-03-15  Youenn Fablet  <you...@apple.com>
+
+        Preventive clean-up: ensure RTCPeerConnection stays valid when calling postTask
+        https://bugs.webkit.org/show_bug.cgi?id=169661
+
+        Reviewed by Alex Christensen.
+
+        * webrtc/negotiatedneeded-event-addStream-expected.txt:
+        * webrtc/negotiatedneeded-event-addStream.html:
+
 2017-03-14  Ryan Haddad  <ryanhad...@apple.com>
 
         Mark media/modern-media-controls/volume-down-support/volume-down-support.html as flaky.

Modified: trunk/LayoutTests/webrtc/negotiatedneeded-event-addStream-expected.txt (213981 => 213982)


--- trunk/LayoutTests/webrtc/negotiatedneeded-event-addStream-expected.txt	2017-03-15 15:58:18 UTC (rev 213981)
+++ trunk/LayoutTests/webrtc/negotiatedneeded-event-addStream-expected.txt	2017-03-15 16:35:18 UTC (rev 213982)
@@ -1,4 +1,5 @@
 
 
 PASS on negotiation needed in case of adding a stream 
+PASS on negotiation needed not called if pc is closed 
 

Modified: trunk/LayoutTests/webrtc/negotiatedneeded-event-addStream.html (213981 => 213982)


--- trunk/LayoutTests/webrtc/negotiatedneeded-event-addStream.html	2017-03-15 15:58:18 UTC (rev 213981)
+++ trunk/LayoutTests/webrtc/negotiatedneeded-event-addStream.html	2017-03-15 16:35:18 UTC (rev 213982)
@@ -26,6 +26,23 @@
         });
     });
 }, "on negotiation needed in case of adding a stream");
+
+promise_test((test) => {
+    if (window.testRunner)
+        testRunner.setUserMediaPermission(true);
+
+    return navigator.mediaDevices.getUserMedia({ video: true}).then((stream) => {
+        return new Promise((resolve, reject) => {
+            if (window.internals)
+                internals.useMockRTCPeerConnectionFactory("OneRealPeerConnection");
+            var pc = new RTCPeerConnection();
+            pc._onnegotiationneeded_ = () => { reject(); };
+            pc.addStream(stream);
+            pc.close();
+            setTimeout(resolve, 500);
+        });
+    });
+}, "on negotiation needed not called if pc is closed");
         </script>
     </body>
 </html>

Modified: trunk/Source/WebCore/ChangeLog (213981 => 213982)


--- trunk/Source/WebCore/ChangeLog	2017-03-15 15:58:18 UTC (rev 213981)
+++ trunk/Source/WebCore/ChangeLog	2017-03-15 16:35:18 UTC (rev 213982)
@@ -1,3 +1,18 @@
+2017-03-15  Youenn Fablet  <you...@apple.com>
+
+        Preventive clean-up: ensure RTCPeerConnection stays valid when calling postTask
+        https://bugs.webkit.org/show_bug.cgi?id=169661
+
+        Reviewed by Alex Christensen.
+
+        Protecting the RTCPeerConnection object when calling postTask since it might get collected between the task post
+        and task run. Also do not send negotiationNeeded event if RTCPeerConnection is closed (covered by added test).
+
+        * Modules/mediastream/RTCPeerConnection.cpp:
+        (WebCore::RTCPeerConnection::updateIceGatheringState):
+        (WebCore::RTCPeerConnection::updateIceConnectionState):
+        (WebCore::RTCPeerConnection::scheduleNegotiationNeededEvent):
+
 2017-03-15  Antoine Quint  <grao...@apple.com>
 
         [Modern Media Controls] Always use six digits to display time when overall media duration is an hour or more

Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp (213981 => 213982)


--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp	2017-03-15 15:58:18 UTC (rev 213981)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp	2017-03-15 16:35:18 UTC (rev 213982)
@@ -477,33 +477,35 @@
 
 void RTCPeerConnection::updateIceGatheringState(IceGatheringState newState)
 {
-    scriptExecutionContext()->postTask([=](ScriptExecutionContext&) {
-        if (m_signalingState == SignalingState::Closed || m_iceGatheringState == newState)
+    scriptExecutionContext()->postTask([protectedThis = makeRef(*this), newState](ScriptExecutionContext&) {
+        if (protectedThis->m_signalingState == SignalingState::Closed || protectedThis->m_iceGatheringState == newState)
             return;
 
-        m_iceGatheringState = newState;
-        dispatchEvent(Event::create(eventNames().icegatheringstatechangeEvent, false, false));
+        protectedThis->m_iceGatheringState = newState;
+        protectedThis->dispatchEvent(Event::create(eventNames().icegatheringstatechangeEvent, false, false));
     });
 }
 
 void RTCPeerConnection::updateIceConnectionState(IceConnectionState newState)
 {
-    scriptExecutionContext()->postTask([=](ScriptExecutionContext&) {
-        if (m_signalingState == SignalingState::Closed || m_iceConnectionState == newState)
+    scriptExecutionContext()->postTask([protectedThis = makeRef(*this), newState](ScriptExecutionContext&) {
+        if (protectedThis->m_signalingState == SignalingState::Closed || protectedThis->m_iceConnectionState == newState)
             return;
 
-        m_iceConnectionState = newState;
-        dispatchEvent(Event::create(eventNames().iceconnectionstatechangeEvent, false, false));
+        protectedThis->m_iceConnectionState = newState;
+        protectedThis->dispatchEvent(Event::create(eventNames().iceconnectionstatechangeEvent, false, false));
     });
 }
 
 void RTCPeerConnection::scheduleNegotiationNeededEvent()
 {
-    scriptExecutionContext()->postTask([=](ScriptExecutionContext&) {
-        if (m_backend->isNegotiationNeeded()) {
-            m_backend->clearNegotiationNeededState();
-            dispatchEvent(Event::create(eventNames().negotiationneededEvent, false, false));
-        }
+    scriptExecutionContext()->postTask([protectedThis = makeRef(*this)](ScriptExecutionContext&) {
+        if (protectedThis->m_signalingState == SignalingState::Closed)
+            return;
+        if (!protectedThis->m_backend->isNegotiationNeeded())
+            return;
+        protectedThis->m_backend->clearNegotiationNeededState();
+        protectedThis->dispatchEvent(Event::create(eventNames().negotiationneededEvent, false, false));
     });
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to