ICE candidate gathering hangs after reloading page |
||||||||||||||||
Issue descriptionVersion: 60.0.3100.0 / Dev OS: Windows 10, MacOS 10.12.4 Re-produce: 1. Open a new browser tab 2. Browse to https://appr.tc/?debug=loopback&audio=true&video=false and press “JOIN” 3. Open the javascript console (alt+cmd+j or ctr+shift+j) to watch for any errors 4. Speak over mic (or scratch the mc) and Listen for audio disruptions 5, Press the *hang-up” icon 6. Reload the page and press “JOIN” Expected: Step 4 & 6 : — Sound is heard — No JavaScript console errors Actual: Step 6: — No Sound is heard Other Information: I do NOT get this bug on MacOS: 10.12.4, Windows 10 using on both Chrome Stable (57.0.2987.133). I’ve observed the non gathering of ice candidates with other WebRTC related things for example in the bug: https://github.com/webrtc/samples/issues/903 plus other werbrtc pages. The common symptom, in all cases, is the non ice gathering and chrome://webrtc-internals showing iceconnectionstateChange: checking' In the following document you can find - the JavaScript logs for step 4 (where the loopback call works) - the Javascript logs for step 6 (when the loopback call fails due non gathering of ice candidates) - a screenshot of webrtc-internals for step 6 (when the loopback call fails due non gathering of ice candidates https://drive.google.com/drive/folders/0BzevCB72sdnCTzlSd0dLeFVZT3c?usp=sharing I will do a bisect and add any relevant information to this bug.
,
May 22 2017
magjed@ can you help with triage?
,
May 22 2017
I can investigate from here. This is an issue that (as you mention) has popped up in various places, but this is the most reliable method I've seen of reproducing it. So hopefully I can get to the bottom of it this week.
,
May 23 2017
I narrowed down the issue to MediaPermission::HasPermission being called, but no callback being called in return. This affects the FilteringNetworkManager class which is used to filter ICE candidates based on media permissions. This CL appeared to introduce the issue: https://codereview.chromium.org/2842013002/ Specifically, it looks like CloseBindings in DidFinishNavigation is an issue, I assume because DidFinishNavigation isn't a case where the renderer side of the pipe is closed after all. reillyg@, what do you think? Do you know what to do about this?
,
May 23 2017
I need to dig into this further because I am not familiar with MediaPermissionDispatcher but my theory is that MediaPermissionDispatcher instances outlive navigations and so we need to put in logic to reconnect to the Permissions service when the connection is closed due to a navigation. The other option is that DidFinishNavigation is not a case where we should be closing the pipe. This may be true because (I believe) navigations do not change the origin that renderer represents and so we do not need to close connections for security isolation purposes.
,
May 23 2017
It looks like the MediaPermissionDispatcher is owned by the RenderFrameImpl (created in RenderFrameImpl::GetMediaPermission()) and so it will survive navigation as long as the RenderFrameImpl is reused. If we add a connection error handler that resets permission_service_ then that should fix this issue because MediaPermissionDispatcher will automatically reconnect. There's a possibility of a race because early permission requests may be issued before the connection close signal is received and so the request will be caught up on the old connection. To avoid this issue we may want to explicitly close the old connection on navigation in the renderer.
,
May 23 2017
This issue is also observed on Chrome OS with M60 60.0.3105.0 / 9578.0.0 dev
,
May 24 2017
I have a patch which fixes this issue by handling the connection close on the renderer side. https://chromium-review.googlesource.com/513387
,
May 24 2017
,
May 24 2017
I get the same behaviour from a data-channel-only Peer-connection (no audio or video and no permissions needed) . Reloading the page results in zero local candidates - closing the window or tab and then loading the page again works. This is version 60.0.3109.0 (Official Build) canary (64-bit) on Macos 10.12.5 It does not happen on version 60.0.3096.4 on android.
,
May 24 2017
tim: a datachannel-only thing might nonetheless check permissions to figure out if it is allowed to use the full set of local candidates.
,
May 24 2017
,
May 24 2017
Issue 726116 has been merged into this issue.
,
May 26 2017
Same probl. here with chrome 60 - macOS. ICE works only for the first Webrtc session of the browser life.
,
May 30 2017
,
May 30 2017
Issue 727489 has been merged into this issue.
,
Jun 1 2017
Any updates on this?
,
Jun 1 2017
Waiting for a review from nick@ so I can land the fix.
,
Jun 6 2017
There's no updates on the CL for a week... Can we get it landed asap since the fix needs to be merged back to M60?
,
Jun 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/355be367c161e11c42991b47f13ba4a915c9e283 commit 355be367c161e11c42991b47f13ba4a915c9e283 Author: Reilly Grant <reillyg@chromium.org> Date: Tue Jun 06 19:15:50 2017 Recreate MediaPermissionDispatcher's service connection on navigation In r470992 we started closing PermissionService connections on navigation however the lifetime of a MediaPermissionDispatcher is that of the frame it is attached to. Since frames can be navigated the service connection must be re-established after it is closed. To prevent a race between the browser-side close and the navigation committing in the renderer we close the connection from that side as well. Bug: 725038 Change-Id: Id5d9d03537c7b7ee0997d77da4bafed055686521 Reviewed-on: https://chromium-review.googlesource.com/513387 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Nick Carter <nick@chromium.org> Commit-Queue: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#477363} [modify] https://crrev.com/355be367c161e11c42991b47f13ba4a915c9e283/content/renderer/media/media_permission_dispatcher.cc [modify] https://crrev.com/355be367c161e11c42991b47f13ba4a915c9e283/content/renderer/media/media_permission_dispatcher.h [modify] https://crrev.com/355be367c161e11c42991b47f13ba4a915c9e283/content/renderer/render_frame_impl.cc
,
Jun 6 2017
I will request a merge to branch 3112 once this change has baked in a Canary release.
,
Jun 7 2017
Fix works in Canary (61.0.3123.0) on Win10, Linux & Mac
,
Jun 7 2017
confirmed fix seems ok in data channel only case on canary 61.0.3123.0 on mac. Thanks.
,
Jun 7 2017
Thank you for verifying.
,
Jun 7 2017
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4d2aa441f353f892cda7029b5bbd908777536010 commit 4d2aa441f353f892cda7029b5bbd908777536010 Author: Reilly Grant <reillyg@chromium.org> Date: Wed Jun 07 19:58:15 2017 Recreate MediaPermissionDispatcher's service connection on navigation In r470992 we started closing PermissionService connections on navigation however the lifetime of a MediaPermissionDispatcher is that of the frame it is attached to. Since frames can be navigated the service connection must be re-established after it is closed. To prevent a race between the browser-side close and the navigation committing in the renderer we close the connection from that side as well. Bug: 725038 Change-Id: Id5d9d03537c7b7ee0997d77da4bafed055686521 Reviewed-on: https://chromium-review.googlesource.com/513387 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Nick Carter <nick@chromium.org> Commit-Queue: Reilly Grant <reillyg@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#477363} Review-Url: https://codereview.chromium.org/2931513003 . Cr-Commit-Position: refs/branch-heads/3112@{#231} Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897} [modify] https://crrev.com/4d2aa441f353f892cda7029b5bbd908777536010/content/renderer/media/media_permission_dispatcher.cc [modify] https://crrev.com/4d2aa441f353f892cda7029b5bbd908777536010/content/renderer/media/media_permission_dispatcher.h [modify] https://crrev.com/4d2aa441f353f892cda7029b5bbd908777536010/content/renderer/render_frame_impl.cc
,
Jun 8 2017
Tested the issue on Windows-10, Ubuntu 14.04 and Mac OS 10.12.5 using chrome latest Beta M60-60.0.3112.24 by following steps mentioned in the original comment. Observed that Sound is heard and No JavaScript console errors and working as expected. Hence adding TE-Verified label. Please find the screen cast for reference. Thank you!
,
Jun 12 2017
Issue webrtc:7770 has been merged into this issue.
,
Jun 15 2017
Issue 727393 has been merged into this issue.
,
Jul 27 2017
Verified this issue on Chrome OS with M60 60.0.3112.80 / 9592.71.0 beta using the steps mentioned in the initial bug report. Audio is heard after leaving the current call and rejoining the same call/joining a new call. |
||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||
Comment 1 by jkallar@chromium.org
, May 22 2017