Clear pending BackgroundTracingAgentClientImpl ctor

When renderer process is created, BackgroundTracingManagerImpl creates
closure for constructing of BackgroundTracingAgentClientImpl and creates
its instance lazily only when any agent observers are registered.
But it never releases if none of agent observer exists so it remained
as leaked memory.

To release pending constructor, BackgroundTracingAgentClientImpl
registers a disconnect handler to BackgroundTracingAgentProvider
then it releases when it is disconnected.
BackgroundTracingAgentProvider would be disconnected when the process
is gone.

Change-Id: I880c31caa7731dd3429c089ff523da5f846af00c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2065195
Commit-Queue: Darin Fisher <darin@chromium.org>
Reviewed-by: Darin Fisher <darin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745142}

content/browser/tracing/background_tracing_agent_client_impl.cc

@@ -16,15 +16,12 @@ namespace content {
 // static
 void BackgroundTracingAgentClientImpl::Create(
     int child_process_id,
-    mojo::PendingRemote<tracing::mojom::BackgroundTracingAgentProvider>
-        pending_provider) {
+    mojo::Remote<tracing::mojom::BackgroundTracingAgentProvider> provider) {
   mojo::PendingRemote<tracing::mojom::BackgroundTracingAgentClient> client;
   auto client_receiver = client.InitWithNewPipeAndPassReceiver();
 
   mojo::Remote<tracing::mojom::BackgroundTracingAgent> agent;
 
-  mojo::Remote<tracing::mojom::BackgroundTracingAgentProvider> provider(
-      std::move(pending_provider));
   provider->Create(ChildProcessHostImpl::ChildProcessUniqueIdToTracingProcessId(
                        child_process_id),
                    std::move(client), agent.BindNewPipeAndPassReceiver());

content/browser/tracing/background_tracing_agent_client_impl.h

@@ -19,8 +19,7 @@ class BackgroundTracingAgentClientImpl
  public:
   static void Create(
       int child_process_id,
-      mojo::PendingRemote<tracing::mojom::BackgroundTracingAgentProvider>
-          pending_provider);
+      mojo::Remote<tracing::mojom::BackgroundTracingAgentProvider> provider);
 
   ~BackgroundTracingAgentClientImpl() override;
 

content/browser/tracing/background_tracing_manager_impl.cc

@@ -70,14 +70,9 @@ void BackgroundTracingManagerImpl::ActivateForProcess(
   child_process->GetBackgroundTracingAgentProvider(
       pending_provider.InitWithNewPipeAndPassReceiver());
 
-  auto constructor =
-      base::BindOnce(&BackgroundTracingAgentClientImpl::Create,
-                     child_process_id, std::move(pending_provider));
-
-  base::PostTask(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&BackgroundTracingManagerImpl::AddPendingAgentConstructor,
-                     std::move(constructor)));
+  base::PostTask(FROM_HERE, {BrowserThread::UI},
+                 base::BindOnce(&BackgroundTracingManagerImpl::AddPendingAgent,
+                                child_process_id, std::move(pending_provider)));
 }
 
 BackgroundTracingManagerImpl::BackgroundTracingManagerImpl()
@@ -456,26 +451,42 @@ void BackgroundTracingManagerImpl::OnScenarioAborted() {
 }
 
 // static
-void BackgroundTracingManagerImpl::AddPendingAgentConstructor(
-    base::OnceClosure constructor) {
+void BackgroundTracingManagerImpl::AddPendingAgent(
+    int child_process_id,
+    mojo::PendingRemote<tracing::mojom::BackgroundTracingAgentProvider>
+        pending_provider) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  // Stash away the parameters here, and delay agent initialization until we
-  // have an interested AgentObserver.
+  // Delay agent initialization until we have an interested AgentObserver.
+  // We set disconnect handler for cleanup when the tracing target is closed.
+  mojo::Remote<tracing::mojom::BackgroundTracingAgentProvider> provider(
+      std::move(pending_provider));
 
-  GetInstance()->pending_agent_constructors_.push_back(std::move(constructor));
+  provider.set_disconnect_handler(base::BindOnce(
+      &BackgroundTracingManagerImpl::ClearPendingAgent, child_process_id));
+
+  GetInstance()->pending_agents_[child_process_id] = std::move(provider);
   GetInstance()->MaybeConstructPendingAgents();
 }
 
+// static
+void BackgroundTracingManagerImpl::ClearPendingAgent(int child_process_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  GetInstance()->pending_agents_.erase(child_process_id);
+}
+
 void BackgroundTracingManagerImpl::MaybeConstructPendingAgents() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (agent_observers_.empty())
     return;
 
-  for (auto& constructor : pending_agent_constructors_)
-    std::move(constructor).Run();
-  pending_agent_constructors_.clear();
+  for (auto& pending_agent : pending_agents_) {
+    pending_agent.second.set_disconnect_handler(base::OnceClosure());
+    BackgroundTracingAgentClientImpl::Create(pending_agent.first,
+                                             std::move(pending_agent.second));
+  }
+  pending_agents_.clear();
 }
 
 }  // namespace content

content/browser/tracing/background_tracing_manager_impl.h

@@ -12,8 +12,10 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "components/tracing/common/background_tracing_agent.mojom.h"
 #include "content/browser/tracing/background_tracing_config_impl.h"
 #include "content/public/browser/background_tracing_manager.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "services/tracing/public/cpp/perfetto/trace_event_data_source.h"
 
 namespace base {
@@ -24,6 +26,7 @@ class NoDestructor;
 namespace tracing {
 namespace mojom {
 class BackgroundTracingAgent;
+class BackgroundTracingAgentProvider;
 }  // namespace mojom
 }  // namespace tracing
 
@@ -153,7 +156,11 @@ class BackgroundTracingManagerImpl : public BackgroundTracingManager {
       bool privacy_filtering_enabled);
   bool IsTriggerHandleValid(TriggerHandle handle) const;
   void OnScenarioAborted();
-  static void AddPendingAgentConstructor(base::OnceClosure constructor);
+  static void AddPendingAgent(
+      int child_process_id,
+      mojo::PendingRemote<tracing::mojom::BackgroundTracingAgentProvider>
+          provider);
+  static void ClearPendingAgent(int child_process_id);
   void MaybeConstructPendingAgents();
 
   std::unique_ptr<BackgroundTracingActiveScenario> active_scenario_;
@@ -168,7 +175,8 @@ class BackgroundTracingManagerImpl : public BackgroundTracingManager {
   std::set<tracing::mojom::BackgroundTracingAgent*> agents_;
   std::set<AgentObserver*> agent_observers_;
 
-  std::vector<base::OnceClosure> pending_agent_constructors_;
+  std::map<int, mojo::Remote<tracing::mojom::BackgroundTracingAgentProvider>>
+      pending_agents_;
 
   IdleCallback idle_callback_;
   base::RepeatingClosure tracing_enabled_callback_for_testing_;