Revert "RenderProcessHost exiting is an implicit PeerConnection removal"

This reverts commit 022ed8ae.

Reason for revert: this broke Cast Audio bots, https://ci.chromium.org/buildbot/chromium.linux/Fuchsia%20ARM64%20Cast%20Audio/4367

Original change's description:
> RenderProcessHost exiting is an implicit PeerConnection removal
> 
> When a RenderProcessHost exits clearly or crashes, all of its
> PeerConnections should be considered as removed.
> 
> Bug: 775415, 805398, 808402
> Change-Id: I223e6e867758ef05d896608fbdcc3bc824238dd8
> Reviewed-on: https://chromium-review.googlesource.com/899348
> Commit-Queue: Elad Alon <eladalon@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#534840}

TBR=guidou@chromium.org,terelius@chromium.org,eladalon@chromium.org

Change-Id: I6f33dded2bba23f471c73495518fdded3ccbc3dc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 775415, 805398, 808402
Reviewed-on: https://chromium-review.googlesource.com/905638Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534845}

content/browser/webrtc/webrtc_event_log_manager.cc

@@ -96,11 +96,6 @@ WebRtcEventLogManager::WebRtcEventLogManager()
 
 WebRtcEventLogManager::~WebRtcEventLogManager() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  for (RenderProcessHost* host : observed_render_process_hosts_) {
-    host->RemoveObserver(this);
-  }
-
   DCHECK(g_webrtc_event_log_manager);
   g_webrtc_event_log_manager = nullptr;
 }
@@ -110,7 +105,6 @@ void WebRtcEventLogManager::EnableForBrowserContext(
     base::OnceClosure reply) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   CHECK(!browser_context->IsOffTheRecord());
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::EnableForBrowserContextInternal,
@@ -123,7 +117,6 @@ void WebRtcEventLogManager::DisableForBrowserContext(
     BrowserContextId browser_context_id,
     base::OnceClosure reply) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::DisableForBrowserContextInternal,
@@ -136,17 +129,6 @@ void WebRtcEventLogManager::PeerConnectionAdded(
     int lid,
     base::OnceCallback<void(bool)> reply) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  RenderProcessHost* host = RenderProcessHost::FromID(render_process_id);
-  auto it = observed_render_process_hosts_.find(host);
-  if (it == observed_render_process_hosts_.end()) {
-    // This is the first PeerConnection which we see that's associated
-    // with this RPH.
-    host->AddObserver(this);
-    observed_render_process_hosts_.insert(host);
-  }
-
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::PeerConnectionAddedInternal,
@@ -174,7 +156,6 @@ void WebRtcEventLogManager::PeerConnectionRemoved(
   }
   const BrowserContext* browser_context = host->GetBrowserContext();
 
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::PeerConnectionRemovedInternal,
@@ -188,7 +169,6 @@ void WebRtcEventLogManager::EnableLocalLogging(
     base::OnceCallback<void(bool)> reply) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!base_path.empty());
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::EnableLocalLoggingInternal,
@@ -199,7 +179,6 @@ void WebRtcEventLogManager::EnableLocalLogging(
 void WebRtcEventLogManager::DisableLocalLogging(
     base::OnceCallback<void(bool)> reply) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::DisableLocalLoggingInternal,
@@ -219,7 +198,6 @@ void WebRtcEventLogManager::StartRemoteLogging(
     browser_context_id = GetBrowserContextId(browser_context);
   }
 
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::StartRemoteLoggingInternal,
@@ -235,7 +213,6 @@ void WebRtcEventLogManager::OnWebRtcEventLogWrite(
     base::OnceCallback<void(std::pair<bool, bool>)> reply) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   const BrowserContext* browser_context = GetBrowserContext(render_process_id);
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::OnWebRtcEventLogWriteInternal,
@@ -248,7 +225,6 @@ void WebRtcEventLogManager::SetLocalLogsObserver(
     WebRtcLocalEventLogsObserver* observer,
     base::OnceClosure reply) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::SetLocalLogsObserverInternal,
@@ -259,7 +235,6 @@ void WebRtcEventLogManager::SetRemoteLogsObserver(
     WebRtcRemoteEventLogsObserver* observer,
     base::OnceClosure reply) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  // Posting to task queue owned by the unretained object - unretained is safe.
   task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&WebRtcEventLogManager::SetRemoteLogsObserverInternal,
@@ -278,38 +253,6 @@ WebRtcEventLogManager::GetTaskRunnerForTesting() {
   return task_runner_;
 }
 
-void WebRtcEventLogManager::RenderProcessExited(RenderProcessHost* host,
-                                                base::TerminationStatus status,
-                                                int exit_code) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  RenderProcessHostExitedDestroyed(host);
-}
-
-void WebRtcEventLogManager::RenderProcessHostDestroyed(
-    RenderProcessHost* host) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  RenderProcessHostExitedDestroyed(host);
-}
-
-void WebRtcEventLogManager::RenderProcessHostExitedDestroyed(
-    RenderProcessHost* host) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK(host);
-
-  auto it = observed_render_process_hosts_.find(host);
-  if (it == observed_render_process_hosts_.end()) {
-    return;  // We've never seen PeerConnections associated with this RPH.
-  }
-  host->RemoveObserver(this);
-  observed_render_process_hosts_.erase(host);
-
-  // Posting to task queue owned by the unretained object - unretained is safe.
-  task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&WebRtcEventLogManager::RenderProcessExitedInternal,
-                     base::Unretained(this), host->GetID()));
-}
-
 void WebRtcEventLogManager::OnLocalLogStarted(PeerConnectionKey peer_connection,
                                               const base::FilePath& file_path) {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
@@ -507,12 +450,6 @@ void WebRtcEventLogManager::OnWebRtcEventLogWriteInternal(
   }
 }
 
