New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 805398 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Diagnostic packet and event recording is always empty for Chromebooks [Logs]

Project Member Reported by vasanthakumar@chromium.org, Jan 24 2018

Issue description

Chrome Version: 65.0.3325.9 / 10323.1.0 Reks, Pyro, Lars
OS: chrome OS

[Not reproducible in Mac OS with the same chrome version]

What steps will reproduce the problem?

1. Goto chrome://webrtc-internals
2. Click "Enable diagnostic packet and event recording" and provide a file name
3. Now we enable the tracing related to the upcoming Apprtc call
4. Make an Apprtc call (I tried peer to peer)
5. After sometime uncheck the check box shown in the step 2 above. (to stop the logs)

Recording is not happening even when we start during the ongoing call. 

What is the expected result?
Logs should be captured with the file name provided in step 2.

What do you see instead?
Log files are empty which is wrong.  


 
Description: Show this description
Screenshot 2018-01-24 at 11.42.45 AM (1).png
136 KB View Download
Labels: -Pri-1 Pri-2
Owner: eladalon@chromium.org
Status: Assigned (was: Untriaged)
Elad, can you take a look at this?
Cc: terelius@chromium.org felixe@chromium.org
The bug is actually reproducible in another form, which I think is more telling of its cause - one can simply turn logging off and on. Immediately upon turning logging on, without any active peer connections being present, five logs are created. That a log with the events from the active calls is not created is just a corollary (there is a cap of five active logs at a time, so once those log files are created, no further log files can be created, including one for the real, active peer connection). It bears mentioning that rebooting the device "solves" the problem.

I need to look more into this, but it seems to me that there are a few likely suspected causes:
* PeerConnections not being properly garbage collected.
* WebRtcEventLogManager not being informed when PeerConnections are closed.
* Something other than Chrome using PeerConnections on ChromeOS. I am not familiar enough with ChromeOS's architecture. Perhaps they could be using the same WebRtcEventLogManager, but not be visible on the WebRTCInternals page? If so, would the recently landed CL, which moved the ownership of WebRtcEventLogManager into MainLoop, perhaps fix this?
I've confirmed that, when a RendererProcess crashes, WebRtcEventLogManager is not made aware of the fact. This prevents it from forgetting the PeerConnections associated with that RendererProcess. I don't know if this is the bug we're seeing here or not, so I'll file a separate bug for that, and keep investigating here.
Some updates:
1. In this bug's true form, it is reproducible on Mac and Linux. Spoiler for the next points - the difference between ChromeOS the other platforms was probably in the pattern of garbage colleciton.
2. That some logs were inhibited is a side-effect of the real bug. The real bug is that WebRtcEventLogManager did not get notified of all peer connection removals. There are several cases in which this happens:
2.1. Pre-existing  bug #780855  - peer connection removal is indicated to PeerConnectionTracker only when garbage collection of the PC occurs, not when the PeerConnection is stopped. This means that already stopped PeerConnections could appear to still be alive. The CL that fixed this got reverted due to increased crashes, and I've not had time to address this since.
2.2. Another possible cause for this (not the one that was encountered by the tester, but still something we would want to fix) is the aforementioned #808402.

Björn, do you think we should merge this bug into one of those, or...?
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/022ed8aefca41af65bed913a23174eb7f04840aa

commit 022ed8aefca41af65bed913a23174eb7f04840aa
Author: Elad Alon <eladalon@chromium.org>
Date: Tue Feb 06 23:46:39 2018

RenderProcessHost exiting is an implicit PeerConnection removal

When a RenderProcessHost exits clearly or crashes, all of its
PeerConnections should be considered as removed.

Bug: 775415,  805398 ,  808402 
Change-Id: I223e6e867758ef05d896608fbdcc3bc824238dd8
Reviewed-on: https://chromium-review.googlesource.com/899348
Commit-Queue: Elad Alon <eladalon@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534840}
[modify] https://crrev.com/022ed8aefca41af65bed913a23174eb7f04840aa/content/browser/webrtc/webrtc_event_log_manager.cc
[modify] https://crrev.com/022ed8aefca41af65bed913a23174eb7f04840aa/content/browser/webrtc/webrtc_event_log_manager.h
[modify] https://crrev.com/022ed8aefca41af65bed913a23174eb7f04840aa/content/browser/webrtc/webrtc_event_log_manager_unittest.cc
[modify] https://crrev.com/022ed8aefca41af65bed913a23174eb7f04840aa/content/browser/webrtc/webrtc_local_event_log_manager.cc
[modify] https://crrev.com/022ed8aefca41af65bed913a23174eb7f04840aa/content/browser/webrtc/webrtc_local_event_log_manager.h
[modify] https://crrev.com/022ed8aefca41af65bed913a23174eb7f04840aa/content/browser/webrtc/webrtc_remote_event_log_manager.cc
[modify] https://crrev.com/022ed8aefca41af65bed913a23174eb7f04840aa/content/browser/webrtc/webrtc_remote_event_log_manager.h

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/743ea2cb3b876bc44a85587a904328cfe8ce2fed

