OOP HP: The profiling service exposes a list of profiled processes.

This CL adds a new method of the profiling_service mojom interface which
asynchronously returns the list of profiled processes. This is used in two places:
  * By the chrome://memory-internals to determine whether a process is being
  profiled. This also allows chrome://memory-internals to expose a "start
  profiling" button for non-profiled processes.
  * By the background profiling triggers, since traces should only be triggered
  if a profiled process exceeds the thresholds.

Bug: 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I44da48eb1203b82f9cef8907fc7c2ab0fdcfdfce
Reviewed-on: https://chromium-review.googlesource.com/809505
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522037}

chrome/browser/profiling_host/background_profiling_triggers.cc

@@ -74,10 +74,11 @@ void BackgroundProfilingTriggers::StartTimer() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   // Register a repeating timer to check memory usage periodically.
-  timer_.Start(
-      FROM_HERE, base::TimeDelta::FromHours(kRepeatingCheckMemoryDelayInHours),
-      base::Bind(&BackgroundProfilingTriggers::PerformMemoryUsageChecks,
-                 weak_ptr_factory_.GetWeakPtr()));
+  timer_.Start(FROM_HERE,
+               base::TimeDelta::FromHours(kRepeatingCheckMemoryDelayInHours),
+               base::BindRepeating(
+                   &BackgroundProfilingTriggers::PerformMemoryUsageChecks,
+                   weak_ptr_factory_.GetWeakPtr()));
 }
 
 bool BackgroundProfilingTriggers::IsAllowedToUpload() const {
@@ -126,13 +127,20 @@ void BackgroundProfilingTriggers::PerformMemoryUsageChecks() {
     return;
   }
 
-  memory_instrumentation::MemoryInstrumentation::GetInstance()
-      ->RequestGlobalDump(
-          base::Bind(&BackgroundProfilingTriggers::OnReceivedMemoryDump,
-                     weak_ptr_factory_.GetWeakPtr()));
+  auto callback = base::BindOnce(
+      [](base::WeakPtr<BackgroundProfilingTriggers> weak_ptr,
+         std::vector<base::ProcessId> result) {
+        memory_instrumentation::MemoryInstrumentation::GetInstance()
+            ->RequestGlobalDump(
+                base::Bind(&BackgroundProfilingTriggers::OnReceivedMemoryDump,
+                           std::move(weak_ptr), std::move(result)));
+      },
+      weak_ptr_factory_.GetWeakPtr());
+  host_->GetProfiledPids(std::move(callback));
 }
 
 void BackgroundProfilingTriggers::OnReceivedMemoryDump(
+    std::vector<base::ProcessId> profiled_pids,
     bool success,
     memory_instrumentation::mojom::GlobalMemoryDumpPtr dump) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
@@ -143,6 +151,9 @@ void BackgroundProfilingTriggers::OnReceivedMemoryDump(
 
   bool should_send_report = false;
   for (const auto& proc : dump->process_dumps) {
+    if (std::find(profiled_pids.begin(), profiled_pids.end(), proc->pid) ==
+        profiled_pids.end())
+      continue;
     if (IsOverTriggerThreshold(GetContentProcessType(proc->process_type),
                                proc->os_dump->private_footprint_kb)) {
       should_send_report = true;
@@ -156,12 +167,12 @@ void BackgroundProfilingTriggers::OnReceivedMemoryDump(
     // If a report was sent, throttle the memory data collection rate to
     // kThrottledReportRepeatingCheckMemoryDelayInHours to avoid sending too
     // many reports from a known problematic client.
-    timer_.Start(
-        FROM_HERE,
-        base::TimeDelta::FromHours(
-            kThrottledReportRepeatingCheckMemoryDelayInHours),
-        base::Bind(&BackgroundProfilingTriggers::PerformMemoryUsageChecks,
-                   weak_ptr_factory_.GetWeakPtr()));
+    timer_.Start(FROM_HERE,
+                 base::TimeDelta::FromHours(
+                     kThrottledReportRepeatingCheckMemoryDelayInHours),
+                 base::BindRepeating(
+                     &BackgroundProfilingTriggers::PerformMemoryUsageChecks,
+                     weak_ptr_factory_.GetWeakPtr()));
   }
 }
 

chrome/browser/profiling_host/background_profiling_triggers.h

@@ -53,6 +53,7 @@ class BackgroundProfilingTriggers {
   // checks on memory usage and trigger a memory report with
   // |TriggerMemoryReportForProcess| if needed.
   void OnReceivedMemoryDump(
+      std::vector<base::ProcessId> profiled_pids,
       bool success,
       memory_instrumentation::mojom::GlobalMemoryDumpPtr ptr);
 

chrome/browser/profiling_host/background_profiling_triggers_unittest.cc

@@ -89,6 +89,8 @@ class BackgroundProfilingTriggersTest : public testing::Test {
     ASSERT_TRUE(testing_profile_manager_.SetUp());
     ChromeMetricsServiceAccessor::SetMetricsAndCrashReportingForTesting(
         &is_metrics_enabled_);
+    for (base::ProcessId i = 1; i <= 10; ++i)
+      profiled_pids_.push_back(i);
   }
 
   void TearDown() override {
@@ -105,6 +107,7 @@ class BackgroundProfilingTriggersTest : public testing::Test {
 
   ProfilingProcessHost host_;
   FakeBackgroundProfilingTriggers triggers_;
+  std::vector<base::ProcessId> profiled_pids_;
 
   bool is_metrics_enabled_;
 };
@@ -117,35 +120,36 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_EmptyCases) {
 
   GlobalMemoryDumpPtr dump_empty(
       memory_instrumentation::mojom::GlobalMemoryDump::New());
-  triggers_.OnReceivedMemoryDump(true, std::move(dump_empty));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump_empty));
   EXPECT_FALSE(triggers_.WasReportTriggered());
   triggers_.Reset();
 
   GlobalMemoryDumpPtr dump_browser(
       memory_instrumentation::mojom::GlobalMemoryDump::New());
   PopulateMetrics(&dump_browser, 1, ProcessType::BROWSER, 1, 1, 1);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump_browser));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump_browser));
   EXPECT_FALSE(triggers_.WasReportTriggered());
   triggers_.Reset();
 
   GlobalMemoryDumpPtr dump_gpu(
       memory_instrumentation::mojom::GlobalMemoryDump::New());
   PopulateMetrics(&dump_gpu, 1, ProcessType::GPU, 1, 1, 1);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump_gpu));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump_gpu));
   EXPECT_FALSE(triggers_.WasReportTriggered());
   triggers_.Reset();
 
   GlobalMemoryDumpPtr dump_renderer(
       memory_instrumentation::mojom::GlobalMemoryDump::New());
   PopulateMetrics(&dump_renderer, 1, ProcessType::RENDERER, 1, 1, 1);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump_renderer));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true,
