Issue 780863 link

Starred by 1 user

Issue metadata

Status:	Fixed
Owner:	eladalon@chromium.org
Closed:	Dec 2017
Cc:	terelius@chromium.org solenberg@chromium.org
Components:	Blink>WebRTC>Tools Internals>Logging
EstimatedDays:	----
NextAction:	----
OS:	----
Pri:	3
Type:	Bug
M-64

New RTC event logs can override existing files

Project Member Reported by eladalon@chromium.org, Nov 2 2017

Issue description

Chrome Version: (copy from chrome://version)
OS: (e.g. Win7, OSX 10.9.5, etc...)

What steps will reproduce the problem?
(1) Start logging an RTC event log. Observe the local-id part of the filename.
(2) Create a new file with the same name, except that the local-id is incremented by one.
(3) Start another peer connection on the same renderer.

What is the expected result?
A new file is created.

What happens instead?
Either the existing file is used, or logging doesn't work. I'll check which of these when I get around to fixing this.

Please use labels and text to provide additional information.


For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.

Comment 1 by eladalon@chromium.org, Nov 9 2017

Description: Show this description

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 1 2017

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

commit 437fa566ed89d7eafc803ae1c304e5e1f34040fb
Author: Elad Alon <eladalon@chromium.org>
Date: Fri Dec 01 13:33:43 2017

Introduce RtcEventLogManager

RtcEventLogManager will manage both:
1. Writing RTC event logs to local files, based on user input via
WebRTCInternals. (Current CL.)
2. Uploading RTC event logs to remote servers, based on JS-triggered
configuration. (Upcoming CLs.)

The latter of the two is the use case which motivated the creation of
RtcEventLogManager, since we need this manager to outlive the call. We
also want RtcEventLogManager to run in new Chrome instances, even if
now WebRTC session exists, so that it may upload still-pending logs
from previous sessions. (This functionality will be added in upcoming
CLs.)

Bug: 775415,  780863 
Change-Id: I7504857ebf662fd914f5988ecbc1fae051f12275
Reviewed-on: https://chromium-review.googlesource.com/760816
Commit-Queue: Elad Alon <eladalon@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Tommi <tommi@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520926}
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/chrome/browser/extensions/api/DEPS
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/BUILD.gn
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/renderer_host/media/peer_connection_tracker_host.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/renderer_host/media/peer_connection_tracker_host.h
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/resources/media/dump_creator.js
[add] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/webrtc/webrtc_event_log_manager.cc
[add] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/webrtc/webrtc_event_log_manager.h
[add] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/webrtc/webrtc_event_log_manager_unittest.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/webrtc/webrtc_eventlog_host.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/webrtc/webrtc_eventlog_host.h
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/webrtc/webrtc_eventlog_host_unittest.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/webrtc/webrtc_internals.h
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/browser/webrtc/webrtc_internals_message_handler.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/common/media/peer_connection_tracker_messages.h
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/public/browser/render_process_host.h
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/public/test/mock_render_process_host.h
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/renderer/BUILD.gn
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/renderer/media/peer_connection_tracker.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/renderer/media/peer_connection_tracker.h
[add] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/renderer/media/rtc_event_log_output_sink.h
[add] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/renderer/media/rtc_event_log_output_sink_proxy.cc
[add] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/renderer/media/rtc_event_log_output_sink_proxy.h
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/renderer/media/rtc_peer_connection_handler.h
[modify] https://crrev.com/437fa566ed89d7eafc803ae1c304e5e1f34040fb/content/test/BUILD.gn

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 1 2017

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

commit 67dc496d6933b65092528bed4a995eef44a24ac9
Author: Avi Drissman <avi@chromium.org>
Date: Fri Dec 01 14:57:39 2017

Revert "Introduce RtcEventLogManager"

This reverts commit 437fa566ed89d7eafc803ae1c304e5e1f34040fb.

Reason for revert:

WebRtcEventLogManagerTest.LocalLogMayNotBeStartedTwice fails across multiple bots (https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests/65151) (https://ci.chromium.org/buildbot/chromium.memory/Linux%20TSan%20Tests/14934) with this change.

Original change's description:
> Introduce RtcEventLogManager
> 
> RtcEventLogManager will manage both:
> 1. Writing RTC event logs to local files, based on user input via
> WebRTCInternals. (Current CL.)
> 2. Uploading RTC event logs to remote servers, based on JS-triggered
> configuration. (Upcoming CLs.)
> 
> The latter of the two is the use case which motivated the creation of
> RtcEventLogManager, since we need this manager to outlive the call. We
> also want RtcEventLogManager to run in new Chrome instances, even if
> now WebRTC session exists, so that it may upload still-pending logs
> from previous sessions. (This functionality will be added in upcoming
> CLs.)
> 
> Bug: 775415,  780863 
> Change-Id: I7504857ebf662fd914f5988ecbc1fae051f12275
> Reviewed-on: https://chromium-review.googlesource.com/760816
> Commit-Queue: Elad Alon <eladalon@chromium.org>
> Reviewed-by: Chris Palmer <palmer@chromium.org>
> Reviewed-by: Tommi <tommi@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Reviewed-by: Camille Lamy <clamy@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Reviewed-by: Henrik Boström <hbos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#520926}

TBR=palmer@chromium.org,tommi@chromium.org,grunell@chromium.org,rdevlin.cronin@chromium.org,clamy@chromium.org,msramek@chromium.org,hbos@chromium.org,guidou@chromium.org,terelius@chromium.org,eladalon@chromium.org

Change-Id: I97ff60cccac1be13dc250f6a3bb129c09265e212
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 775415,  780863 
Reviewed-on: https://chromium-review.googlesource.com/803614
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520938}
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/chrome/browser/extensions/api/DEPS
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/BUILD.gn
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/renderer_host/media/peer_connection_tracker_host.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/renderer_host/media/peer_connection_tracker_host.h
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/resources/media/dump_creator.js
[delete] https://crrev.com/96b3189981d1eac151492a477b9994408b0063d3/content/browser/webrtc/webrtc_event_log_manager.cc
[delete] https://crrev.com/96b3189981d1eac151492a477b9994408b0063d3/content/browser/webrtc/webrtc_event_log_manager.h
[delete] https://crrev.com/96b3189981d1eac151492a477b9994408b0063d3/content/browser/webrtc/webrtc_event_log_manager_unittest.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/webrtc/webrtc_eventlog_host.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/webrtc/webrtc_eventlog_host.h
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/webrtc/webrtc_eventlog_host_unittest.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/webrtc/webrtc_internals.h
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/browser/webrtc/webrtc_internals_message_handler.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/common/media/peer_connection_tracker_messages.h
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/public/browser/render_process_host.h
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/public/test/mock_render_process_host.h
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/renderer/BUILD.gn
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/renderer/media/peer_connection_tracker.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/renderer/media/peer_connection_tracker.h
[delete] https://crrev.com/96b3189981d1eac151492a477b9994408b0063d3/content/renderer/media/rtc_event_log_output_sink.h
[delete] https://crrev.com/96b3189981d1eac151492a477b9994408b0063d3/content/renderer/media/rtc_event_log_output_sink_proxy.cc
[delete] https://crrev.com/96b3189981d1eac151492a477b9994408b0063d3/content/renderer/media/rtc_event_log_output_sink_proxy.h
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/renderer/media/rtc_peer_connection_handler.h
[modify] https://crrev.com/67dc496d6933b65092528bed4a995eef44a24ac9/content/test/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 1 2017

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

commit 2afe249c24e8e8a4721cfd6ae532f608e9655689
Author: Elad Alon <eladalon@chromium.org>
Date: Fri Dec 01 15:11:46 2017

Revert "Revert "Introduce RtcEventLogManager""