commit 743ea2cb3b876bc44a85587a904328cfe8ce2fed
Author: Andrey Kosyakov <caseq@chromium.org>
Date: Wed Feb 07 00:09:21 2018

Revert "RenderProcessHost exiting is an implicit PeerConnection removal"

This reverts commit 022ed8aefca41af65bed913a23174eb7f04840aa.

Reason for revert: this broke Cast Audio bots, https://ci.chromium.org/buildbot/chromium.linux/Fuchsia%20ARM64%20Cast%20Audio/4367

Original change's description:
> RenderProcessHost exiting is an implicit PeerConnection removal
> 
> When a RenderProcessHost exits clearly or crashes, all of its
> PeerConnections should be considered as removed.
> 
> Bug: 775415,  805398 ,  808402 
> Change-Id: I223e6e867758ef05d896608fbdcc3bc824238dd8
> Reviewed-on: https://chromium-review.googlesource.com/899348
> Commit-Queue: Elad Alon <eladalon@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#534840}

TBR=guidou@chromium.org,terelius@chromium.org,eladalon@chromium.org

Change-Id: I6f33dded2bba23f471c73495518fdded3ccbc3dc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 775415,  805398 ,  808402 
Reviewed-on: https://chromium-review.googlesource.com/905638
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534845}
[modify] https://crrev.com/743ea2cb3b876bc44a85587a904328cfe8ce2fed/content/browser/webrtc/webrtc_event_log_manager.cc
[modify] https://crrev.com/743ea2cb3b876bc44a85587a904328cfe8ce2fed/content/browser/webrtc/webrtc_event_log_manager.h
[modify] https://crrev.com/743ea2cb3b876bc44a85587a904328cfe8ce2fed/content/browser/webrtc/webrtc_event_log_manager_unittest.cc
[modify] https://crrev.com/743ea2cb3b876bc44a85587a904328cfe8ce2fed/content/browser/webrtc/webrtc_local_event_log_manager.cc
[modify] https://crrev.com/743ea2cb3b876bc44a85587a904328cfe8ce2fed/content/browser/webrtc/webrtc_local_event_log_manager.h
[modify] https://crrev.com/743ea2cb3b876bc44a85587a904328cfe8ce2fed/content/browser/webrtc/webrtc_remote_event_log_manager.cc
[modify] https://crrev.com/743ea2cb3b876bc44a85587a904328cfe8ce2fed/content/browser/webrtc/webrtc_remote_event_log_manager.h

With the freshly landed https://chromium-review.googlesource.com/c/chromium/src/+/899348, the bug should be mostly mitigated - even if a peer connection is not garbage collected for a long time, closing the tab should now terminate its associated log and make room for new logs. Vasant, could you please check with a canary?
Project Member

Comment 11 by bugdroid1@chromium.org, Feb 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4ac547420e6715e1da6272408c519898ae09b58

commit b4ac547420e6715e1da6272408c519898ae09b58
Author: Elad Alon <eladalon@chromium.org>
Date: Wed Feb 07 18:13:55 2018

Revert "Revert "RenderProcessHost exiting is an implicit PeerConnection removal""

This reverts commit 743ea2cb3b876bc44a85587a904328cfe8ce2fed.

Reason for revert: Build break fixed; relanding.
Note: The build break was in the underlying CL - fixed by 904905.

