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

chrome/browser/media/webrtc_event_log_handler.cc

 // 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 "chrome/browser/media/webrtc_event_log_handler.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/media/webrtc_log_list.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.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"
+#include "content/public/browser/webrtc_callback_interface.h"
+#include "ipc/ipc_platform_file.h"
 
 using content::BrowserThread;
 
 // Keys used to attach handler to the RenderProcessHost
 const char WebRtcEventLogHandler::kWebRtcEventLogHandlerKey[] =
     "kWebRtcEventLogHandlerKey";
 
 namespace {
 
 // Returns a path name to be used as prefix for RTC event log files.
-base::FilePath GetWebRtcEventLogPrefixPath(const base::FilePath& directory,
-                                           uint64_t rtc_event_log_id) {
+base::FilePath GetWebRtcEventLogPrefixPath(const base::FilePath& directory) {
   static const char kWebRtcEventLogFilePrefix[] = "WebRtcEventLog.";
-  return directory.AppendASCII(kWebRtcEventLogFilePrefix +
-                               base::Int64ToString(rtc_event_log_id));
+  return directory.AppendASCII(kWebRtcEventLogFilePrefix);
 }
 
+// Appends the IDs to the RTC event log file name.
+base::FilePath GetWebRtcEventLogPath(const base::FilePath& base_file,
+                                     int render_process_id,
+                                     uint64_t rtc_event_log_id) {
+  return base_file.AddExtension(base::IntToString(render_process_id))
+      .AddExtension(base::Int64ToString(rtc_event_log_id));

[dcheng, 2016/05/13 06:50:11]

Uint64ToString, since the input type is unsigned.

[Ivo-OOO until feb 6, 2016/05/18 16:26:54]

Done.

+}
+
+// Opens a logfile to pass on to the renderer.
+IPC::PlatformFileForTransit CreateFileForProcess(
+    const base::FilePath& base_path,
+    int render_process_id,
+    uint64_t rtc_event_log_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::FilePath file_path =
+      GetWebRtcEventLogPath(base_path, render_process_id, rtc_event_log_id);
+  base::File event_log_file(
+      file_path, base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_APPEND);
+  if (!event_log_file.IsValid()) {
+    VLOG(1) << "Could not open WebRTC event log file, error="
+            << event_log_file.error_details();
+    return IPC::InvalidPlatformFileForTransit();
+  }
+  return IPC::TakePlatformFileForTransit(std::move(event_log_file));
+}
+
+void SendEventLogFileToRenderer(content::RenderProcessHost* rph,
+                                int local_id,
+                                IPC::PlatformFileForTransit file_for_transit) {
+  if (file_for_transit == IPC::InvalidPlatformFileForTransit()) {
+    return;
+  }
+#if defined(OS_ANDROID)
+  const int64_t max_filesize_bytes = 10000000;
+#else
+  const int64_t max_filesize_bytes = 30000000;

[dcheng, 2016/05/13 06:50:11]

Out of curiosity... why do we need to send this over IPC? Couldn't it just be
hardcoded in the renderer?

[Henrik Grunell, 2016/05/13 09:05:31]

Agree.

[Ivo-OOO until feb 6, 2016/05/18 16:26:54]

Ok, I will move it there, good point.

+#endif
+
+  rph->Send(new PeerConnectionTracker_StartEventLog(local_id, file_for_transit,
+                                                    max_filesize_bytes));
+}
 }  // namespace
 
-WebRtcEventLogHandler::WebRtcEventLogHandler(Profile* profile)
+WebRtcEventLogHandler::WebRtcEventLogHandler(int process_id, Profile* profile)
     : profile_(profile),
       is_rtc_event_logging_in_progress_(false),
+      number_log_files_(0),
       current_rtc_event_log_id_(0) {
   DCHECK(profile_);
   thread_checker_.DetachFromThread();
+  content::RenderProcessHost* host =
+      content::RenderProcessHost::FromID(process_id);
+  using StartEventLogFn = void (WebRtcEventLogHandler::*)(
+      content::RenderProcessHost* host, const base::FilePath& file_path);

[dcheng, 2016/05/13 06:50:11]

I think it's probably shorter (and clearer) to just write:

content::WebRTCCallbackInterface::GetInstance()->RegisterEventLogHandler(
    process_id,
    base::Bind(&WebRtcEventLogHandler::StartWebRtcEventLogging, this, host),
    base::Bind(&WebRtcEventLogHandler::StopWebRtcEventLogging, this, host)));

[Ivo-OOO until feb 6, 2016/05/18 16:26:54]

You're right that it's both shorter and clearer, however it doesn't work in this
case. I think it's because there are two functions with the same name, and the
compiler cannot figure out which one to use. Changing the name of the start/stop
functions would probably fix this, but I think function names are more
important.

+  using StopEventLogFn =
+      void (WebRtcEventLogHandler::*)(content::RenderProcessHost* host);
+  StartEventLogFn start_fn = &WebRtcEventLogHandler::StartWebRtcEventLogging;
+  StopEventLogFn stop_fn = &WebRtcEventLogHandler::StopWebRtcEventLogging;
+  content::WebRTCCallbackInterface::GetInstance()->RegisterEventLogHandler(
+      process_id, base::Bind(start_fn, this, host),
+      base::Bind(stop_fn, this, host));
+  content::WebRTCCallbackInterface::GetInstance()
+      ->RegisterPeerConnectionCallbacks(
+          process_id,
+          base::Bind(&WebRtcEventLogHandler::OnPeerConnectionAdded, this),
+          base::Bind(&WebRtcEventLogHandler::OnPeerConnectionRemoved, this));
 }
 
 WebRtcEventLogHandler::~WebRtcEventLogHandler() {}
 
 void WebRtcEventLogHandler::StartWebRtcEventLogging(
     content::RenderProcessHost* host,
     base::TimeDelta delay,
     const RecordingDoneCallback& callback,
     const RecordingErrorCallback& error_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&WebRtcEventLogHandler::GetLogDirectoryAndEnsureExists, this),
       base::Bind(&WebRtcEventLogHandler::DoStartWebRtcEventLogging, this, host,
                  delay, callback, error_callback));
 }
 