This reverts commit 67dc496d6933b65092528bed4a995eef44a24ac9.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Revert "Introduce RtcEventLogManager"
> 
> This reverts commit 437fa566ed89d7eafc803ae1c304e5e1f34040fb.
> 
> Reason for revert:
> 
> WebRtcEventLogManagerTest.LocalLogMayNotBeStartedTwice fails across multiple bots (https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests/65151) (https://ci.chromium.org/buildbot/chromium.memory/Linux%20TSan%20Tests/14934) with this change.
> 
> Original change's description:
> > Introduce RtcEventLogManager
> > 
> > RtcEventLogManager will manage both:
> > 1. Writing RTC event logs to local files, based on user input via
> > WebRTCInternals. (Current CL.)
> > 2. Uploading RTC event logs to remote servers, based on JS-triggered
> > configuration. (Upcoming CLs.)
> > 
> > The latter of the two is the use case which motivated the creation of
> > RtcEventLogManager, since we need this manager to outlive the call. We
> > also want RtcEventLogManager to run in new Chrome instances, even if
> > now WebRTC session exists, so that it may upload still-pending logs
> > from previous sessions. (This functionality will be added in upcoming
> > CLs.)
> > 
> > Bug: 775415,  780863 
> > Change-Id: I7504857ebf662fd914f5988ecbc1fae051f12275
> > Reviewed-on: https://chromium-review.googlesource.com/760816
> > Commit-Queue: Elad Alon <eladalon@chromium.org>
> > Reviewed-by: Chris Palmer <palmer@chromium.org>
> > Reviewed-by: Tommi <tommi@chromium.org>
> > Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> > Reviewed-by: Camille Lamy <clamy@chromium.org>
> > Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> > Reviewed-by: Henrik Boström <hbos@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#520926}
> 
> TBR=palmer@chromium.org,tommi@chromium.org,grunell@chromium.org,rdevlin.cronin@chromium.org,clamy@chromium.org,msramek@chromium.org,hbos@chromium.org,guidou@chromium.org,terelius@chromium.org,eladalon@chromium.org
> 
> Change-Id: I97ff60cccac1be13dc250f6a3bb129c09265e212
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 775415,  780863 
> Reviewed-on: https://chromium-review.googlesource.com/803614
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#520938}

TBR=avi@chromium.org,palmer@chromium.org,tommi@chromium.org,grunell@chromium.org,rdevlin.cronin@chromium.org,clamy@chromium.org,msramek@chromium.org,hbos@chromium.org,guidou@chromium.org,terelius@chromium.org,eladalon@chromium.org

Change-Id: Id36daaafaba9e9582f9fe1edd1c3e96493272f3c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 775415,  780863 
Reviewed-on: https://chromium-review.googlesource.com/803220
Reviewed-by: Henrik Andreasson <henrika@chromium.org>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520941}
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/chrome/browser/extensions/api/DEPS
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/BUILD.gn
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/renderer_host/media/peer_connection_tracker_host.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/renderer_host/media/peer_connection_tracker_host.h
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/resources/media/dump_creator.js
[add] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/webrtc/webrtc_event_log_manager.cc
[add] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/webrtc/webrtc_event_log_manager.h
[add] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/webrtc/webrtc_event_log_manager_unittest.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/webrtc/webrtc_eventlog_host.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/webrtc/webrtc_eventlog_host.h
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/webrtc/webrtc_eventlog_host_unittest.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/webrtc/webrtc_internals.h
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/browser/webrtc/webrtc_internals_message_handler.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/common/media/peer_connection_tracker_messages.h
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/public/browser/render_process_host.h
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/public/test/mock_render_process_host.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/public/test/mock_render_process_host.h
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/renderer/BUILD.gn
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/renderer/media/peer_connection_tracker.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/renderer/media/peer_connection_tracker.h
[add] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/renderer/media/rtc_event_log_output_sink.h
[add] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/renderer/media/rtc_event_log_output_sink_proxy.cc
[add] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/renderer/media/rtc_event_log_output_sink_proxy.h
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/renderer/media/rtc_peer_connection_handler.cc
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/renderer/media/rtc_peer_connection_handler.h
[modify] https://crrev.com/2afe249c24e8e8a4721cfd6ae532f608e9655689/content/test/BUILD.gn

Comment 5 by eladalon@chromium.org, Dec 6 2017

Labels: M-64
Status: Fixed (was: Assigned)

►

Issue 780863 link

Issue metadata

New RTC event logs can override existing files

Issue description

Comment 1 by eladalon@chromium.org, Nov 9 2017

Comment 1 Deleted

Comment 2 by bugdroid1@chromium.org, Dec 1 2017

Comment 2 Deleted

Comment 3 by bugdroid1@chromium.org, Dec 1 2017

Comment 3 Deleted

Comment 4 by bugdroid1@chromium.org, Dec 1 2017

Comment 4 Deleted

Comment 5 by eladalon@chromium.org, Dec 6 2017

Comment 5 Deleted

Sign in to add a comment