+                                 std::move(dump_renderer));
   EXPECT_FALSE(triggers_.WasReportTriggered());
   triggers_.Reset();
 
   GlobalMemoryDumpPtr dump_other(
       memory_instrumentation::mojom::GlobalMemoryDump::New());
   PopulateMetrics(&dump_other, 1, ProcessType::OTHER, 1, 1, 1);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump_other));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump_other));
   EXPECT_FALSE(triggers_.WasReportTriggered());
   triggers_.Reset();
 }
@@ -168,7 +172,7 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeNone) {
   PopulateMetrics(&dump, 6, ProcessType::RENDERER, kProcessMallocTriggerKb,
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
 
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_FALSE(triggers_.WasReportTriggered());
 }
 
@@ -183,7 +187,7 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeAll) {
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
 
   // Small processes and OTHER type processes don't trigger.
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_FALSE(triggers_.WasReportTriggered());
 
   // Ensure each process type triggers.
@@ -191,21 +195,39 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeAll) {
   dump = memory_instrumentation::mojom::GlobalMemoryDump::New();
   PopulateMetrics(&dump, 1, ProcessType::BROWSER, kProcessMallocTriggerKb,
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_TRUE(triggers_.WasReportTriggered());
 
   triggers_.Reset();
   dump = memory_instrumentation::mojom::GlobalMemoryDump::New();
   PopulateMetrics(&dump, 1, ProcessType::RENDERER, kProcessMallocTriggerKb,
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_TRUE(triggers_.WasReportTriggered());
 
   triggers_.Reset();
   dump = memory_instrumentation::mojom::GlobalMemoryDump::New();
   PopulateMetrics(&dump, 1, ProcessType::GPU, kProcessMallocTriggerKb,
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
+  EXPECT_TRUE(triggers_.WasReportTriggered());
+}
+
+// Non-profiled processes don't trigger.
+TEST_F(BackgroundProfilingTriggersTest, OnlyProfiledProcessesTrigger) {
+  SetMode(ProfilingProcessHost::Mode::kAll);
+
+  GlobalMemoryDumpPtr dump(
+      memory_instrumentation::mojom::GlobalMemoryDump::New());
+  PopulateMetrics(&dump, 101, ProcessType::BROWSER, kProcessMallocTriggerKb,
+                  kProcessMallocTriggerKb, kProcessMallocTriggerKb);
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
+  EXPECT_FALSE(triggers_.WasReportTriggered());
+
+  dump = memory_instrumentation::mojom::GlobalMemoryDump::New();
+  PopulateMetrics(&dump, 1, ProcessType::BROWSER, kProcessMallocTriggerKb,
+                  kProcessMallocTriggerKb, kProcessMallocTriggerKb);
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_TRUE(triggers_.WasReportTriggered());
 }
 
@@ -224,7 +246,7 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeMinimal) {
 
   // Ensure Browser and GPU processes under threshold do not trigger.
   // Ensure other process types ignored.
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_FALSE(triggers_.WasReportTriggered());
 
   // Ensure BROWSER and GPU types trigger.
@@ -233,7 +255,7 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeMinimal) {
   PopulateMetrics(&dump, 1, ProcessType::BROWSER, kProcessMallocTriggerKb,
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
   PopulateMetrics(&dump, 2, ProcessType::GPU, 1, 1, 1);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_TRUE(triggers_.WasReportTriggered());
 
   triggers_.Reset();
@@ -241,7 +263,7 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeMinimal) {
   PopulateMetrics(&dump, 1, ProcessType::GPU, kProcessMallocTriggerKb,
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
   PopulateMetrics(&dump, 2, ProcessType::BROWSER, 1, 1, 1);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_TRUE(triggers_.WasReportTriggered());
 }
 
@@ -261,7 +283,7 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeBrowser) {
 
   // Ensure Browser processes under threshold do not trigger.
   // Ensure other process types ignored.
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_FALSE(triggers_.WasReportTriggered());
 
   // Ensure BROWSER type triggers.
@@ -269,7 +291,7 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeBrowser) {
   dump = memory_instrumentation::mojom::GlobalMemoryDump::New();
   PopulateMetrics(&dump, 1, ProcessType::BROWSER, kProcessMallocTriggerKb,
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_TRUE(triggers_.WasReportTriggered());
 }
 
@@ -289,7 +311,7 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeGpu) {
 
   // Ensure GPU processes under threshold do not trigger.
   // Ensure other process types ignored.
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_FALSE(triggers_.WasReportTriggered());
 
   // Ensure GPU type triggers.
@@ -297,7 +319,7 @@ TEST_F(BackgroundProfilingTriggersTest, OnReceivedMemoryDump_ModeGpu) {
   dump = memory_instrumentation::mojom::GlobalMemoryDump::New();
   PopulateMetrics(&dump, 1, ProcessType::GPU, kProcessMallocTriggerKb,
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_TRUE(triggers_.WasReportTriggered());
 }
 
@@ -318,7 +340,7 @@ TEST_F(BackgroundProfilingTriggersTest,
 
   // Ensure RENDERER processes under threshold do not trigger.
   // Ensure other process types ignored.
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_FALSE(triggers_.WasReportTriggered());
 
   // Ensure RENDERER type triggers.
@@ -326,7 +348,7 @@ TEST_F(BackgroundProfilingTriggersTest,
   dump = memory_instrumentation::mojom::GlobalMemoryDump::New();
   PopulateMetrics(&dump, 1, ProcessType::RENDERER, kProcessMallocTriggerKb,
                   kProcessMallocTriggerKb, kProcessMallocTriggerKb);
-  triggers_.OnReceivedMemoryDump(true, std::move(dump));
+  triggers_.OnReceivedMemoryDump(profiled_pids_, true, std::move(dump));
   EXPECT_TRUE(triggers_.WasReportTriggered());
 }
 

chrome/browser/profiling_host/profiling_process_host.cc

@@ -34,6 +34,7 @@
 #include "chrome/common/profiling/profiling_constants.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_child_process_host.h"
+#include "content/public/browser/browser_child_process_host_iterator.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
@@ -192,7 +193,6 @@ ProfilingProcessHost::ProfilingProcessHost()
     : is_registered_(false),
       background_triggers_(this),
       mode_(Mode::kNone),
-      profiled_renderer_(nullptr),
       always_sample_for_tests_(false) {}
 
 ProfilingProcessHost::~ProfilingProcessHost() {
@@ -229,17 +229,7 @@ void ProfilingProcessHost::BrowserChildProcessLaunchedAndConnected(
     return;
   }
 
-  profiling::mojom::ProcessType process_type =
-      (data.process_type == content::ProcessType::PROCESS_TYPE_GPU)
-          ? profiling::mojom::ProcessType::GPU
-          : profiling::mojom::ProcessType::OTHER;
-
-  content::BrowserThread::GetTaskRunnerForThread(content::BrowserThread::IO)
-      ->PostTask(
-          FROM_HERE,
-          base::BindOnce(&ProfilingProcessHost::StartProfilingNonRendererChild,
-                         base::Unretained(this), data.id,
-                         base::GetProcId(data.handle), process_type));
+  StartProfilingNonRendererChild(data);
 }
 
 void ProfilingProcessHost::Observe(
@@ -255,32 +245,14 @@ void ProfilingProcessHost::Observe(
   // the RenderProcessHost's lifetime is ending. Ideally, we'd only listen to
   // the former, but if the RenderProcessHost is destroyed before the
   // RenderProcess, then the former is never sent.
-  if (host == profiled_renderer_ &&
-      (type == content::NOTIFICATION_RENDERER_PROCESS_TERMINATED ||
+  if ((type == content::NOTIFICATION_RENDERER_PROCESS_TERMINATED ||
        type == content::NOTIFICATION_RENDERER_PROCESS_CLOSED)) {
-    // |profiled_renderer_| is only ever set in kRendererSampling mode. This
-    // code is a deadstore otherwise so it's safe but having a DCHECK makes the
-    // intent clear.
-    DCHECK_EQ(mode(), Mode::kRendererSampling);
-    profiled_renderer_ = nullptr;
+    profiled_renderers_.erase(host);
   }
+
   if (type == content::NOTIFICATION_RENDERER_PROCESS_CREATED &&
       ShouldProfileNewRenderer(host)) {
-    // In kRendererSampling mode, store the RPH to ensure only one renderer is
-    // ever sampled inside a chrome instance.
-    if (mode() == Mode::kRendererSampling) {
-      profiled_renderer_ = host;
-    }
-    // Tell the child process to start profiling.
-    ProfilingClientBinder client(host);
-
-    content::BrowserThread::GetTaskRunnerForThread(content::BrowserThread::IO)
-        ->PostTask(
-            FROM_HERE,
-            base::BindOnce(&ProfilingProcessHost::AddClientToProfilingService,
-                           base::Unretained(this), client.take(),
-                           base::GetProcId(host->GetHandle()),
-                           profiling::mojom::ProcessType::RENDERER));
+    StartProfilingRenderer(host);
   }
 }
 
@@ -567,6 +539,85 @@ void ProfilingProcessHost::RequestTraceWithHeapDump(
   }
 }
 
+void ProfilingProcessHost::GetProfiledPids(GetProfiledPidsCallback callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  content::BrowserThread::GetTaskRunnerForThread(content::BrowserThread::IO)
+      ->PostTask(
+          FROM_HERE,
+          base::BindOnce(&ProfilingProcessHost::GetProfiledPidsOnIOThread,
+                         base::Unretained(this), std::move(callback)));
+}
+
+void ProfilingProcessHost::GetProfiledPidsOnIOThread(
+    GetProfiledPidsCallback callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+  if (!profiling_service_.is_bound()) {
+    content::BrowserThread::GetTaskRunnerForThread(content::BrowserThread::UI)
+        ->PostTask(FROM_HERE, base::BindOnce(std::move(callback),
+                                             std::vector<base::ProcessId>()));
+    return;
+  }
+
+  auto post_result_to_ui_thread = base::BindOnce(
+      [](GetProfiledPidsCallback callback,
+         const std::vector<base::ProcessId>& result) {
+        content::BrowserThread::GetTaskRunnerForThread(
+            content::BrowserThread::UI)
+            ->PostTask(FROM_HERE, base::BindOnce(std::move(callback), result));
+      },
+      std::move(callback));
+  profiling_service_->GetProfiledPids(std::move(post_result_to_ui_thread));
+}
+
+void ProfilingProcessHost::StartProfiling(base::ProcessId pid) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  // The RenderProcessHost iterator must be used on the UI thread.
+  // The BrowserChildProcessHostIterator iterator must be used on the IO thread.
+  for (auto iter = content::RenderProcessHost::AllHostsIterator();
+       !iter.IsAtEnd(); iter.Advance()) {
+    if (pid == base::GetProcId(iter.GetCurrentValue()->GetHandle())) {
+      StartProfilingRenderer(iter.GetCurrentValue());
+      return;
+    }
+  }
+
+  content::BrowserThread::GetTaskRunnerForThread(content::BrowserThread::IO)
+      ->PostTask(
+          FROM_HERE,
+          base::BindOnce(&ProfilingProcessHost::StartProfilingPidOnIOThread,
+                         base::Unretained(this), pid));
+}
+
+void ProfilingProcessHost::StartProfilingPidOnIOThread(base::ProcessId pid) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+
+  // Check if the request is for the current process.
+  if (pid == base::GetCurrentProcId()) {
+    ProfilingClientBinder client(connector_.get());
+    AddClientToProfilingService(client.take(), base::Process::Current().Pid(),
+                                profiling::mojom::ProcessType::BROWSER);
+    return;
+  }
+
+  for (content::BrowserChildProcessHostIterator browser_child_iter;
+       !browser_child_iter.Done(); ++browser_child_iter) {
+    const content::ChildProcessData& data = browser_child_iter.GetData();
+    if (base::GetProcId(data.handle) == pid) {
+      content::BrowserThread::GetTaskRunnerForThread(content::BrowserThread::UI)
+          ->PostTask(FROM_HERE,
+                     base::BindOnce(
+                         &ProfilingProcessHost::StartProfilingNonRendererChild,
+                         base::Unretained(this), data));
+      return;
+    }
+  }
+
+  DLOG(WARNING)
+      << "Attempt to start profiling failed as no process was found with pid: "
+      << pid;
+}
+
 void ProfilingProcessHost::MakeConnector(
     content::ServiceManagerConnection* connection) {
   connector_ = connection->GetConnector()->Clone();
@@ -685,7 +736,7 @@ bool ProfilingProcessHost::ShouldProfileNewRenderer(
 
   if (mode() == Mode::kAll) {
     return true;
-  } else if (mode() == Mode::kRendererSampling && !profiled_renderer_) {
+  } else if (mode() == Mode::kRendererSampling && profiled_renderers_.empty()) {
     if (always_sample_for_tests_) {
       return true;
     }
@@ -698,6 +749,23 @@ bool ProfilingProcessHost::ShouldProfileNewRenderer(
 }
 
 void ProfilingProcessHost::StartProfilingNonRendererChild(
+    const content::ChildProcessData& data) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  profiling::mojom::ProcessType process_type =
+      (data.process_type == content::ProcessType::PROCESS_TYPE_GPU)
+          ? profiling::mojom::ProcessType::GPU
+          : profiling::mojom::ProcessType::OTHER;
+
+  content::BrowserThread::GetTaskRunnerForThread(content::BrowserThread::IO)
+      ->PostTask(
+          FROM_HERE,
+          base::BindOnce(
+              &ProfilingProcessHost::StartProfilingNonRendererChildOnIOThread,
+              base::Unretained(this), data.id, base::GetProcId(data.handle),
+              process_type));
+}
+
+void ProfilingProcessHost::StartProfilingNonRendererChildOnIOThread(
     int child_process_id,
     base::ProcessId proc_id,
     profiling::mojom::ProcessType process_type) {
@@ -713,6 +781,22 @@ void ProfilingProcessHost::StartProfilingNonRendererChild(
   AddClientToProfilingService(client.take(), proc_id, process_type);
 }
 
+void ProfilingProcessHost::StartProfilingRenderer(
+    content::RenderProcessHost* host) {
+  profiled_renderers_.insert(host);
+
+  // Tell the child process to start profiling.
+  ProfilingClientBinder client(host);
+
+  content::BrowserThread::GetTaskRunnerForThread(content::BrowserThread::IO)
+      ->PostTask(
+          FROM_HERE,
+          base::BindOnce(&ProfilingProcessHost::AddClientToProfilingService,
+                         base::Unretained(this), client.take(),
+                         base::GetProcId(host->GetHandle()),
+                         profiling::mojom::ProcessType::RENDERER));
+}
+
 void ProfilingProcessHost::SetRendererSamplingAlwaysProfileForTest() {
   always_sample_for_tests_ = true;
 }

chrome/browser/profiling_host/profiling_process_host.h

@@ -137,6 +137,15 @@ class ProfilingProcessHost : public content::BrowserChildProcessObserver,
       TraceFinishedCallback callback,
       bool stop_immediately_after_heap_dump_for_tests);
 
+  // Returns the pids of all profiled processes. The callback is posted on the
+  // UI thread.
+  using GetProfiledPidsCallback =
+      base::OnceCallback<void(std::vector<base::ProcessId> pids)>;
+  void GetProfiledPids(GetProfiledPidsCallback callback);
+
+  // Starts profiling the process with the given id.
+  void StartProfiling(base::ProcessId pid);
+
  private:
   friend struct base::DefaultSingletonTraits<ProfilingProcessHost>;
   friend class BackgroundProfilingTriggersTest;
@@ -197,14 +206,20 @@ class ProfilingProcessHost : public content::BrowserChildProcessObserver,
   bool ShouldProfileNewRenderer(content::RenderProcessHost* renderer) const;
 
   // Sets up the profiling connection for the given child process.
-  void StartProfilingNonRendererChild(
+  void StartProfilingNonRendererChild(const content::ChildProcessData&);
+  void StartProfilingNonRendererChildOnIOThread(
       int child_process_id,
       base::ProcessId proc_id,
       profiling::mojom::ProcessType process_type);
 
+  void StartProfilingRenderer(content::RenderProcessHost* host);
+
   // SetRenderer.
   void SetRendererSamplingAlwaysProfileForTest();
 
+  void GetProfiledPidsOnIOThread(GetProfiledPidsCallback callback);
+  void StartProfilingPidOnIOThread(base::ProcessId pid);
+
   content::NotificationRegistrar registrar_;
   std::unique_ptr<service_manager::Connector> connector_;
   mojom::ProfilingServicePtr profiling_service_;
@@ -225,17 +240,21 @@ class ProfilingProcessHost : public content::BrowserChildProcessObserver,
   // Whether or not the profiling host is started.
   static bool has_started_;
 
-  // If in kRendererSampling mode, this is used to identify the currently
-  // profiled renderer. If no renderer is being profiled, this is set to
-  // nullptr.  This variable shouild only be accessed on the UI thread and
-  // the value should be considered opaque.
+  // This is used to identify the currently profiled renderers. Elements should
+  // only be accessed on the UI thread and their values should be considered
+  // opaque.
   //
-  // Semantically, the value must be something that identifies which
+  // Semantically, the elements must be something that identifies which
   // specific RenderProcess is being profiled. When the underlying RenderProcess
-  // goes away, this value needs to be reset to nullptr. The RenderProcessHost
+  // goes away, the element must be removed. The RenderProcessHost
   // pointer and the NOTIFICATION_RENDERER_PROCESS_CREATED notification can be
   // used to provide these semantics.
-  void* profiled_renderer_;
+  //
+  // This variable represents renderers that have been instructed to start
+  // profiling - it does not reflect whether a renderer is currently still being
+  // profiled. That information is only known by the profiling service, and for
+  // simplicity, it's easier to just track this variable in this process.
+  std::unordered_set<void*> profiled_renderers_;
 
   // For Tests. In kRendererSampling mode, overrides sampling to always profile
   // a renderer process if one is already not going.

chrome/browser/resources/memory_internals.js

@@ -16,6 +16,10 @@ function reportProcess(pid) {
   chrome.send('reportProcess', [pid]);
 }
 
+function startProfiling(pid) {
+  chrome.send('startProfiling', [pid]);
+}
+
 // celltype should either be "td" or "th". The contents of the |cols| will be
 // added as children of each table cell if they are non-null.
 function addListRow(table, celltype, cols) {
@@ -68,17 +72,25 @@ function returnProcessList(data) {
     null, document.createTextNode('Process ID'), document.createTextNode('Name')
   ]);
 
-  for (let proc of processes) {
-    let procId = proc[0];
-
-    let reportButton = document.createElement('button');
-    reportButton.innerText = '\uD83D\uDC1E Report';
-    reportButton.onclick = () => reportProcess(procId);
-
-    let procIdText = document.createTextNode(procId.toString());
-    let description = document.createTextNode(proc[1]);
-
-    addListRow(table, 'td', [reportButton, procIdText, description]);
+  for (const proc of processes) {
+    const procId = proc[0];
+
+    const procIdText = document.createTextNode(procId.toString());
+    const description = document.createTextNode(proc[1]);
+    const profiled = proc[2];
+
+    let button = null;
+    if (profiled) {
+      button = document.createElement('button');
+      button.innerText = '\uD83D\uDC1E Report';
+      button.onclick = () => reportProcess(procId);
+    } else {
+      button = document.createElement('button');
+      button.innerText = '\u2600 Start profiling';
+      button.onclick = () => startProfiling(procId);
+    }
+
+    addListRow(table, 'td', [button, procIdText, description]);
   }
 
   proclist.appendChild(table);

chrome/browser/ui/webui/memory_internals_ui.cc

@@ -131,12 +131,17 @@ class MemoryInternalsDOMHandler : public content::WebUIMessageHandler,
   // Callback for the "reportProcess" message.
   void HandleReportProcess(const base::ListValue* args);
 
+  // Callback for the "startProfiling" message.
+  void HandleStartProfiling(const base::ListValue* args);
+
  private:
   // Hops to the IO thread to enumerate child processes, and back to the UI
   // thread to fill in the renderer processes.
   static void GetChildProcessesOnIOThread(
       base::WeakPtr<MemoryInternalsDOMHandler> dom_handler);
-  void ReturnProcessListOnUIThread(std::vector<base::Value> children);
+  void GetProfiledPids(std::vector<base::Value> children);
+  void ReturnProcessListOnUIThread(std::vector<base::Value> children,
+                                   std::vector<base::ProcessId> profiled_pids);
 
   // SelectFileDialog::Listener implementation:
   void FileSelected(const base::FilePath& path,
@@ -177,6 +182,10 @@ void MemoryInternalsDOMHandler::RegisterMessages() {
       "reportProcess",
       base::BindRepeating(&MemoryInternalsDOMHandler::HandleReportProcess,
                           base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "startProfiling",
+      base::BindRepeating(&MemoryInternalsDOMHandler::HandleStartProfiling,
+                          base::Unretained(this)));
 }
 
 void MemoryInternalsDOMHandler::HandleRequestProcessList(
@@ -234,33 +243,48 @@ void MemoryInternalsDOMHandler::HandleReportProcess(
       "MEMLOG_MANUAL_TRIGGER");
 }
 
+void MemoryInternalsDOMHandler::HandleStartProfiling(
+    const base::ListValue* args) {
+  if (!args->is_list() || args->GetList().size() != 1)
+    return;
+
+  ProfilingProcessHost::GetInstance()->StartProfiling(
+      args->GetList()[0].GetInt());
+}
+
 void MemoryInternalsDOMHandler::GetChildProcessesOnIOThread(
     base::WeakPtr<MemoryInternalsDOMHandler> dom_handler) {
   std::vector<base::Value> result;
 
-  if (ProfilingProcessHost::GetCurrentMode() !=
-      ProfilingProcessHost::Mode::kNone) {
-    // Add child processes (this does not include renderers).
-    for (content::BrowserChildProcessHostIterator iter; !iter.Done(); ++iter) {
-      // Note that ChildProcessData.id is a child ID and not an OS PID.
-      const content::ChildProcessData& data = iter.GetData();
-
-      if (ProfilingProcessHost::GetInstance()->ShouldProfileProcessType(
-              data.process_type)) {
-        result.push_back(MakeProcessInfo(base::GetProcId(data.handle),
-                                         GetChildDescription(data)));
-      }
+  // The only non-renderer child process that currently supports out-of-process
+  // heap profiling is GPU.
+  for (content::BrowserChildProcessHostIterator iter; !iter.Done(); ++iter) {
+    // Note that ChildProcessData.id is a child ID and not an OS PID.
+    const content::ChildProcessData& data = iter.GetData();
+
+    if (data.process_type == content::PROCESS_TYPE_GPU) {
+      result.push_back(MakeProcessInfo(base::GetProcId(data.handle),
+                                       GetChildDescription(data)));
     }
   }
 
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&MemoryInternalsDOMHandler::GetProfiledPids, dom_handler,
+                     std::move(result)));
+}
+
+void MemoryInternalsDOMHandler::GetProfiledPids(
+    std::vector<base::Value> children) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  ProfilingProcessHost::GetInstance()->GetProfiledPids(
       base::BindOnce(&MemoryInternalsDOMHandler::ReturnProcessListOnUIThread,
-                     dom_handler, std::move(result)));
+                     weak_factory_.GetWeakPtr(), std::move(children)));
 }
 
 void MemoryInternalsDOMHandler::ReturnProcessListOnUIThread(
-    std::vector<base::Value> children) {
+    std::vector<base::Value> children,
+    std::vector<base::ProcessId> profiled_pids) {
   // This function will be called with the child processes that are not
   // renderers. It will fill in the browser and renderer processes on the UI
   // thread (RenderProcessHost is UI-thread only) and return the full list.
@@ -268,26 +292,19 @@ void MemoryInternalsDOMHandler::ReturnProcessListOnUIThread(
   std::vector<base::Value>& process_list = process_list_value.GetList();
 
   // Add browser process.
-  if (ProfilingProcessHost::GetInstance()->ShouldProfileProcessType(
-          content::ProcessType::PROCESS_TYPE_BROWSER)) {
-    process_list.push_back(
-        MakeProcessInfo(base::GetCurrentProcId(), "Browser"));
-  }
+  process_list.push_back(MakeProcessInfo(base::GetCurrentProcId(), "Browser"));
 
   // Append renderer processes.
-  if (ProfilingProcessHost::GetInstance()->ShouldProfileProcessType(
-          content::ProcessType::PROCESS_TYPE_RENDERER)) {
-    auto iter = content::RenderProcessHost::AllHostsIterator();
-    while (!iter.IsAtEnd()) {
-      base::ProcessHandle renderer_handle = iter.GetCurrentValue()->GetHandle();
-      base::ProcessId renderer_pid = base::GetProcId(renderer_handle);
-      if (renderer_pid != 0) {
-        // TODO(brettw) make a better description of the process, maybe see
-        // what TaskManager does to get the page title.
-        process_list.push_back(MakeProcessInfo(renderer_pid, "Renderer"));
-      }
-      iter.Advance();
+  auto iter = content::RenderProcessHost::AllHostsIterator();
+  while (!iter.IsAtEnd()) {
+    base::ProcessHandle renderer_handle = iter.GetCurrentValue()->GetHandle();
+    base::ProcessId renderer_pid = base::GetProcId(renderer_handle);
+    if (renderer_pid != 0) {
+      // TODO(brettw) make a better description of the process, maybe see
+      // what TaskManager does to get the page title.
+      process_list.push_back(MakeProcessInfo(renderer_pid, "Renderer"));
     }
+    iter.Advance();
   }
 
   // Append all child processes collected on the IO thread.
@@ -295,6 +312,21 @@ void MemoryInternalsDOMHandler::ReturnProcessListOnUIThread(
                       std::make_move_iterator(std::begin(children)),
                       std::make_move_iterator(std::end(children)));
 
+  // Sort profiled_pids to allow binary_search in the loop.
+  std::sort(profiled_pids.begin(), profiled_pids.end());
+
+  // Append whether each process is being profiled.
+  for (base::Value& value : process_list) {
+    std::vector<base::Value>& value_as_list = value.GetList();
+    DCHECK_EQ(value_as_list.size(), 2u);
+
+    base::ProcessId pid =
+        static_cast<base::ProcessId>(value_as_list[0].GetInt());
+    bool is_profiled =
+        std::binary_search(profiled_pids.begin(), profiled_pids.end(), pid);
+    value_as_list.push_back(base::Value(is_profiled));
+  }
+
   // Pass the results in a dictionary.
   base::Value result(base::Value::Type::DICTIONARY);
   result.SetKey("message", base::Value(GetMessageString()));

chrome/common/profiling/profiling_client.cc

@@ -20,7 +20,8 @@ namespace {
 const int kTimeoutDurationMs = 10000;
 }  // namespace
 
-ProfilingClient::ProfilingClient() : binding_(this) {}
+ProfilingClient::ProfilingClient()
+    : binding_(this), started_profiling_(false) {}
 
 ProfilingClient::~ProfilingClient() {
   StopAllocatorShimDangerous();
@@ -49,6 +50,10 @@ void ProfilingClient::BindToInterface(mojom::ProfilingClientRequest request) {
 }
 
 void ProfilingClient::StartProfiling(mojo::ScopedHandle memlog_sender_pipe) {
+  if (started_profiling_)
+    return;
+  started_profiling_ = true;
+
   base::PlatformFile platform_file;
   CHECK_EQ(MOJO_RESULT_OK, mojo::UnwrapPlatformFile(
                                std::move(memlog_sender_pipe), &platform_file));

chrome/common/profiling/profiling_client.h

@@ -38,6 +38,8 @@ class ProfilingClient : public mojom::ProfilingClient {
   // The most recent client request is bound and kept alive.
   mojo::Binding<mojom::ProfilingClient> binding_;
 
+  bool started_profiling_;
+
   std::unique_ptr<MemlogSenderPipe> memlog_sender_pipe_;
 };
 

chrome/common/profiling/profiling_service.mojom

@@ -47,4 +47,7 @@ interface ProfilingService {
   // |buffers|.
   DumpProcessesForTracing() =>
       (array<SharedBufferWithSize> buffers);
+
+  // Returns the pids of all profiled processes.
+  GetProfiledPids() => (array<mojo.common.mojom.ProcessId> pids);
 };

chrome/profiling/memlog_connection_manager.cc

@@ -113,8 +113,19 @@ void MemlogConnectionManager::OnNewConnection(
     mojom::ProcessType process_type) {
   base::AutoLock lock(connections_lock_);
 
-  // Shouldn't be asked to profile a process more than once.
-  DCHECK(connections_.find(pid) == connections_.end());
+  // Attempting to start profiling on an already profiled processs should have
+  // no effect.
+  if (connections_.find(pid) != connections_.end())
+    return;
+
+  // It's theoretically possible that we started profiling a process, the
+  // profiling was stopped [e.g. by hitting the 10-s timeout], and then we tried
+  // to start profiling again. The ProfilingClient will refuse to start again.
+  // But the MemlogConnectionManager will not be able to distinguish this
+  // never-started ProfilingClient from a brand new ProfilingClient that happens
+  // to share the same pid. This is a rare condition which should only happen
+  // when the user is attempting to manually start profiling for processes, so
+  // we ignore this edge case.
 
   base::PlatformFile receiver_handle;
   CHECK_EQ(MOJO_RESULT_OK, mojo::UnwrapPlatformFile(
@@ -151,6 +162,16 @@ void MemlogConnectionManager::OnNewConnection(
   connections_[pid] = std::move(connection);  // Transfers ownership.
 }
 
+std::vector<base::ProcessId> MemlogConnectionManager::GetConnectionPids() {
+  base::AutoLock lock(connections_lock_);
+  std::vector<base::ProcessId> results;
+  results.reserve(connections_.size());
+  for (const auto& pair : connections_) {
+    results.push_back(pair.first);
+  }
+  return results;
+}
+
 void MemlogConnectionManager::OnConnectionComplete(base::ProcessId pid) {
   base::AutoLock lock(connections_lock_);
   auto found = connections_.find(pid);

chrome/profiling/memlog_connection_manager.h

@@ -73,6 +73,8 @@ class MemlogConnectionManager {
                        mojo::ScopedHandle receiver_pipe_end,
                        mojom::ProcessType process_type);
 
+  std::vector<base::ProcessId> GetConnectionPids();
+
  private:
   struct Connection;
   struct DumpProcessesForTracingTracking;

chrome/profiling/profiling_service.cc

@@ -80,6 +80,10 @@ void ProfilingService::DumpProcessesForTracing(
           weak_factory_.GetWeakPtr(), base::Passed(&callback)));
 }
 
+void ProfilingService::GetProfiledPids(GetProfiledPidsCallback callback) {
+  std::move(callback).Run(connection_manager_.GetConnectionPids());
+}
+
 void ProfilingService::OnGetVmRegionsCompleteForDumpProcessesForTracing(
     mojom::ProfilingService::DumpProcessesForTracingCallback callback,
     bool success,

chrome/profiling/profiling_service.h

@@ -48,6 +48,7 @@ class ProfilingService : public service_manager::Service,
                           mojom::ProcessType process_type) override;
   void DumpProcessesForTracing(
       DumpProcessesForTracingCallback callback) override;
+  void GetProfiledPids(GetProfiledPidsCallback callback) override;
 
  private:
   void MaybeRequestQuitDelayed();