Original change's description:
> Revert "RenderProcessHost exiting is an implicit PeerConnection removal"
>
> This reverts commit 022ed8aefca41af65bed913a23174eb7f04840aa.
>
> Reason for revert: this broke Cast Audio bots, https://ci.chromium.org/buildbot/chromium.linux/Fuchsia%20ARM64%20Cast%20Audio/4367
>
> Original change's description:
> > RenderProcessHost exiting is an implicit PeerConnection removal
> >
> > When a RenderProcessHost exits clearly or crashes, all of its
> > PeerConnections should be considered as removed.
> >
> > Bug: 775415,  805398 ,  808402 
> > Change-Id: I223e6e867758ef05d896608fbdcc3bc824238dd8
> > Reviewed-on: https://chromium-review.googlesource.com/899348
> > Commit-Queue: Elad Alon <eladalon@chromium.org>
> > Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#534840}
>
>
> Change-Id: I6f33dded2bba23f471c73495518fdded3ccbc3dc
> Bug: 775415,  805398 ,  808402 
> Reviewed-on: https://chromium-review.googlesource.com/905638
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#534845}

Change-Id: Ic231cb5466ce1411c8d8d9441ad6105a6da169ff
Bug: 775415,  805398 ,  808402 
Reviewed-on: https://chromium-review.googlesource.com/905607
Commit-Queue: Elad Alon <eladalon@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535063}
[modify] https://crrev.com/b4ac547420e6715e1da6272408c519898ae09b58/content/browser/webrtc/webrtc_event_log_manager.cc
[modify] https://crrev.com/b4ac547420e6715e1da6272408c519898ae09b58/content/browser/webrtc/webrtc_event_log_manager.h
[modify] https://crrev.com/b4ac547420e6715e1da6272408c519898ae09b58/content/browser/webrtc/webrtc_event_log_manager_unittest.cc
[modify] https://crrev.com/b4ac547420e6715e1da6272408c519898ae09b58/content/browser/webrtc/webrtc_local_event_log_manager.cc
[modify] https://crrev.com/b4ac547420e6715e1da6272408c519898ae09b58/content/browser/webrtc/webrtc_local_event_log_manager.h
[modify] https://crrev.com/b4ac547420e6715e1da6272408c519898ae09b58/content/browser/webrtc/webrtc_remote_event_log_manager.cc
[modify] https://crrev.com/b4ac547420e6715e1da6272408c519898ae09b58/content/browser/webrtc/webrtc_remote_event_log_manager.h

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8e05bb782597ac21ac3e42abed0fe08243ce69fd

commit 8e05bb782597ac21ac3e42abed0fe08243ce69fd
Author: Elad Alon <eladalon@chromium.org>
Date: Thu Feb 08 16:24:45 2018

Propagate PeerConnection closing signal to WebRtcEventLogManager

Propagate he signal that a PeerConnection was closed to
WebRtcEventLogManager. This is done separately from its
removal, since removal waits for garbage collection.

Bug:  780855 ,  805398 , 775415
Change-Id: I8f40cc2a3b6c859c9c185b3b90b56a21305186d9
Reviewed-on: https://chromium-review.googlesource.com/904003
Commit-Queue: Elad Alon <eladalon@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535398}
[modify] https://crrev.com/8e05bb782597ac21ac3e42abed0fe08243ce69fd/content/browser/renderer_host/media/peer_connection_tracker_host.cc
[modify] https://crrev.com/8e05bb782597ac21ac3e42abed0fe08243ce69fd/content/browser/webrtc/webrtc_event_log_manager.cc
[modify] https://crrev.com/8e05bb782597ac21ac3e42abed0fe08243ce69fd/content/browser/webrtc/webrtc_event_log_manager.h

Vasant, the two CLs fixing this have landed today (Feb 8th). Could you please take a build from later than right now (time of posting this comment) and verify that the issue has been resolved?
Yes definitely. I will verify and update the status. Thanks!
Status: Verified (was: Assigned)
Verified in Canary M66.0.3344.0 / 10390.0.0. (Wolf device).
Status: Available (was: Verified)
This bug is still reprodible in Windows 10. Chrome Version : 65.0.3325.51 (Chrome beta), Reopening. 
m65 was cut before the fix landed. I am not sure it's critical enough to cherry-pick, but I'll ask holmer@ for a second opinion.

For my information, suppose we don't cherry-pick this; what is the procedure for marking this bug as resolved and verified on m66, but won't-fix on m65?
Cc: holmer@chromium.org
Labels: -M-65 M-66
Owner: rantonysamy@chromium.org
Okey, We will take a note and verify this in M66, as of now I can assign it to myself and keep it open. 
@rantonysamy: It is already verified in Canary M66. we can test again in M65 if the fix is merged into M65. 
Sure, Lets keep this open. 
Verified at M66 Dev. Closing.
Status: Fixed (was: Available)
Status: Verified (was: Fixed)

Sign in to add a comment