WebRTCEventLogHost::StopWebRTCEventLog() not called when a PeerConnection is torn down |
|||||
Issue descriptionThis leads to number_active_log_files_ never being decremented, which in turn leads us to have up to 5 logs per Chrome session, rather than up to 5 ACTIVE logs per Chrome session.
,
Nov 2 2017
,
Nov 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d138e38ab67e6badaacbd041dcdd67eeeaadd735 commit d138e38ab67e6badaacbd041dcdd67eeeaadd735 Author: Elad Alon <eladalon@chromium.org> Date: Tue Nov 07 14:36:25 2017 Unregister RTCPeerConnectionHandler upon native PC's Close() The existing code only unregisters the handler it is destructed. However, garbage collection might not destroy the object for a while. Bug: 780855 Change-Id: I68d2c1b49cc5042d53464878977400bfd3b75c81 Reviewed-on: https://chromium-review.googlesource.com/754838 Commit-Queue: Elad Alon <eladalon@chromium.org> Reviewed-by: Henrik Boström <hbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#514466} [modify] https://crrev.com/d138e38ab67e6badaacbd041dcdd67eeeaadd735/content/renderer/media/rtc_peer_connection_handler.cc
,
Nov 7 2017
Anatoli, this is too new for git-branch to let me know into which cut it's made it. I will update when you ping me. :-)
,
Nov 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c497f4cf075d4f5253822b99183d2292049c8607 commit c497f4cf075d4f5253822b99183d2292049c8607 Author: Elad Alon <eladalon@chromium.org> Date: Tue Nov 07 16:14:27 2017 Fix limit on concurrent RTC event log files WebRTCEventLogHost::StopWebRTCEventLog() is not called when a PeerConnection is torn down. This leads to number_active_log_files_ never being decremented, which in turn leads us to have up to 5 logs per Chrome session, rather than up to 5 ACTIVE logs per Chrome session. Bug: 780855 Change-Id: I06e916136bc539d6783a91d8967f8a09f0f31c9e Reviewed-on: https://chromium-review.googlesource.com/751666 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Elad Alon <eladalon@chromium.org> Cr-Commit-Position: refs/heads/master@{#514481} [modify] https://crrev.com/c497f4cf075d4f5253822b99183d2292049c8607/content/browser/webrtc/webrtc_eventlog_host.cc [modify] https://crrev.com/c497f4cf075d4f5253822b99183d2292049c8607/content/browser/webrtc/webrtc_eventlog_host.h
,
Nov 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/443c81f945a354dd3b11d220895b9c98009552f0 commit 443c81f945a354dd3b11d220895b9c98009552f0 Author: Benjamin Joyce <bjoyce@google.com> Date: Sun Nov 12 21:52:50 2017 Revert "Unregister RTCPeerConnectionHandler upon native PC's Close()" This reverts commit d138e38ab67e6badaacbd041dcdd67eeeaadd735. Reason for revert: assessing if cause for https://bugs.chromium.org/p/chromium/issues/detail?id=783010 Original change's description: > Unregister RTCPeerConnectionHandler upon native PC's Close() > > The existing code only unregisters the handler it is destructed. > However, garbage collection might not destroy the object for a while. > > Bug: 780855 > Change-Id: I68d2c1b49cc5042d53464878977400bfd3b75c81 > Reviewed-on: https://chromium-review.googlesource.com/754838 > Commit-Queue: Elad Alon <eladalon@chromium.org> > Reviewed-by: Henrik Boström <hbos@chromium.org> > Cr-Commit-Position: refs/heads/master@{#514466} TBR=hbos@chromium.org,guidou@chromium.org,terelius@chromium.org,eladalon@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 780855 Change-Id: I567bfb7f5aa7f2658b1dbee06a3fe5b66f3dec4b Reviewed-on: https://chromium-review.googlesource.com/765147 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Benjamin Joyce <bjoyce@google.com> Cr-Commit-Position: refs/heads/master@{#515870} [modify] https://crrev.com/443c81f945a354dd3b11d220895b9c98009552f0/content/renderer/media/rtc_peer_connection_handler.cc
,
Feb 2 2018
We forgot to reopen the bug when unrolling the fix.
,
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
,
Feb 8 2018
For posterity - that exact class (WebRTCEventLogHost) has since been removed, but the issue has been resolved by the attached CL (WebRtcEventLogManager now gets an indication that the PeerConnection has been stopped). |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by eladalon@chromium.org
, Nov 2 2017