+void WebRtcEventLogHandler::StartWebRtcEventLogging(
+    content::RenderProcessHost* host,
+    const base::FilePath& file_path) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  is_rtc_event_logging_in_progress_ = true;

[dcheng, 2016/05/13 06:50:11]

This is triggered by a renderer IPC, right? What if a renderer sends this IPC
multiple times? Seems like it might let the renderer exceed the
number_log_files_ limit.

[Ivo-OOO until feb 6, 2016/05/18 16:26:54]

This is not triggered by renderer IPC (it is called from WebRTCInternals via
WebRTCCallbackInterface in the browser process), but you're right that calling
multiple times could cause problems, so I'll add some code to prevent that,
thanks.

+  base_file_path_ = file_path;
+#if defined(OS_ANDROID)
+  number_log_files_ = 3;

[Henrik Grunell, 2016/05/13 09:05:31]

Are these number based on something particular?

[Ivo-OOO until feb 6, 2016/05/18 16:26:54]

Not really, just on what seems like a reasonably upper limit for most situations
and on the intuition that desktops/laptops are faster than mobile phones. 

+#else
+  number_log_files_ = 10;
+#endif
+  for (int local_id : active_peer_connection_local_id_) {
+    if (number_log_files_ > 0) {
+      BrowserThread::PostTaskAndReplyWithResult(
+          BrowserThread::FILE, FROM_HERE,
+          base::Bind(&CreateFileForProcess, file_path, host->GetID(), local_id),
+          base::Bind(&SendEventLogFileToRenderer, host, local_id));
+      number_log_files_--;
+    }
+  }
+}
+
 void WebRtcEventLogHandler::StopWebRtcEventLogging(
     content::RenderProcessHost* host,
     const RecordingDoneCallback& callback,
     const RecordingErrorCallback& error_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   const bool is_manual_stop = true;
   BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&WebRtcEventLogHandler::GetLogDirectoryAndEnsureExists, this),
       base::Bind(&WebRtcEventLogHandler::DoStopWebRtcEventLogging, this, host,
                  is_manual_stop, current_rtc_event_log_id_, callback,
                  error_callback));
 }
 
