New issue
Advanced search Search tips

Issue 759785 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
tommi@chromium.org
sergeyu@chromium.org
r...@chromium.org
tbarzic@chromium.org
eladalon@chromium.org
grunell@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 675413



Sign in to add a comment

Move chrome.webrtcLoggingPrivate and webrtcAudioPrivate to //extensions

Project Member Reported by michae...@chromium.org, Aug 28 2017 
chrome.webrtcLoggingPrivate and chrome.webrtcAudioPrivate are implemented in //chrome. To provide their functionality to other clients, they should be moved to //extensions to the extent that their Chrome dependencies can be removed/modified.

Specifically, these APIs need to be used from AppShell (go/app-shell-linux).

This work may involve a large number of files in chrome/browser/media/webrtc, but at first glance the existing dependencies on //chrome don't seem too bad.

For additional motivation and context, see internal discussion starting at b/33415667#comment42.

Comment 1 by grunell@chromium.org, Sep 29 2017 

[Re-triage] What's the right prio for this? Can you also set a milestone?

Comment 2 by michae...@chromium.org, Sep 29 2017

Labels: -Pri-1 M-64 Pri-2
I guess Pri-2. It's my next task after moving the APIs that are more strictly necessary to dogfood AppShell.

Comment 3 by michae...@chromium.org, Dec 8 2017

Labels: -Pri-2 -M-64 M-65 Pri-1
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 16 2018 

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

commit 82740190c4ae389041743a9a706ae638cd776c58
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Tue Jan 16 03:10:37 2018

Reduce Chrome dependencies in WebRTC logging

Remove some unnecessary Chrome-specific references, like
Profile => BrowserContext, and unused #includes.

Later we will extract some WebRTC logging code into a common location
for use by other content embedders.

Bug: 759785
Change-Id: I8e2503d6d47540dde7ebe771d01e672836656edc
Reviewed-on: https://chromium-review.googlesource.com/823175
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529358}
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/extensions/api/webrtc_logging_private/webrtc_logging_private_browsertest.cc
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/media/webrtc/audio_debug_recordings_handler.h
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/media/webrtc/webrtc_log_list.cc
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/media/webrtc/webrtc_log_list.h
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/media/webrtc/webrtc_log_util.cc
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/media/webrtc/webrtc_logging_handler_host.cc
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/media/webrtc/webrtc_logging_handler_host.h
[modify] https://crrev.com/82740190c4ae389041743a9a706ae638cd776c58/chrome/browser/ui/webui/media/webrtc_logs_ui.cc

Comment 5 by grunell@chromium.org, Jan 19 2018 

[Re-triage] Michael, please update the milestone. Also, it doesn't seem to be a P1 judging by the small amount of activity for M65. Set to P2 unless it is a must to land for the set milestone.

Comment 6 by michae...@chromium.org, Jan 19 2018

Labels: -Pri-1 -M-65 M-66 Pri-2
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 22 2018 

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

commit 79fff65b43728942bbe35eb72223ce2de10c6ce2
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Mon Jan 22 07:36:51 2018

Create webrtc_logging component

Extracts some Chrome WebRTC logging functionality into a component, to
eventually be used by other content embedders.

Part 1 of ~4 CLs to move the WebRTC code responsible for diagnostic
logging out of //chrome. This will enable moving some Chrome extension
APIs from //chrome to //extensions, so that chrome.webrtcLoggingPrivate
and chrome.webrtcAudioPrivate can be used from app_shell.

Bug: 759785
Change-Id: Ic48cd405d69a36a031d787e0ccb0ef155f67af95
Reviewed-on: https://chromium-review.googlesource.com/822872
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Tommi <tommi@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530823}
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/BUILD.gn
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/extensions/api/webrtc_logging_private/webrtc_logging_private_browsertest.cc
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/media/webrtc/audio_debug_recordings_handler.cc
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/media/webrtc/webrtc_log_uploader.cc
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/media/webrtc/webrtc_log_util.cc
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/media/webrtc/webrtc_log_util.h
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/media/webrtc/webrtc_logging_handler_host.cc
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/media/webrtc/webrtc_text_log_handler.cc
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/media/webrtc/webrtc_text_log_handler.h
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/browser/ui/webui/media/webrtc_logs_ui.cc
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/common/BUILD.gn
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/renderer/media/DEPS
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/renderer/media/chrome_webrtc_log_message_delegate.cc
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/chrome/test/BUILD.gn
[modify] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/BUILD.gn
[add] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/OWNERS
[add] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/README.md
[add] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/browser/BUILD.gn
[add] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/browser/DEPS
[add] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/browser/log_cleanup.cc
[add] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/browser/log_cleanup.h
[rename] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/browser/log_cleanup_unittest.cc
[rename] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/browser/log_list.cc
[rename] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/browser/log_list.h
[add] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/common/BUILD.gn
[rename] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/common/partial_circular_buffer.cc
[rename] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/common/partial_circular_buffer.h
[rename] https://crrev.com/79fff65b43728942bbe35eb72223ce2de10c6ce2/components/webrtc_logging/common/partial_circular_buffer_unittest.cc

Comment 8 by grunell@chromium.org, Jan 23 2018 

Please make sure to test logging manually for each CL you work on. I've tested with ToT which includes the landed CL above.

To test:
* Do a Hangouts call.
* Open chrome://webrtc-logs in a new tab. (Note: reloading it if already open doesn't refresh it unfortunately - same as with chrome://crashes.)
* Verify that there is a log file for the just made call. Check that it contains log lines. Check that it has been uploaded.
* I can verify that it has been received properly.

For the audio API, I can instruct when you have a CL to move that.

Comment 9 by michae...@chromium.org, Jan 23 2018 

Will do, thanks.

Comment 10 by michae...@chromium.org, Mar 16 2018 

grunell: How do I get Chrome to upload WebRTC logs?

I've built an official build locally, made a bunch of calls (generating logs), even tried sending feedback from Hangouts, but none of the webrtcLoggingPrivate functions get called, and the WebUI says all of the log files are "not uploaded".

Metrics reporting is enabled and the Hangouts extension seems to be working.

Comment 11 by michae...@chromium.org, Mar 16 2018

Cc: eladalon@chromium.org
Actually, I just discovered https://bugs.chromium.org/p/chromium/issues/detail?id=775415#c35 which might explain what I'm seeing in chrome://webrtc-logs.

Comment 12 by michae...@chromium.org, Jun 28 2018

Owner: ----
Status: Available (was: Assigned)
►

Sign in to add a comment