[heap profiler] Move heap profiler earlier in the startup sequence.

The earlier we start heap profiler the more startup allocations it
will be able to record. It is now started right after field trial flags
become available.

Bug: 1007491
Change-Id: Ia25d8c9134f2e17f135d5731a0d5ed0b35905ffe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1827476Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701410}

chrome/app/chrome_main_delegate.cc

@@ -37,6 +37,7 @@
 #include "chrome/common/chrome_result_codes.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/crash_keys.h"
+#include "chrome/common/heap_profiler_controller.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/gpu/chrome_content_gpu_client.h"
@@ -568,16 +569,16 @@ bool ChromeMainDelegate::ShouldCreateFeatureList() {
 #endif
 
 void ChromeMainDelegate::PostFieldTrialInitialization() {
+  std::string process_type =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kProcessType);
+  bool is_browser_process = process_type.empty();
+
 #if BUILDFLAG(ENABLE_GWP_ASAN_MALLOC)
   {
     version_info::Channel channel = chrome::GetChannel();
     bool is_canary_dev = (channel == version_info::Channel::CANARY ||
                           channel == version_info::Channel::DEV);
-    const base::CommandLine& command_line =
-        *base::CommandLine::ForCurrentProcess();
-    std::string process_type =
-        command_line.GetSwitchValueASCII(switches::kProcessType);
-    bool is_browser_process = process_type.empty();
     gwp_asan::EnableForMalloc(is_canary_dev || is_browser_process,
                               process_type.c_str());
   }
@@ -588,13 +589,16 @@ void ChromeMainDelegate::PostFieldTrialInitialization() {
     version_info::Channel channel = chrome::GetChannel();
     bool is_canary_dev = (channel == version_info::Channel::CANARY ||
                           channel == version_info::Channel::DEV);
-    const base::CommandLine& command_line =
-        *base::CommandLine::ForCurrentProcess();
-    std::string process_type =
-        command_line.GetSwitchValueASCII(switches::kProcessType);
     gwp_asan::EnableForPartitionAlloc(is_canary_dev, process_type.c_str());
   }
 #endif
+
+  // Start heap profiling as early as possible so it can start recording
+  // memory allocations.
+  if (is_browser_process) {
+    heap_profiler_controller_ = std::make_unique<HeapProfilerController>();
+    heap_profiler_controller_->Start();
+  }
 }
 
 bool ChromeMainDelegate::BasicStartupComplete(int* exit_code) {

chrome/app/chrome_main_delegate.h

@@ -27,6 +27,7 @@ class TracingSamplerProfiler;
 }
 
 class ChromeContentBrowserClient;
+class HeapProfilerController;
 
 // Chrome implementation of ContentMainDelegate.
 class ChromeMainDelegate : public content::ContentMainDelegate {
@@ -89,6 +90,10 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
 
   std::unique_ptr<tracing::TracingSamplerProfiler> tracing_sampler_profiler_;
 
+  // The controller schedules UMA heap profiles collections and forwarding down
+  // the reporting pipeline.
+  std::unique_ptr<HeapProfilerController> heap_profiler_controller_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeMainDelegate);
 };
 

chrome/browser/chrome_browser_main.cc

@@ -116,7 +116,6 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/crash_keys.h"
 #include "chrome/common/env_vars.h"
-#include "chrome/common/heap_profiler_controller.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/media/media_resource_provider.h"
 #include "chrome/common/net/net_resource_provider.h"
@@ -648,7 +647,6 @@ ChromeBrowserMainParts::ChromeBrowserMainParts(
     : parameters_(parameters),
       parsed_command_line_(parameters.command_line),
       result_code_(service_manager::RESULT_CODE_NORMAL_EXIT),
-      heap_profiler_controller_(std::make_unique<HeapProfilerController>()),
       should_call_pre_main_loop_start_startup_on_variations_service_(
           !parameters.ui_task),
       profile_(NULL),
@@ -873,8 +871,6 @@ void ChromeBrowserMainParts::PostMainMessageLoopStart() {
 
   ThreadProfiler::SetMainThreadTaskRunner(base::ThreadTaskRunnerHandle::Get());
 
-  heap_profiler_controller_->Start();
-
   system_monitor_ = performance_monitor::SystemMonitor::Create();
 
   // TODO(sebmarchand): Allow this to be created earlier if startup tracing is

chrome/browser/chrome_browser_main.h

@@ -24,7 +24,6 @@
 class BrowserProcessImpl;
 class ChromeBrowserMainExtraParts;
 class StartupData;
-class HeapProfilerController;
 class PrefService;
 class Profile;
 class StartupBrowserCreator;
@@ -154,10 +153,6 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
   // Parts are deleted in the inverse order they are added.
   std::vector<ChromeBrowserMainExtraParts*> chrome_extra_parts_;
 
-  // The controller schedules UMA heap profiles collections and forwarding down
-  // the reporting pipeline.
-  std::unique_ptr<HeapProfilerController> heap_profiler_controller_;
-
   // The system monitor instance, used by some subsystems to collect the system
   // metrics they need.
   std::unique_ptr<performance_monitor::SystemMonitor> system_monitor_;

chrome/common/heap_profiler_controller.cc

@@ -43,15 +43,16 @@ HeapProfilerController::~HeapProfilerController() {
 }
 
 void HeapProfilerController::Start() {
-  if (base::FeatureList::IsEnabled(
+  if (!base::FeatureList::IsEnabled(
           metrics::CallStackProfileMetricsProvider::kHeapProfilerReporting)) {
-    int sampling_rate = base::GetFieldTrialParamByFeatureAsInt(
-        metrics::CallStackProfileMetricsProvider::kHeapProfilerReporting,
-        kHeapProfilerSamplingRate, 0);
-    if (sampling_rate > 0)
-      base::SamplingHeapProfiler::Get()->SetSamplingInterval(sampling_rate);
-    base::SamplingHeapProfiler::Get()->Start();
+    return;
   }
+  int sampling_rate = base::GetFieldTrialParamByFeatureAsInt(
+      metrics::CallStackProfileMetricsProvider::kHeapProfilerReporting,
+      kHeapProfilerSamplingRate, 0);
+  if (sampling_rate > 0)
+    base::SamplingHeapProfiler::Get()->SetSamplingInterval(sampling_rate);
+  base::SamplingHeapProfiler::Get()->Start();
   ScheduleNextSnapshot(stopped_);
 }
 

chrome/common/heap_profiler_controller_unittest.cc

@@ -6,9 +6,11 @@
 
 #include "base/sampling_heap_profiler/sampling_heap_profiler.h"
 #include "base/test/bind_test_util.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "build/build_config.h"
 #include "components/metrics/call_stack_profile_builder.h"
+#include "components/metrics/call_stack_profile_metrics_provider.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/metrics_proto/sampled_profile.pb.h"
 
@@ -67,13 +69,13 @@ TEST_F(HeapProfilerControllerTest, ProfileCollectionsScheduler) {
       controller.reset();
   };
 
-  base::SamplingHeapProfiler::Init();
-  auto* profiler = base::SamplingHeapProfiler::Get();
-  profiler->SetSamplingInterval(1024);
-  profiler->Start();
-
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      metrics::CallStackProfileMetricsProvider::kHeapProfilerReporting);
   metrics::CallStackProfileBuilder::SetBrowserProcessReceiverCallback(
       base::BindLambdaForTesting(check_profile));
+  base::SamplingHeapProfiler::Get()->SetSamplingInterval(1024);
+
   controller->Start();
 
   auto* sampler = base::PoissonAllocationSampler::Get();