Issue metadata
Sign in to add a comment
|
webrtc: setRemoteDescriptionOnFailure doesn't show up webrtc-internals if SDP parsing fails
Reported by
fi...@appear.in,
Dec 29 2016
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/53.0.2785.143 Chrome/53.0.2785.143 Safari/537.36 Steps to reproduce the problem: 1. join appr.tc room with Firefox 2. join appr.tc room with Chrome Canary 57.0.2966.0 3. call gets established What is the expected behavior? due to https://bugzilla.mozilla.org/show_bug.cgi?id=1326288 the remote offer SDP is rejected with an operations error. This should show up in chrome://webrtc-internals What went wrong? Neither the setRemoteDescription call nor the failure showed up Did this work before? Yes Does this work in other browsers? N/A Chrome version: 53.0.2785.143 Channel: n/a OS Version: Flash Version: Shockwave Flash 16.0 r0
,
Dec 30 2016
,
Jan 3 2017
,
Jan 4 2017
,
Jan 4 2017
some failures like https://jsfiddle.net/vy74r1yt/2/ still show up. Repro fiddle for this failure: https://jsfiddle.net/0cmbwea3/1/ The fiddle might work again in a canary that does has https://bugs.chromium.org/p/webrtc/issues/detail?id=4674#c15 but I think itis still worth investigating why this didn't show up.
,
Jan 4 2017
It sounds like, more generally, the issue is that SDP parse failures aren't tracked as "setRemoteDescription" failures. Notice that the code block for "if SDP parsing failed" doesn't do anything with peer_connection_tracker_: https://cs.chromium.org/chromium/src/content/renderer/media/rtc_peer_connection_handler.cc?sq=package:chromium&rcl=1483532378&l=1307 I don't know if this is intentional or not.
,
Jan 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/36722c63d70844654fc8d04a2aca71729c7365e5 commit 36722c63d70844654fc8d04a2aca71729c7365e5 Author: philipp.hancke <philipp.hancke@googlemail.com> Date: Mon Jan 09 10:40:30 2017 track SDP parse errors SDP parse errors were not tracked in chrome://webrtc-internals BUG= 677550 Review-Url: https://codereview.chromium.org/2622453002 Cr-Commit-Position: refs/heads/master@{#442228} [modify] https://crrev.com/36722c63d70844654fc8d04a2aca71729c7365e5/content/renderer/media/rtc_peer_connection_handler.cc
,
Jan 9 2017
Micro-test case:
var pc = new webkitRTCPeerConnection(null);
pc.setRemoteDescription(new RTCSessionDescription({type: 'offer', sdp: 'blob'}))
should show up both with the SRD call and the failure.
,
Jan 9 2017
similarly, when there is an error with addIceCandidate, both addIceCandidate and addIceCandidateOnFailure should show up (and I think I am to blame for this not being true :-))
,
Jan 9 2017
Technically we should also do the same thing for local descriptions, and it would be nice to have unit tests. I'll take care of that.
,
Jan 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/536b626025606ce4ecca3f2595d384cd4b947ebe commit 536b626025606ce4ecca3f2595d384cd4b947ebe Author: deadbeef <deadbeef@chromium.org> Date: Wed Jan 11 01:47:36 2017 Track SDP parse errors for SLD in chrome://webrtc-internals Just the SLD-equivalent of: https://codereview.chromium.org/2622453002 Also adds tests. BUG= chromium:677550 Review-Url: https://codereview.chromium.org/2619343003 Cr-Commit-Position: refs/heads/master@{#442766} [modify] https://crrev.com/536b626025606ce4ecca3f2595d384cd4b947ebe/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/536b626025606ce4ecca3f2595d384cd4b947ebe/content/renderer/media/rtc_peer_connection_handler_unittest.cc [modify] https://crrev.com/536b626025606ce4ecca3f2595d384cd4b947ebe/content/renderer/media/webrtc/mock_peer_connection_dependency_factory.cc [modify] https://crrev.com/536b626025606ce4ecca3f2595d384cd4b947ebe/content/renderer/media/webrtc/mock_peer_connection_dependency_factory.h [modify] https://crrev.com/536b626025606ce4ecca3f2595d384cd4b947ebe/third_party/WebKit/Source/platform/exported/WebRTCVoidRequest.cpp
,
Jan 11 2017
Just tested, looks fixed.
,
Jan 12 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by fi...@appear.in
, Dec 29 2016