-void WebRtcEventLogManager::RenderProcessExitedInternal(int render_process_id) {
-  DCHECK(task_runner_->RunsTasksInCurrentSequence());
-  local_logs_manager_.RenderProcessHostExitedDestroyed(render_process_id);
-  remote_logs_manager_.RenderProcessHostExitedDestroyed(render_process_id);
-}
-
 void WebRtcEventLogManager::SetLocalLogsObserverInternal(
     WebRtcLocalEventLogsObserver* observer,
     base::OnceClosure reply) {

content/browser/webrtc/webrtc_event_log_manager.h

@@ -11,7 +11,6 @@
 #include <utility>
 
 #include "base/callback.h"
-#include "base/containers/flat_set.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/optional.h"
@@ -21,7 +20,6 @@
 #include "content/browser/webrtc/webrtc_local_event_log_manager.h"
 #include "content/browser/webrtc/webrtc_remote_event_log_manager.h"
 #include "content/common/content_export.h"
-#include "content/public/browser/render_process_host_observer.h"
 
 namespace content {
 
@@ -34,8 +32,7 @@ class BrowserContext;
 // user from the WebRTCIntenals. (A log may simulatenously be written to both,
 // either, or none.)
 class CONTENT_EXPORT WebRtcEventLogManager
-    : public RenderProcessHostObserver,
-      public WebRtcLocalEventLogsObserver,
+    : public WebRtcLocalEventLogsObserver,
       public WebRtcRemoteEventLogsObserver {
  public:
   using BrowserContextId = WebRtcRemoteEventLogManager::BrowserContextId;
@@ -212,16 +209,6 @@ class CONTENT_EXPORT WebRtcEventLogManager
 
   static WebRtcEventLogManager* g_webrtc_event_log_manager;
 
-  // RenderProcessHostObserver implementation.
-  void RenderProcessExited(RenderProcessHost* host,
-                           base::TerminationStatus status,
-                           int exit_code) override;
-  void RenderProcessHostDestroyed(RenderProcessHost* host) override;
-
-  // RenderProcessExited() and RenderProcessHostDestroyed() treated similarly
-  // by this function.
-  void RenderProcessHostExitedDestroyed(RenderProcessHost* host);
-
   // WebRtcLocalEventLogsObserver implementation:
   void OnLocalLogStarted(PeerConnectionKey peer_connection,
                          const base::FilePath& file_path) override;
@@ -270,8 +257,6 @@ class CONTENT_EXPORT WebRtcEventLogManager
       const std::string& message,
       base::OnceCallback<void(std::pair<bool, bool>)> reply);
 
-  void RenderProcessExitedInternal(int render_process_id);
-
   void SetLocalLogsObserverInternal(WebRtcLocalEventLogsObserver* observer,
                                     base::OnceClosure reply);
 
@@ -309,12 +294,6 @@ class CONTENT_EXPORT WebRtcEventLogManager
   std::map<PeerConnectionKey, LoggingTargetBitmap>
       peer_connections_with_event_logging_enabled_;
 
-  // The set of RenderProcessHosts with which the manager is registered for
-  // observation. Allows us to register for each RPH only once, and get notified
-  // when it exits (cleanly or due to a crash).
-  // This object is only to be accessed on the UI thread.
-  base::flat_set<RenderProcessHost*> observed_render_process_hosts_;
-
   // In production, this holds a small object that just tells WebRTC (via
   // PeerConnectionTracker) to start/stop producing event logs for a specific
   // peer connection. In (relevant) unit tests, a mock will be injected.

content/browser/webrtc/webrtc_local_event_log_manager.cc

@@ -121,30 +121,6 @@ bool WebRtcLocalEventLogManager::EventLogWrite(int render_process_id,
   return WriteToLogFile(it, message);
 }
 
-void WebRtcLocalEventLogManager::RenderProcessHostExitedDestroyed(
-    int render_process_id) {
-  // Remove all of the peer connections associated with this render process.
-  auto pc_it = active_peer_connections_.begin();
-  while (pc_it != active_peer_connections_.end()) {
-    if (pc_it->render_process_id == render_process_id) {
-      pc_it = active_peer_connections_.erase(pc_it);
-    } else {
-      ++pc_it;
-    }
-  }
-
-  // Close all of the files that were associated with peer connections which
-  // belonged to this render process.
-  auto log_it = log_files_.begin();
-  while (log_it != log_files_.end()) {
-    if (log_it->first.render_process_id == render_process_id) {
-      log_it = CloseLogFile(log_it);
-    } else {
-      ++log_it;
-    }
-  }
-}
-
 void WebRtcLocalEventLogManager::SetClockForTesting(base::Clock* clock) {
   clock_for_testing_ = clock;
 }

content/browser/webrtc/webrtc_local_event_log_manager.h

@@ -32,8 +32,6 @@ class WebRtcLocalEventLogManager final : public LogFileWriter {
                      int lid,
                      const std::string& message);
 
-  void RenderProcessHostExitedDestroyed(int render_process_id);
-
   // This function is public, but this entire class is a protected
   // implementation detail of WebRtcEventLogManager, which hides this
   // function from everybody except its own unit tests.

content/browser/webrtc/webrtc_remote_event_log_manager.cc

@@ -158,41 +158,6 @@ bool WebRtcRemoteEventLogManager::EventLogWrite(int render_process_id,
   return WriteToLogFile(it, message);
 }
 
-void WebRtcRemoteEventLogManager::RenderProcessHostExitedDestroyed(
-    int render_process_id) {
-  // Closing files will call MaybeStartUploading(). Avoid letting that upload
-  // any recently expired files.
-  PrunePendingLogs();
-
-  // Remove all of the peer connections associated with this render process.
-  // It's important to do this before closing the actual files, because closing
-  // files can trigger a new upload if no active peer connections are present.
-  auto pc_it = active_peer_connections_.begin();
-  while (pc_it != active_peer_connections_.end()) {
-    if (pc_it->render_process_id == render_process_id) {
-      pc_it = active_peer_connections_.erase(pc_it);
-    } else {
-      ++pc_it;
-    }
-  }
-
-  // Close all of the files that were associated with peer connections which
-  // belonged to this render process.
-  auto log_it = active_logs_.begin();
-  while (log_it != active_logs_.end()) {
-    if (log_it->first.render_process_id == render_process_id) {
-      log_it = CloseLogFile(log_it);
-    } else {
-      ++log_it;
-    }
-  }
-
-  // Though CloseLogFile() calls this, it's important to also do this
-  // explicitly, since it could be that no files were closed, but some
-  // active PeerConnections that were suppressing uploading are now gone.
-  MaybeStartUploading();
-}
-
 void WebRtcRemoteEventLogManager::OnWebRtcEventLogUploadComplete(
     const base::FilePath& file_path,
     bool upload_successful) {

content/browser/webrtc/webrtc_remote_event_log_manager.h

@@ -83,10 +83,6 @@ class CONTENT_EXPORT WebRtcRemoteEventLogManager final
                      int lid,
                      const std::string& message);
 
-  // An implicit PeerConnectionRemoved() on all of the peer connections that
-  // were associated with the renderer process.
-  void RenderProcessHostExitedDestroyed(int render_process_id);
-
   // WebRtcEventLogUploaderObserver implementation.
   void OnWebRtcEventLogUploadComplete(const base::FilePath& file_path,
                                       bool upload_successful) override;