RTCPeerConnection, MediaStream and MediaStreamTrack events should fire in the right order |
||||||||
Issue descriptionRTCPeerConnection, MediaStream and MediaStreamTrack have similarly implemented DispatchScheduleEvent functions that puts the event on a queue to be fired asynchronously using a TaskRunnerTimer. This makes them execute *after* any promises resolved in the same event loop causing problems, e.g. https://crbug.com/777777 . If we use microtasks instead, this makes them execute *before* any such promises. That would solve any WebRTC-related bug that I can think of... but then you would have a problem if you ever wanted them to fire after a related promise has resolved. Maybe not an issue? Possible solutions: 1. Use microtasks, always fire before pending promises. 2. Use Promise.resolve() or similar to ensure events are placed in the same queue as promises are placed. 3. Get rid of the idea of general "schedule to fire event" calls, instead fire events on a case-by-case basis when they should fire. (For example, setRemoteDescription[1] describes a trackEvents queue as part of the steps of executing that function, ending with a "for each trackEvent in trackEvents, fire trackEvent...".) This gives a very well-defined order with less surprises. [1] https://rawgit.com/w3c/webrtc-pc/master/webrtc.html#set-the-rtcsessiondescription
,
Oct 24 2017
...before we decide on what type of queue to use for the event.
,
Oct 27 2017
,
Oct 30 2017
,
Nov 5 2017
,
Nov 13 2017
FYI this comment was referenced from https://crbug.com/774303#c14 with regards to test coverage.
,
Nov 21 2017
,
Nov 27 2017
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6 commit 5366645016a98a9c9c69d5b729bb5d0d1d7da6c6 Author: Henrik Boström <hbos@chromium.org> Date: Wed Nov 29 12:48:47 2017 setRemoteDescripton: Unify SRD and events, fire in the correct order. This includes RTCPeerConnection.onaddtrack, MediaStream.onaddtrack/ onremovetrack and MediaStreamTrack.onmute. Instead of updating which tracks belong to which streams in an observer that listens to OnChanged in webrtc and PostTasks to the main thread once per stream, all stream states are updated in the SRD callback along with all the other state changes. Spec: https://w3c.github.io/webrtc-pc/#processing-remote-mediastreamtracks With this CL, https://crbug.com/777999 and https://crbug.com/webrtc/8473 are fixed. All events are currently scheduled to fire instead of firing synchronously (https://crbug.com/788558), so to make the promise resolve after all the events have fired we PostTask to resolve the promise. Bug: chromium:777999 , webrtc:8473 , chromium:788558 Change-Id: I21cce27b4e541ae6a53d6a9f91c73d181f3dd88e Reviewed-on: https://chromium-review.googlesource.com/789843 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#520084} [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/webrtc/webrtc_media_stream_adapter.cc [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/webrtc/webrtc_media_stream_adapter.h [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.h [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https-expected.txt [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https.html [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-ontrack.html
,
Nov 29 2017
,
Dec 11 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by hbos@chromium.org
, Oct 24 2017