WebRTC tests relying on timers running fast in background tabs |
|||
Issue descriptionAfter enabling timer throttling for background renderers (https://chromium-review.googlesource.com/c/567932/), gathering of ICE candidates started failing flakingly with this error log: [7466:7466:0713/070330.078566:INFO:CONSOLE(13)] "Returning Test failed: Error: Received invalid ICE candidate list from peer: [] at failTest (http://127.0.0.1:52364/webrtc/test_functions.js:46:15) at receiveIceCandidates (http://127.0.0.1:52364/webrtc/peerconnection.js:371:11) at <anonymous>:1:51 to test.", source: http://127.0.0.1:52364/webrtc/test_functions.js (13) ../../chrome/browser/media/webrtc/webrtc_browsertest_base.cc:412: Failure Expected: "ok-received-candidates" To be equal to: ExecuteJavascript( base::StringPrintf("receiveIceCandidates('%s')", ice_candidates.c_str()), to_tab) Which is: "Test failed: Error: Received invalid ICE candidate list from peer: []\n at failTest (http://127.0.0.1:52364/webrtc/test_functions.js:46:15)\n at receiveIceCandidates (http://127.0.0.1:52364/webrtc/peerconnection.js:371:11)\n at <anonymous>:1:51" With diff: @@ -1,1 +1,4 @@ -ok-received-candidates +Test failed: Error: Received invalid ICE candidate list from peer: [] + at failTest (http://127.0.0.1:52364/webrtc/test_functions.js:46:15) + at receiveIceCandidates (http://127.0.0.1:52364/webrtc/peerconnection.js:371:11) + at <anonymous>:1:51 Link to bots: https://build.chromium.org/p/chromium.webrtc/builders/Linux%20Tester/builds/30651 https://build.chromium.org/p/chromium.webrtc/builders/Mac%20Tester/builds/69890 https://build.chromium.org/p/chromium.webrtc/builders/Win7%20Tester/builds/36437 The ICE candidate gathering probably fails from a timeout. The WebRTC tests are currently blocking https://chromium-review.googlesource.com/c/567932/. We should update the WebRTC tests so we can land this desired behavior.
,
Jul 18 2017
I believe I understand the issue. The root cause is that "RTCPeerConnection.iceGatheringState" isn't changed atomically in the same task in which "icegatheringstatechange" is fired. So the application may see "the gathering state is complete" before it's seen the "icegatheringstatechange" or even "icecandidate" events. The "background timer throttling" revealed this issue because RTCPeerConnection uses an AsyncMethodRunner to schedule the "icecandidate" events, which internally uses Timer, which uses TimerTaskRunner. So, normally the events will be dispatched effectively immediately, and this happens: 1. "gathering" state change seen from webrtc; cached state set to "gathering", event queued. 2. Event fired. Application reads "gathering". 3. "complete" state change seen from webrtc; cached state set to "complete", event queued. 4. Event fired. Application reads "complete". But with the throttling, this happens: 1. "gathering" state change seen from webrtc; cached state set to "gathering", event queued. 2. "complete" state change seen from webrtc; cached state set to "complete", event queued. 3. Event fired. Application reads "complete". 4. Event fired. Application reads "complete" again. Should be simple to fix. It looks like this issue already was noticed and fixed for iceConnectionState, just not for iceGatheringState...
,
Jul 18 2017
,
Jul 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/129ff07e68b7505ce6ccdd5511cead4ae83d5d3c commit 129ff07e68b7505ce6ccdd5511cead4ae83d5d3c Author: Taylor Brandstetter <deadbeef@chromium.org> Date: Wed Jul 19 23:30:01 2017 Update gathering state at the same time as the event is fired. This CL ensures that it won't be possible for "RTCPeerConnection.iceGatheringState" to return a new value until the "icegatheringstatechange" event has been dispatched. This is something the WebRTC specification guarantees. This was previously fixed for iceConnectionState: https://codereview.chromium.org/1523213002 This CL extends that logic to iceGatheringState. This CL also ensures that the special "null" candidate (used as another indication that gathering is complete) is emitted at the proper time. To do this, the responsibility for creating this special candidate is moved from the content layer to the Blink layer. Lastly, this CL updates the "setConfiguration" test, resolving a TODO. Bug: 742859 Change-Id: I55fb605913314ff5e780fa3ca30112180bdc7f37 Reviewed-on: https://chromium-review.googlesource.com/575849 Commit-Queue: Taylor Brandstetter <deadbeef@chromium.org> Reviewed-by: Tommi <tommi@chromium.org> Cr-Commit-Position: refs/heads/master@{#488038} [modify] https://crrev.com/129ff07e68b7505ce6ccdd5511cead4ae83d5d3c/content/renderer/media/mock_web_rtc_peer_connection_handler_client.cc [modify] https://crrev.com/129ff07e68b7505ce6ccdd5511cead4ae83d5d3c/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/129ff07e68b7505ce6ccdd5511cead4ae83d5d3c/content/test/data/media/peerconnection-setConfiguration.html [modify] https://crrev.com/129ff07e68b7505ce6ccdd5511cead4ae83d5d3c/content/test/data/media/webrtc_test_common.js [modify] https://crrev.com/129ff07e68b7505ce6ccdd5511cead4ae83d5d3c/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp [modify] https://crrev.com/129ff07e68b7505ce6ccdd5511cead4ae83d5d3c/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.h
,
Jul 19 2017
Alright altimin@, I believe that should fix the WebRTC issue.
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1ff0bb6199125c14dcf548f7f4b4ef597e73de50 commit 1ff0bb6199125c14dcf548f7f4b4ef597e73de50 Author: Alexander Timin <altimin@chromium.org> Date: Thu Aug 31 13:24:47 2017 [blink] Use TaskRunnerTimer in media element event queues. MediaElements used to be the only user of GenericEventQueue which posted tasks to renderer-global default timer queue. Rename GenericEventQueue to MediaElementEventQueue and make this event queue post tasks with kMediaElementEvent type to a relevant per-frame queue. R=haraken@chromium.org BUG= 742859 Change-Id: I0de9721a92b4848b88791d09c4ac04af2aeb95ab Reviewed-on: https://chromium-review.googlesource.com/574713 Commit-Queue: Alexander Timin <altimin@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Fredrik Söderquist <fs@opera.com> Cr-Commit-Position: refs/heads/master@{#498843} [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/core/dom/BUILD.gn [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/core/dom/TaskRunnerHelper.cpp [rename] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/core/dom/events/MediaElementEventQueue.cpp [rename] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/core/dom/events/MediaElementEventQueue.h [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/core/html/HTMLMediaElement.h [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/core/html/track/TextTrackList.cpp [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/core/html/track/TextTrackList.h [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.h [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/modules/mediasource/MediaSource.cpp [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/modules/mediasource/MediaSource.h [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/modules/mediasource/SourceBuffer.h [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/modules/mediasource/SourceBufferList.cpp [modify] https://crrev.com/1ff0bb6199125c14dcf548f7f4b4ef597e73de50/third_party/WebKit/Source/modules/mediasource/SourceBufferList.h |
|||
►
Sign in to add a comment |
|||
Comment 1 by altimin@chromium.org
, Jul 14 2017Status: Started (was: Assigned)