Skip to content

Commit

Permalink
Introduce PeerConnectionTrackerHostObserver
Browse files Browse the repository at this point in the history 
This is a new interface that allows a class to subscribe to
peer connection events.

Both WebRTCInternals and WebRtcEventLogManager are refactored to use
this common interface instead of being called directly by the
PeerConnectionTrackerHost.

Change 6/7

Bug: 1191026
Change-Id: I95edfd04c8994034efd9d9398795bcf895e6ed8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2770361
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Elad Alon <eladalon@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#867944}
  • Loading branch information
@plmonette-zz
plmonette-zz authored and Chromium LUCI CQ committed Mar 31, 2021
1 parent 97e457b commit 9025a10
Show file tree
Hide file tree
Showing 10 changed files with 255 additions and 134 deletions.
12 changes: 9 additions & 3 deletions chrome/browser/extensions/api/webrtc_logging_private/webrtc_logging_private_apitest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,11 +399,17 @@ class WebrtcLoggingPrivateApiTest : public extensions::ExtensionApiTest {
void SetUpPeerConnection(const std::string& session_id = "") {
auto* manager = WebRtcEventLogManager::GetInstance();

content::GlobalFrameRoutingId frame_id =
web_contents()->GetMainFrame()->GetGlobalFrameRoutingId();
content::RenderFrameHost* render_frame_host =
web_contents()->GetMainFrame();
const content::GlobalFrameRoutingId frame_id =
render_frame_host->GetGlobalFrameRoutingId();
const base::ProcessId pid =
render_frame_host->GetProcess()->GetProcess().Pid();
const int lid = 0;

manager->OnPeerConnectionAdded(frame_id, lid);
manager->OnPeerConnectionAdded(frame_id, lid, pid, /*url=*/std::string(),
/*rtc_configuration=*/std::string(),
/*constraints=*/std::string());

if (!session_id.empty()) {
manager->OnPeerConnectionSessionIdSet(frame_id, lid, session_id);
Expand Down
6 changes: 5 additions & 1 deletion chrome/browser/media/webrtc/webrtc_event_log_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,11 @@ void WebRtcEventLogManager::DisableForBrowserContext(

void WebRtcEventLogManager::OnPeerConnectionAdded(
content::GlobalFrameRoutingId frame_id,
int lid) {
int lid,
base::ProcessId pid,
const std::string& url,
const std::string& rtc_configuration,
const std::string& constraints) {
OnPeerConnectionAdded(frame_id, lid, base::NullCallback());
}

Expand Down
53 changes: 31 additions & 22 deletions chrome/browser/media/webrtc/webrtc_event_log_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "components/prefs/pref_change_registrar.h"
#include "components/upload_list/upload_list.h"
#include "content/public/browser/global_routing_id.h"
#include "content/public/browser/peer_connection_tracker_host_observer.h"
#include "content/public/browser/render_process_host_observer.h"
#include "content/public/browser/webrtc_event_logger.h"

Expand All @@ -48,10 +49,12 @@ namespace webrtc_event_logging {
// destroyed from ~BrowserProcessImpl(), at which point any tasks posted to the
// internal SequencedTaskRunner, or coming from another thread, would no longer
// execute.
class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
public content::WebRtcEventLogger,
public WebRtcLocalEventLogsObserver,
public WebRtcRemoteEventLogsObserver {
class WebRtcEventLogManager final
: public content::RenderProcessHostObserver,
public content::PeerConnectionTrackerHostObserver,
public content::WebRtcEventLogger,
public WebRtcLocalEventLogsObserver,
public WebRtcRemoteEventLogsObserver {
public:
using BrowserContextId = WebRtcEventLogPeerConnectionKey::BrowserContextId;

Expand Down Expand Up @@ -98,9 +101,13 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
void DisableForBrowserContext(content::BrowserContext* browser_context,
base::OnceClosure reply);

// content::WebRtcEventLogger implementation.
// content::PeerConnectionTrackerHostObserver implementation.
void OnPeerConnectionAdded(content::GlobalFrameRoutingId frame_id,
int lid) override;
int lid,
base::ProcessId pid,
const std::string& url,
const std::string& rtc_configuration,
const std::string& constraints) override;
void OnPeerConnectionRemoved(content::GlobalFrameRoutingId frame_id,
int lid) override;
void OnPeerConnectionUpdated(content::GlobalFrameRoutingId frame_id,
Expand All @@ -110,12 +117,14 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
void OnPeerConnectionSessionIdSet(content::GlobalFrameRoutingId frame_id,
int lid,
const std::string& session_id) override;
void EnableLocalLogging(const base::FilePath& base_path) override;
void DisableLocalLogging() override;
void OnWebRtcEventLogWrite(content::GlobalFrameRoutingId frame_id,
int lid,
const std::string& message) override;

// content::WebRtcEventLogger implementation.
void EnableLocalLogging(const base::FilePath& base_path) override;
void DisableLocalLogging() override;

// Start logging a peer connection's WebRTC events to a file, which will
// later be uploaded to a remote server. If a reply is provided, it will be
// posted back to BrowserThread::UI with the log-identifier (if successful)
Expand Down Expand Up @@ -210,9 +219,9 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
// by this function.
void RenderProcessHostExitedDestroyed(content::RenderProcessHost* host);

// Each method overridden from content::WebRtcEventLogger has an overload that
// posts back a reply on the UI thread with the result of the operation. Used
// for testing only.
// Each method overridden from content::PeerConnectionTrackerHostObserver and
// content::WebRtcEventLogger has an overload that posts back a reply on the
// UI thread with the result of the operation. Used for testing only.

// An overload of OnPeerConnectionAdded() that replies true if and only if the
// operation was successful. A failure can happen if a peer connection with
Expand Down Expand Up @@ -243,6 +252,17 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
const std::string& session_id,
base::OnceCallback<void(bool)> reply);

// An overload of OnWebRtcEventLogWrite() that replies with a pair of bool.
// The first bool is associated with local logging and the second bool is
// associated with remote-bound logging. Each bool assumes the value true if
// and only if the message was written in its entirety into a
// local/remote-bound log file.
void OnWebRtcEventLogWrite(
content::GlobalFrameRoutingId frame_id,
int lid,
const std::string& message,
base::OnceCallback<void(std::pair<bool, bool>)> reply);

// An overload of EnableLocalLogging() replies true if the logging was
// actually enabled. i.e. The logging was not already enabled before the call.
void EnableLocalLogging(const base::FilePath& base_path,
Expand All @@ -258,17 +278,6 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
// actually disabled. i.e. The logging was enabled before the call.
void DisableLocalLogging(base::OnceCallback<void(bool)> reply);

// An overload of OnWebRtcEventLogWrite() that replies with a pair of bool.
// The first bool is associated with local logging and the second bool is
// associated with remote-bound logging. Each bool assumes the value true if
// and only if the message was written in its entirety into a
// local/remote-bound log file.
void OnWebRtcEventLogWrite(
content::GlobalFrameRoutingId frame_id,
int lid,
const std::string& message,
base::OnceCallback<void(std::pair<bool, bool>)> reply);

// WebRtcLocalEventLogsObserver implementation:
void OnLocalLogStarted(PeerConnectionKey peer_connection,
const base::FilePath& file_path) override;
Expand Down
90 changes: 45 additions & 45 deletions content/browser/renderer_host/media/peer_connection_tracker_host.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,29 @@

#include "base/bind.h"
#include "base/no_destructor.h"
#include "base/observer_list.h"
#include "base/power_monitor/power_monitor.h"
#include "base/ranges/algorithm.h"
#include "base/task/post_task.h"
#include "content/browser/renderer_host/render_process_host_impl.h"
#include "content/browser/webrtc/webrtc_internals.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/global_routing_id.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/webrtc_event_logger.h"
#include "services/service_manager/public/cpp/interface_provider.h"

namespace content {

namespace {

using ObserverListType = base::ObserverList<PeerConnectionTrackerHostObserver,
/*check_empty=*/true,
/*allow_reentrancy=*/false>;
ObserverListType& GetObserverList() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
static base::NoDestructor<ObserverListType> observer_list{};
return *observer_list;
}

std::set<PeerConnectionTrackerHost*>& AllHosts() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
static base::NoDestructor<std::set<PeerConnectionTrackerHost*>> all_hosts{};
Expand All @@ -39,6 +47,22 @@ void RemoveHost(PeerConnectionTrackerHost* host) {

} // namespace

// static
void PeerConnectionTrackerHost::AddObserver(
base::PassKey<PeerConnectionTrackerHostObserver>,
PeerConnectionTrackerHostObserver* observer) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
GetObserverList().AddObserver(observer);
}

// static
void PeerConnectionTrackerHost::RemoveObserver(
base::PassKey<PeerConnectionTrackerHostObserver>,
PeerConnectionTrackerHostObserver* observer) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
GetObserverList().RemoveObserver(observer);
}

// static
const std::set<PeerConnectionTrackerHost*>&
PeerConnectionTrackerHost::GetAllHosts() {
Expand Down Expand Up @@ -75,30 +99,17 @@ void PeerConnectionTrackerHost::AddPeerConnection(
blink::mojom::PeerConnectionInfoPtr info) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

WebRTCInternals* webrtc_internals = WebRTCInternals::GetInstance();
if (webrtc_internals) {
webrtc_internals->OnPeerConnectionAdded(frame_id_, info->lid, peer_pid_,
info->url, info->rtc_configuration,
info->constraints);
}

WebRtcEventLogger* logger = WebRtcEventLogger::Get();
if (logger) {
logger->OnPeerConnectionAdded(frame_id_, info->lid);
for (auto& observer : GetObserverList()) {
observer.OnPeerConnectionAdded(frame_id_, info->lid, peer_pid_, info->url,
info->rtc_configuration, info->constraints);
}
}

void PeerConnectionTrackerHost::RemovePeerConnection(int lid) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

WebRTCInternals* webrtc_internals = WebRTCInternals::GetInstance();
if (webrtc_internals) {
webrtc_internals->OnPeerConnectionRemoved(frame_id_, lid);
}

WebRtcEventLogger* logger = WebRtcEventLogger::Get();
if (logger) {
logger->OnPeerConnectionRemoved(frame_id_, lid);
for (auto& observer : GetObserverList()) {
observer.OnPeerConnectionRemoved(frame_id_, lid);
}
}

Expand All @@ -107,14 +118,8 @@ void PeerConnectionTrackerHost::UpdatePeerConnection(int lid,
const std::string& value) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

WebRtcEventLogger* logger = WebRtcEventLogger::Get();
if (logger) {
logger->OnPeerConnectionUpdated(frame_id_, lid, type, value);
}

WebRTCInternals* webrtc_internals = WebRTCInternals::GetInstance();
if (webrtc_internals) {
webrtc_internals->OnPeerConnectionUpdated(frame_id_, lid, type, value);
for (auto& observer : GetObserverList()) {
observer.OnPeerConnectionUpdated(frame_id_, lid, type, value);
}
}

Expand All @@ -123,27 +128,24 @@ void PeerConnectionTrackerHost::OnPeerConnectionSessionIdSet(
const std::string& session_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

WebRtcEventLogger* logger = WebRtcEventLogger::Get();
if (logger) {
logger->OnPeerConnectionSessionIdSet(frame_id_, lid, session_id);
for (auto& observer : GetObserverList()) {
observer.OnPeerConnectionSessionIdSet(frame_id_, lid, session_id);
}
}

void PeerConnectionTrackerHost::AddStandardStats(int lid, base::Value value) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

WebRTCInternals* webrtc_internals = WebRTCInternals::GetInstance();
if (webrtc_internals) {
webrtc_internals->OnAddStandardStats(frame_id_, lid, std::move(value));
for (auto& observer : GetObserverList()) {
observer.OnAddStandardStats(frame_id_, lid, value.Clone());
}
}

void PeerConnectionTrackerHost::AddLegacyStats(int lid, base::Value value) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

WebRTCInternals* webrtc_internals = WebRTCInternals::GetInstance();
if (webrtc_internals) {
webrtc_internals->OnAddLegacyStats(frame_id_, lid, std::move(value));
for (auto& observer : GetObserverList()) {
observer.OnAddLegacyStats(frame_id_, lid, value.Clone());
}
}

Expand All @@ -155,10 +157,9 @@ void PeerConnectionTrackerHost::GetUserMedia(
const std::string& video_constraints) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

WebRTCInternals* webrtc_internals = WebRTCInternals::GetInstance();
if (webrtc_internals) {
webrtc_internals->OnGetUserMedia(frame_id_, peer_pid_, origin, audio, video,
audio_constraints, video_constraints);
for (auto& observer : GetObserverList()) {
observer.OnGetUserMedia(frame_id_, peer_pid_, origin, audio, video,
audio_constraints, video_constraints);
}
}

Expand All @@ -167,10 +168,9 @@ void PeerConnectionTrackerHost::WebRtcEventLogWrite(
const std::vector<uint8_t>& output) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

std::string converted_output(output.begin(), output.end());
WebRtcEventLogger* logger = WebRtcEventLogger::Get();
if (logger) {
logger->OnWebRtcEventLogWrite(frame_id_, lid, converted_output);
std::string message(output.begin(), output.end());
for (auto& observer : GetObserverList()) {
observer.OnWebRtcEventLogWrite(frame_id_, lid, message);
}
}

Expand Down
11 changes: 11 additions & 0 deletions content/browser/renderer_host/media/peer_connection_tracker_host.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
#include "base/macros.h"
#include "base/power_monitor/power_observer.h"
#include "base/process/process_handle.h"
#include "base/types/pass_key.h"
#include "content/public/browser/global_routing_id.h"
#include "content/public/browser/peer_connection_tracker_host_observer.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
Expand All @@ -29,6 +31,9 @@ class RenderFrameHost;
// managed by RenderFrameHostImpl. It receives PeerConnection events from
// PeerConnectionTracker as IPC messages that it forwards to WebRTCInternals.
// It also forwards browser process events to PeerConnectionTracker via IPC.
//
// Note: This class and all of its methods are meant to only be used on the UI
// thread.
class PeerConnectionTrackerHost
: public base::PowerSuspendObserver,
public base::PowerThermalObserver,
Expand All @@ -37,6 +42,12 @@ class PeerConnectionTrackerHost
explicit PeerConnectionTrackerHost(RenderFrameHost* rfh);
~PeerConnectionTrackerHost() override;

// Adds/removes a PeerConnectionTrackerHostObserver.
static void AddObserver(base::PassKey<PeerConnectionTrackerHostObserver>,
PeerConnectionTrackerHostObserver* observer);
static void RemoveObserver(base::PassKey<PeerConnectionTrackerHostObserver>,
PeerConnectionTrackerHostObserver* observer);

static const std::set<PeerConnectionTrackerHost*>& GetAllHosts();

// base::PowerSuspendObserver override.
Expand Down

0 comments on commit 9025a10

Please sign in to comment.