+void WebRtcEventLogHandler::StopWebRtcEventLogging(
+    content::RenderProcessHost* host) {
+  is_rtc_event_logging_in_progress_ = false;
+  for (int local_id : active_peer_connection_local_id_) {
+    host->Send(new PeerConnectionTracker_StopEventLog(local_id));
+  }
+}
+
+void WebRtcEventLogHandler::OnPeerConnectionAdded(int process_id,
+                                                  int connection_id) {
+  active_peer_connection_local_id_.insert(connection_id);
+  if (is_rtc_event_logging_in_progress_ && number_log_files_ > 0) {
+    content::RenderProcessHost* host =
+        content::RenderProcessHost::FromID(process_id);
+    if (host) {
+      BrowserThread::PostTaskAndReplyWithResult(
+          BrowserThread::FILE, FROM_HERE,
+          base::Bind(&CreateFileForProcess, base_file_path_, process_id,
+                     connection_id),
+          base::Bind(&SendEventLogFileToRenderer, host, connection_id));
+      number_log_files_--;
+    }
+  }
+}
+
+void WebRtcEventLogHandler::OnPeerConnectionRemoved(int process_id,
+                                                    int connection_id) {
+  active_peer_connection_local_id_.erase(connection_id);
+}
+
 base::FilePath WebRtcEventLogHandler::GetLogDirectoryAndEnsureExists() {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   base::FilePath log_dir_path =
       WebRtcLogList::GetWebRtcLogDirectoryForProfile(profile_->GetPath());
   base::File::Error error;
   if (!base::CreateDirectoryAndGetError(log_dir_path, &error)) {
     DLOG(ERROR) << "Could not create WebRTC log directory, error: " << error;
     return base::FilePath();
   }
   return log_dir_path;
 }
 
 void WebRtcEventLogHandler::DoStartWebRtcEventLogging(
     content::RenderProcessHost* host,
     base::TimeDelta delay,
     const RecordingDoneCallback& callback,
     const RecordingErrorCallback& error_callback,
     const base::FilePath& log_directory) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (is_rtc_event_logging_in_progress_) {
     error_callback.Run("RTC event logging already in progress");
     return;
   }
-
-  is_rtc_event_logging_in_progress_ = true;
-  base::FilePath prefix_path =
-      GetWebRtcEventLogPrefixPath(log_directory, ++current_rtc_event_log_id_);
-  host->EnableEventLogRecordings(prefix_path);
+  base::FilePath prefix_path = GetWebRtcEventLogPrefixPath(log_directory);
+  StartWebRtcEventLogging(host, prefix_path);
 
   if (delay.is_zero()) {
     const bool is_stopped = false, is_manual_stop = false;
     callback.Run(prefix_path.AsUTF8Unsafe(), is_stopped, is_manual_stop);
     return;
   }
 
   const bool is_manual_stop = false;
   BrowserThread::PostDelayedTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&WebRtcEventLogHandler::DoStopWebRtcEventLogging, this, host,
                  is_manual_stop, current_rtc_event_log_id_, callback,
                  error_callback, log_directory),
       delay);
 }
 
 void WebRtcEventLogHandler::DoStopWebRtcEventLogging(
     content::RenderProcessHost* host,
     bool is_manual_stop,
     uint64_t rtc_event_log_id,
     const RecordingDoneCallback& callback,
     const RecordingErrorCallback& error_callback,
     const base::FilePath& log_directory) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_LE(rtc_event_log_id, current_rtc_event_log_id_);
 
-  base::FilePath prefix_path =
-      GetWebRtcEventLogPrefixPath(log_directory, rtc_event_log_id);
+  base::FilePath prefix_path = GetWebRtcEventLogPrefixPath(log_directory);
+  base::FilePath file_path =
+      GetWebRtcEventLogPath(prefix_path, host->GetID(), rtc_event_log_id);
   // Prevent an old posted DoStopWebRtcEventLogging() call to stop a newer dump.
   // This could happen in a sequence like:
   //   Start(10);  // Start dump 1. Post Stop() to run after 10 seconds.
   //   Stop();     // Manually stop dump 1 before 10 seconds;
   //   Start(20);  // Start dump 2. Posted Stop() for 1 should not stop dump 2.
   if (rtc_event_log_id < current_rtc_event_log_id_) {
     const bool is_stopped = false;
-    callback.Run(prefix_path.AsUTF8Unsafe(), is_stopped, is_manual_stop);
+    callback.Run(file_path.AsUTF8Unsafe(), is_stopped, is_manual_stop);
     return;
   }
 
   if (!is_rtc_event_logging_in_progress_) {
     error_callback.Run("No RTC event logging in progress");
     return;
   }
 
-  host->DisableEventLogRecordings();
-  is_rtc_event_logging_in_progress_ = false;
+  StopWebRtcEventLogging(host);
   const bool is_stopped = true;
-  callback.Run(prefix_path.AsUTF8Unsafe(), is_stopped, is_manual_stop);
+  callback.Run(file_path.AsUTF8Unsafe(), is_stopped, is_manual_stop);
 }