Move the call to enable the WebRTC event log from PeerConnectionFactory to PeerConnection.

content/browser/media/webrtc/webrtc_eventlog_host.cc

Index: content/browser/media/webrtc/webrtc_eventlog_host.cc
diff --git a/content/browser/media/webrtc/webrtc_eventlog_host.cc b/content/browser/media/webrtc/webrtc_eventlog_host.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0c0c8f9cddb9a158352911dc81268292c03452b1
--- /dev/null
+++ b/content/browser/media/webrtc/webrtc_eventlog_host.cc
@@ -0,0 +1,141 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/media/webrtc/webrtc_eventlog_host.h"
+
+#include <algorithm>
+#include <string>
+
+#include "base/files/file_util.h"
+#include "base/strings/string_number_conversions.h"
+#include "content/browser/media/webrtc/webrtc_internals.h"
+#include "content/common/media/peer_connection_tracker_messages.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_process_host.h"
+
+namespace content {
+
+int WebRTCEventLogHost::number_active_log_files_ = 0;
+
+namespace {
+
+#if defined(OS_ANDROID)
+const int kMaxNumberLogFiles = 3;
+#else
+const int kMaxNumberLogFiles = 5;
+#endif
+
+// Appends the IDs to the RTC event log file name.
+base::FilePath GetWebRtcEventLogPath(const base::FilePath& base_file,
+                                     int render_process_id,
+                                     int connection_id) {
+  return base_file.AddExtension(base::IntToString(render_process_id))
+      .AddExtension(base::IntToString(connection_id));
+}
+
+// Opens a logfile to pass on to the renderer.
+IPC::PlatformFileForTransit CreateFileForProcess(
+    const base::FilePath& base_path,
+    int render_process_id,
+    int connection_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::FilePath file_path =
+      GetWebRtcEventLogPath(base_path, render_process_id, connection_id);
+  base::File event_log_file(
+      file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
+  if (!event_log_file.IsValid()) {
+    PLOG(ERROR) << "Could not open WebRTC event log file, error="
+                << event_log_file.error_details();
+    return IPC::InvalidPlatformFileForTransit();
+  }
+  return IPC::TakePlatformFileForTransit(std::move(event_log_file));
+}
+
+}  // namespace
+
+WebRTCEventLogHost::WebRTCEventLogHost(int render_process_id)
+    : render_process_id_(render_process_id),
+      rtc_event_logging_enabled_(false),
+      weak_ptr_factory_(this) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  auto webrtc_internals = WebRTCInternals::GetInstance();
+  if (webrtc_internals->IsEventLogRecordingsEnabled())
+    StartWebRTCEventLog(webrtc_internals->GetEventLogFilePath());
+}
+
+WebRTCEventLogHost::~WebRTCEventLogHost() {}
+
+void WebRTCEventLogHost::PeerConnectionAdded(int peer_connection_local_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(std::find(active_peer_connection_local_ids_.begin(),
+                   active_peer_connection_local_ids_.end(),
+                   peer_connection_local_id) ==
+         active_peer_connection_local_ids_.end());
+  active_peer_connection_local_ids_.push_back(peer_connection_local_id);
+  if (rtc_event_logging_enabled_ &&
+      number_active_log_files_ < kMaxNumberLogFiles) {
+    DoStartEventLog(peer_connection_local_id);
+  }
+}
+
+void WebRTCEventLogHost::PeerConnectionRemoved(int peer_connection_local_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  const auto found = std::find(active_peer_connection_local_ids_.begin(),
+                               active_peer_connection_local_ids_.end(),
+                               peer_connection_local_id);
+  DCHECK(found != active_peer_connection_local_ids_.end());
+  active_peer_connection_local_ids_.erase(found);
+}
+
+bool WebRTCEventLogHost::StartWebRTCEventLog(const base::FilePath& file_path) {

[Henrik Grunell, 2016/06/03 07:40:52]

Why did the thread check go away? You should have it in all functions.

[Ivo-OOO until feb 6, 2016/06/03 16:07:01]

Done.

+  if (rtc_event_logging_enabled_)
+    return false;
+  rtc_event_logging_enabled_ = true;
+  base_file_path_ = file_path;
+  for (int local_id : active_peer_connection_local_ids_) {
+    DoStartEventLog(local_id);

[Henrik Grunell, 2016/06/03 07:40:52]

Nit: Name function "StartEventLogForPeerConnection()"? Increased readability
imo.

[Ivo-OOO until feb 6, 2016/06/03 16:07:02]

Done.

+  }
+  return true;
+}
+
+bool WebRTCEventLogHost::StopWebRTCEventLog() {
+  if (!rtc_event_logging_enabled_)
+    return false;
+  rtc_event_logging_enabled_ = false;
+  RenderProcessHost* host = RenderProcessHost::FromID(render_process_id_);
+  if (host) {
+    for (int local_id : active_peer_connection_local_ids_)
+      host->Send(new PeerConnectionTracker_StopEventLog(local_id));
+  }
+  return true;
+}
+
+bool WebRTCEventLogHost::DoStartEventLog(int peer_connection_local_id) {
+  if (number_active_log_files_ < kMaxNumberLogFiles) {

[Henrik Grunell, 2016/06/03 07:40:52]

OK, moving the counter increase causes a problem with this condition. It needs
to be increased here, and decreased in SendEventLogFileToRenderer if failed.

[Ivo-OOO until feb 6, 2016/06/03 16:07:01]

Good catch, thanks.

+    BrowserThread::PostTaskAndReplyWithResult(
+        BrowserThread::FILE, FROM_HERE,
+        base::Bind(&CreateFileForProcess, base_file_path_, render_process_id_,
+                   peer_connection_local_id),
+        base::Bind(&WebRTCEventLogHost::SendEventLogFileToRenderer,
+                   weak_ptr_factory_.GetWeakPtr(), peer_connection_local_id));
+  }
+  return true;
+}
+
+void WebRTCEventLogHost::SendEventLogFileToRenderer(
+    int peer_connection_local_id,
+    IPC::PlatformFileForTransit file_for_transit) {
+  if (file_for_transit == IPC::InvalidPlatformFileForTransit())
+    return;
+  RenderProcessHost* rph = RenderProcessHost::FromID(render_process_id_);
+  if (rph) {
+    rph->Send(new PeerConnectionTracker_StartEventLog(peer_connection_local_id,
+                                                      file_for_transit));
+    ++number_active_log_files_;
+  } else {
+    IPC::PlatformFileForTransitToFile(file_for_transit).Close();

[Henrik Grunell, 2016/06/03 07:40:52]

Great that you're closing it here.

+  }
+}
+
+}  // namespace content