Title: [132420] trunk
Revision
132420
Author
tom...@google.com
Date
2012-10-24 16:24:05 -0700 (Wed, 24 Oct 2012)

Log Message

Source/WebCore: MediaStream API: Make sure all events are dispatched asynchronously
https://bugs.webkit.org/show_bug.cgi?id=100286

Reviewed by Adam Barth.

This is necessary to safeguard against if the UA uses synchronous UA->WebKit calls,
and the web application calls methods on the RTCPeerConnection in the event callbacks.

Test: fast/mediastream/RTCPeerConnection-events.html
Also tested by the chromium webrtc fuzz tests.

* Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::RTCPeerConnection):
(WebCore::RTCPeerConnection::negotiationNeeded):
(WebCore::RTCPeerConnection::didGenerateIceCandidate):
(WebCore::RTCPeerConnection::didAddRemoteStream):
(WebCore::RTCPeerConnection::didRemoveRemoteStream):
(WebCore::RTCPeerConnection::didAddRemoteDataChannel):
(WebCore::RTCPeerConnection::changeReadyState):
(WebCore::RTCPeerConnection::changeIceState):
(WebCore):
(WebCore::RTCPeerConnection::scheduleDispatchEvent):
(WebCore::RTCPeerConnection::scheduledEventTimerFired):
* Modules/mediastream/RTCPeerConnection.h:
(RTCPeerConnection):

LayoutTests: MediaStream API: Make sure all RTCPeerConnection events are dispatched asynchronously
https://bugs.webkit.org/show_bug.cgi?id=100286

Reviewed by Adam Barth.

RTCPeerConnection::close is the only function that dispatches an event directly so only test that.

* fast/mediastream/RTCPeerConnection-events-expected.txt: Added.
* fast/mediastream/RTCPeerConnection-events.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (132419 => 132420)


--- trunk/LayoutTests/ChangeLog	2012-10-24 23:19:45 UTC (rev 132419)
+++ trunk/LayoutTests/ChangeLog	2012-10-24 23:24:05 UTC (rev 132420)
@@ -1,3 +1,15 @@
+2012-10-24  Tommy Widenflycht  <tom...@google.com>
+
+        MediaStream API: Make sure all RTCPeerConnection events are dispatched asynchronously
+        https://bugs.webkit.org/show_bug.cgi?id=100286
+
+        Reviewed by Adam Barth.
+
+        RTCPeerConnection::close is the only function that dispatches an event directly so only test that.
+
+        * fast/mediastream/RTCPeerConnection-events-expected.txt: Added.
+        * fast/mediastream/RTCPeerConnection-events.html: Added.
+
 2012-10-24  Levi Weintraub  <le...@chromium.org>
 
         Unreviewed gardening. Chromium revision 163873 also made

Added: trunk/LayoutTests/fast/mediastream/RTCPeerConnection-events-expected.txt (0 => 132420)


--- trunk/LayoutTests/fast/mediastream/RTCPeerConnection-events-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/mediastream/RTCPeerConnection-events-expected.txt	2012-10-24 23:24:05 UTC (rev 132420)
@@ -0,0 +1,15 @@
+Tests that RTCPeerConnection event callbacks are async so that for example close can be called safely. The order of the messages is very important.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS gotStream was called.
+PASS gotStream done.
+PASS onNegotiationNeeded was called.
+PASS onNegotiationNeeded done.
+PASS onStateChange was called.
+PASS pc.readyState is 'closed'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/mediastream/RTCPeerConnection-events.html (0 => 132420)


--- trunk/LayoutTests/fast/mediastream/RTCPeerConnection-events.html	                        (rev 0)
+++ trunk/LayoutTests/fast/mediastream/RTCPeerConnection-events.html	2012-10-24 23:24:05 UTC (rev 132420)
@@ -0,0 +1,60 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script>
+description("Tests that RTCPeerConnection event callbacks are async so that for example close can be called safely. The order of the messages is very important.");
+
+var stream = null;
+var pc = null;
+
+function error() {
+    testFailed('Stream generation failed.');
+    finishJSTest();
+}
+
+function getUserMedia(dictionary, callback) {
+    try {
+        navigator.webkitGetUserMedia(dictionary, callback, error);
+    } catch (e) {
+        testFailed('webkitGetUserMedia threw exception :' + e);
+        finishJSTest();
+    }
+}
+
+function onStateChange(event) {
+    testPassed('onStateChange was called.');
+    shouldBe("pc.readyState", "'closed'");
+    finishJSTest();
+}
+
+function onNegotiationNeeded(event) {
+    testPassed('onNegotiationNeeded was called.');
+    pc._onstatechange_ = onStateChange;
+    pc.close();
+    testPassed('onNegotiationNeeded done.')
+}
+
+function gotStream(s) {
+    testPassed('gotStream was called.');
+    stream = s;
+
+    pc = new webkitRTCPeerConnection(null, null);
+    pc._onnegotiationneeded_ = onNegotiationNeeded;
+
+    pc.addStream(stream);
+    testPassed('gotStream done.');
+}
+
+getUserMedia({audio:true, video:true}, gotStream);
+
+window.jsTestIsAsync = true;
+window.successfullyParsed = true;
+
+
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (132419 => 132420)


