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

content/browser/media/webrtc/webrtc_internals.cc

 // Copyright (c) 2013 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_internals.h"
 
 #include <stddef.h>
 
 #include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
 #include "content/browser/media/webrtc/webrtc_internals_ui_observer.h"
+#include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/web_contents/web_contents_view.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/power_save_blocker.h"
-#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
+#include "ipc/ipc_platform_file.h"
+
+#if defined(OS_WIN)
+#define IntToStringType base::IntToString16
+#else
+#define IntToStringType base::IntToString
+#endif
 
 using base::ProcessId;
 using std::string;
 
 namespace content {
 
 namespace {
 
 static base::LazyInstance<WebRTCInternals>::Leaky g_webrtc_internals =
     LAZY_INSTANCE_INITIALIZER;
@@ skipping 307 matching lines @@
     select_file_dialog_ = ui::SelectFileDialog::Create(this, nullptr);
     select_file_dialog_->SelectFile(
         ui::SelectFileDialog::SELECT_SAVEAS_FILE, base::string16(),
         event_log_recordings_file_path_, nullptr, 0, FILE_PATH_LITERAL(""),
         web_contents->GetTopLevelNativeWindow(), nullptr);
 #endif
   } else {
     event_log_recordings_ = false;
     // Tear down the dialog since the user has unchecked the audio debug
     // recordings box.
+    DCHECK(select_file_dialog_->HasOneRef());
     select_file_dialog_ = nullptr;
-    DCHECK(select_file_dialog_->HasOneRef());

[tommi (sloooow) - chröme, 2016/05/25 15:16:21]

yikes. shows the value of testing with dcheck enabled.

[Ivo-OOO until feb 6, 2016/05/30 15:04:15]

Indeed.

 
     for (RenderProcessHost::iterator i(
              content::RenderProcessHost::AllHostsIterator());
          !i.IsAtEnd(); i.Advance()) {
-      i.GetCurrentValue()->DisableEventLogRecordings();
+      RenderProcessHostImpl* rph =
+          static_cast<RenderProcessHostImpl*>(i.GetCurrentValue());

[tommi (sloooow) - chröme, 2016/05/25 15:16:21]

this feels hacky.  How can you be sure that this isn't e.g.
MockRenderProcessHost?

[ncarter (slow), 2016/05/25 21:28:58]

As tommi suggests, this is an illegal cast. In content, you can usually downcast
from interface to Impl, but RPH is a glaring exception to that rule, since the
Mock doesn't inherit from Impl. To the extent this downcast ever works, it means
there is no unittest coverage for this code.

You don't need this cast anyway; since the method you need to call is on the
interface.

[Ivo-OOO until feb 6, 2016/05/30 15:04:15]

Good points, removed the cast since it's not necessary.

+      rph->StopWebRTCEventLog();
     }
   }
 #endif
 }
 
 bool WebRTCInternals::IsEventLogRecordingsEnabled() const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   return event_log_recordings_;
 }
 
-const base::FilePath& WebRTCInternals::GetEventLogRecordingsFilePath() const {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  return event_log_recordings_file_path_;
-}
-
 void WebRTCInternals::SendUpdate(const string& command,
                                  std::unique_ptr<base::Value> value) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(observers_.might_have_observers());
 
   bool queue_was_empty = pending_updates_.empty();
   pending_updates_.push(PendingUpdate(command, std::move(value)));
 
   if (queue_was_empty) {
     BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,
@@ skipping 102 matching lines @@
   }
 }
 
 void WebRTCInternals::EnableEventLogRecordingsOnAllRenderProcessHosts() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   event_log_recordings_ = true;
   for (RenderProcessHost::iterator i(
            content::RenderProcessHost::AllHostsIterator());
        !i.IsAtEnd(); i.Advance()) {
-    i.GetCurrentValue()->EnableEventLogRecordings(
-        event_log_recordings_file_path_);
+    RenderProcessHostImpl* rph =
+        static_cast<RenderProcessHostImpl*>(i.GetCurrentValue());

[tommi (sloooow) - chröme, 2016/05/25 15:16:21]

same here

[Ivo-OOO until feb 6, 2016/05/30 15:04:15]

Removed here as well.

+    rph->StartWebRTCEventLog(event_log_recordings_file_path_);
   }
 }
 #endif
 
 void WebRTCInternals::CreateOrReleasePowerSaveBlocker() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (peer_connection_data_.empty() && power_save_blocker_) {
     DVLOG(1) << ("Releasing the block on application suspension since no "
                  "PeerConnections are active anymore.");
@@ skipping 12 matching lines @@
   while (!pending_updates_.empty()) {
     const auto& update = pending_updates_.front();
     FOR_EACH_OBSERVER(WebRTCInternalsUIObserver,
                       observers_,
                       OnUpdate(update.command(), update.value()));
     pending_updates_.pop();
   }
 }
 
 }  // namespace content