--- trunk/Source/WebCore/ChangeLog	2012-10-24 23:19:45 UTC (rev 132419)
+++ trunk/Source/WebCore/ChangeLog	2012-10-24 23:24:05 UTC (rev 132420)
@@ -1,3 +1,31 @@
+2012-10-24  Tommy Widenflycht  <tom...@google.com>
+
+        MediaStream API: Make sure all events are dispatched asynchronously
+        https://bugs.webkit.org/show_bug.cgi?id=100286
+
+        Reviewed by Adam Barth.
+
+        This is necessary to safeguard against if the UA uses synchronous UA->WebKit calls,
+        and the web application calls methods on the RTCPeerConnection in the event callbacks.
+
+        Test: fast/mediastream/RTCPeerConnection-events.html
+        Also tested by the chromium webrtc fuzz tests.
+
+        * Modules/mediastream/RTCPeerConnection.cpp:
+        (WebCore::RTCPeerConnection::RTCPeerConnection):
+        (WebCore::RTCPeerConnection::negotiationNeeded):
+        (WebCore::RTCPeerConnection::didGenerateIceCandidate):
+        (WebCore::RTCPeerConnection::didAddRemoteStream):
+        (WebCore::RTCPeerConnection::didRemoveRemoteStream):
+        (WebCore::RTCPeerConnection::didAddRemoteDataChannel):
+        (WebCore::RTCPeerConnection::changeReadyState):
+        (WebCore::RTCPeerConnection::changeIceState):
+        (WebCore):
+        (WebCore::RTCPeerConnection::scheduleDispatchEvent):
+        (WebCore::RTCPeerConnection::scheduledEventTimerFired):
+        * Modules/mediastream/RTCPeerConnection.h:
+        (RTCPeerConnection):
+
 2012-10-24  Mark Pilgrim  <pilg...@chromium.org>
 
         [Chromium] Remove screen-related functions from PlatformSupport

Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp (132419 => 132420)


--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp	2012-10-24 23:19:45 UTC (rev 132419)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp	2012-10-24 23:24:05 UTC (rev 132420)
@@ -134,6 +134,7 @@
     , m_iceState(IceStateClosed)
     , m_localStreams(MediaStreamList::create())
     , m_remoteStreams(MediaStreamList::create())
+    , m_scheduledEventTimer(this, &RTCPeerConnection::scheduledEventTimerFired)
 {
     ASSERT(m_scriptExecutionContext->isDocument());
     Document* document = static_cast<Document*>(m_scriptExecutionContext);
@@ -441,17 +442,17 @@
 
 void RTCPeerConnection::negotiationNeeded()
 {
-    dispatchEvent(Event::create(eventNames().negotiationneededEvent, false, false));
+    scheduleDispatchEvent(Event::create(eventNames().negotiationneededEvent, false, false));
 }
 
 void RTCPeerConnection::didGenerateIceCandidate(PassRefPtr<RTCIceCandidateDescriptor> iceCandidateDescriptor)
 {
     ASSERT(scriptExecutionContext()->isContextThread());
     if (!iceCandidateDescriptor)
-        dispatchEvent(RTCIceCandidateEvent::create(false, false, 0));
+        scheduleDispatchEvent(RTCIceCandidateEvent::create(false, false, 0));
     else {
         RefPtr<RTCIceCandidate> iceCandidate = RTCIceCandidate::create(iceCandidateDescriptor);
-        dispatchEvent(RTCIceCandidateEvent::create(false, false, iceCandidate.release()));
+        scheduleDispatchEvent(RTCIceCandidateEvent::create(false, false, iceCandidate.release()));
     }
 }
 
@@ -477,7 +478,7 @@
     RefPtr<MediaStream> stream = MediaStream::create(scriptExecutionContext(), streamDescriptor);
     m_remoteStreams->append(stream);
 
-    dispatchEvent(MediaStreamEvent::create(eventNames().addstreamEvent, false, false, stream.release()));
+    scheduleDispatchEvent(MediaStreamEvent::create(eventNames().addstreamEvent, false, false, stream.release()));
 }
 
 void RTCPeerConnection::didRemoveRemoteStream(MediaStreamDescriptor* streamDescriptor)
@@ -494,7 +495,7 @@
     ASSERT(m_remoteStreams->contains(stream.get()));
     m_remoteStreams->remove(stream.get());
 
-    dispatchEvent(MediaStreamEvent::create(eventNames().removestreamEvent, false, false, stream.release()));
+    scheduleDispatchEvent(MediaStreamEvent::create(eventNames().removestreamEvent, false, false, stream.release()));
 }
 
 void RTCPeerConnection::didAddRemoteDataChannel(PassRefPtr<RTCDataChannelDescriptor> channelDescriptor)
@@ -507,7 +508,7 @@
     RefPtr<RTCDataChannel> channel = RTCDataChannel::create(scriptExecutionContext(), m_peerHandler.get(), channelDescriptor);
     m_dataChannels.append(channel);
 
-    dispatchEvent(RTCDataChannelEvent::create(eventNames().datachannelEvent, false, false, channel.release()));
+    scheduleDispatchEvent(RTCDataChannelEvent::create(eventNames().datachannelEvent, false, false, channel.release()));
 }
 
 const AtomicString& RTCPeerConnection::interfaceName() const
@@ -556,7 +557,7 @@
     case ReadyStateOpening:
         break;
     case ReadyStateActive:
-        dispatchEvent(Event::create(eventNames().openEvent, false, false));
+        scheduleDispatchEvent(Event::create(eventNames().openEvent, false, false));
         break;
     case ReadyStateClosing:
     case ReadyStateClosed:
@@ -566,7 +567,7 @@
         break;
     }
 
-    dispatchEvent(Event::create(eventNames().statechangeEvent, false, false));
+    scheduleDispatchEvent(Event::create(eventNames().statechangeEvent, false, false));
 }
 
 void RTCPeerConnection::changeIceState(IceState iceState)
@@ -575,9 +576,29 @@
         return;
 
     m_iceState = iceState;
-    dispatchEvent(Event::create(eventNames().icechangeEvent, false, false));
+    scheduleDispatchEvent(Event::create(eventNames().icechangeEvent, false, false));
 }
 
+void RTCPeerConnection::scheduleDispatchEvent(PassRefPtr<Event> event)
+{
+    m_scheduledEvents.append(event);
+
+    if (!m_scheduledEventTimer.isActive())
+        m_scheduledEventTimer.startOneShot(0);
+}
+
+void RTCPeerConnection::scheduledEventTimerFired(Timer<RTCPeerConnection>*)
+{
+    Vector<RefPtr<Event> > events;
+    events.swap(m_scheduledEvents);
+
+    Vector<RefPtr<Event> >::iterator it = events.begin();
+    for (; it != events.end(); ++it)
+        dispatchEvent((*it).release());
+
+    events.clear();
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(MEDIA_STREAM)

Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h (132419 => 132420)


--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h	2012-10-24 23:19:45 UTC (rev 132419)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h	2012-10-24 23:24:05 UTC (rev 132420)
@@ -42,6 +42,7 @@
 #include "RTCIceCandidate.h"
 #include "RTCPeerConnectionHandler.h"
 #include "RTCPeerConnectionHandlerClient.h"
+#include "Timer.h"
 #include <wtf/RefCounted.h>
 
 namespace WebCore {
@@ -125,6 +126,8 @@
     RTCPeerConnection(ScriptExecutionContext*, PassRefPtr<RTCConfiguration>, PassRefPtr<MediaConstraints>, ExceptionCode&);
 
     static PassRefPtr<RTCConfiguration> parseConfiguration(const Dictionary& configuration, ExceptionCode&);
+    void scheduleDispatchEvent(PassRefPtr<Event>);
+    void scheduledEventTimerFired(Timer<RTCPeerConnection>*);
 
     // EventTarget implementation.
     virtual EventTargetData* eventTargetData();
@@ -145,6 +148,9 @@
     Vector<RefPtr<RTCDataChannel> > m_dataChannels;
 
     OwnPtr<RTCPeerConnectionHandler> m_peerHandler;
+
+    Timer<RTCPeerConnection> m_scheduledEventTimer;
+    Vector<RefPtr<Event> > m_scheduledEvents;
 };
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to