[Reland #1] Remove "--enable-heap-profiling"

The original CL was reverted because --memlog did not support Android Webview,
but --enable-heap-profiling did. Now that they both support Android Webview,
there is no reason to keep "--enable-heap-profiling" anymore.

> The flag is deprecated and replaced by --memlog and co. See
> https://chromium.googlesource.com/chromium/src/+/lkcr/docs/memory/debugging_memory_issues.md#taking-a-heap-dump
> for more details.
>
> Bug: 758739
> Change-Id: I8f7ac614298334a90103cd5b4c9667468d662845
> Reviewed-on: https://chromium-review.googlesource.com/970852
> Reviewed-by: Tommy Martino <tmartino@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
> Reviewed-by: Primiano Tucci <primiano@chromium.org>
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
> Commit-Queue: Erik Chen <erikchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#545929}

Change-Id: I2f503b6f685aa6030ca27b60fa2e578aa9766b94
TBR: dskiba@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1022533
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552529}

base/base_switches.cc

@@ -24,25 +24,6 @@ const char kEnableCrashReporter[]           = "enable-crash-reporter";
 // Comma-separated list of feature names to enable. See also kDisableFeatures.
 const char kEnableFeatures[] = "enable-features";
 
-// Makes memory allocators keep track of their allocations and context, so a
-// detailed breakdown of memory usage can be presented in chrome://tracing when
-// the memory-infra category is enabled.
-const char kEnableHeapProfiling[]           = "enable-heap-profiling";
-
-// Report pseudo allocation traces. Pseudo traces are derived from currently
-// active trace events.
-const char kEnableHeapProfilingModePseudo[] = "";
-
-// Report native (walk the stack) allocation traces. By default pseudo stacks
-// derived from trace events are reported.
-const char kEnableHeapProfilingModeNative[] = "native";
-
-// Report per-task heap usage and churn in the task profiler.
-// Does not keep track of individual allocations unlike the default and native
-// mode. Keeps only track of summarized churn stats in the task profiler
-// (chrome://profiler).
-const char kEnableHeapProfilingTaskProfiler[] = "task-profiler";
-
 // Generates full memory crash dump.
 const char kFullMemoryCrashReport[]         = "full-memory-crash-report";
 

base/base_switches.h

@@ -17,10 +17,6 @@ extern const char kDisableFeatures[];
 extern const char kDisableLowEndDeviceMode[];
 extern const char kEnableCrashReporter[];
 extern const char kEnableFeatures[];
-extern const char kEnableHeapProfiling[];
-extern const char kEnableHeapProfilingModePseudo[];
-extern const char kEnableHeapProfilingModeNative[];
-extern const char kEnableHeapProfilingTaskProfiler[];
 extern const char kEnableLowEndDeviceMode[];
 extern const char kForceFieldTrials[];
 extern const char kFullMemoryCrashReport[];

base/trace_event/heap_profiler_allocation_context_tracker.h

@@ -15,11 +15,25 @@
 namespace base {
 namespace trace_event {
 
-// The allocation context tracker keeps track of thread-local context for heap
-// profiling. It includes a pseudo stack of trace events. On every allocation
-// the tracker provides a snapshot of its context in the form of an
-// |AllocationContext| that is to be stored together with the allocation
-// details.
+// AllocationContextTracker is a thread-local object. Its main purpose is to
+// keep track of a pseudo stack of trace events. Chrome has been instrumented
+// with lots of `TRACE_EVENT` macros. These trace events push their name to a
+// thread-local stack when they go into scope, and pop when they go out of
+// scope, if all of the following conditions have been met:
+//
+//  * A trace is being recorded.
+//  * The category of the event is enabled in the trace config.
+//  * Heap profiling is enabled (with the `--enable-heap-profiling` flag).
+//
+// This means that allocations that occur before tracing is started will not
+// have backtrace information in their context.
+//
+// AllocationContextTracker also keeps track of some thread state not related to
+// trace events. See |AllocationContext|.
+//
+// A thread-local instance of the context tracker is initialized lazily when it
+// is first accessed. This might be because a trace event pushed or popped, or
+// because `GetContextSnapshot()` was called when an allocation occurred
 class BASE_EXPORT AllocationContextTracker {
  public:
   enum class CaptureMode : int32_t {

base/trace_event/memory_dump_manager.cc

@@ -206,44 +206,6 @@ MemoryDumpManager::~MemoryDumpManager() {
   g_memory_dump_manager_for_testing = nullptr;
 }
 
-// static
-HeapProfilingMode MemoryDumpManager::GetHeapProfilingModeFromCommandLine() {
-  if (!CommandLine::InitializedForCurrentProcess() ||
-      !CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableHeapProfiling)) {
-    return kHeapProfilingModeDisabled;
-  }
-#if BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
-  std::string profiling_mode =
-      CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          switches::kEnableHeapProfiling);
-  if (profiling_mode == switches::kEnableHeapProfilingTaskProfiler)
-    return kHeapProfilingModeTaskProfiler;
-  if (profiling_mode == switches::kEnableHeapProfilingModePseudo)
-    return kHeapProfilingModePseudo;
-  if (profiling_mode == switches::kEnableHeapProfilingModeNative)
-    return kHeapProfilingModeNative;
-#endif  // BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
-  return kHeapProfilingModeInvalid;
-}
-
-void MemoryDumpManager::EnableHeapProfilingIfNeeded() {
-#if BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
-  HeapProfilingMode profiling_mode = GetHeapProfilingModeFromCommandLine();
-  if (IsHeapProfilingModeEnabled(profiling_mode)) {
-    EnableHeapProfiling(profiling_mode);
-  } else {
-    if (profiling_mode == kHeapProfilingModeInvalid) {
-      // Heap profiling is misconfigured, disable it permanently.
-      EnableHeapProfiling(kHeapProfilingModeDisabled);
-    }
-  }
-#else
-  // Heap profiling is unsupported, disable it permanently.
-  EnableHeapProfiling(kHeapProfilingModeDisabled);
-#endif  // BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
-}
-
 bool MemoryDumpManager::EnableHeapProfiling(HeapProfilingMode profiling_mode) {
   AutoLock lock(lock_);
 #if BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
@@ -341,7 +303,6 @@ void MemoryDumpManager::Initialize(
     request_dump_function_ = request_dump_function;
     is_coordinator_ = is_coordinator;
   }
-  EnableHeapProfilingIfNeeded();
 
 // Enable the core dump providers.
 #if defined(MALLOC_MEMORY_TRACING_SUPPORTED)

base/trace_event/memory_dump_manager.h

@@ -131,15 +131,6 @@ class BASE_EXPORT MemoryDumpManager {
   void CreateProcessDump(const MemoryDumpRequestArgs& args,
                          const ProcessMemoryDumpCallback& callback);
 
-  // Returns the heap profiling mode configured on the command-line, if any.
-  // If heap profiling is configured but not supported by this binary, or if an
-  // invalid mode is specified, then kHeapProfilingInvalid is returned.
-  static HeapProfilingMode GetHeapProfilingModeFromCommandLine();
-
-  // Enable heap profiling if supported, and kEnableHeapProfiling command line
-  // is specified.
-  void EnableHeapProfilingIfNeeded();
-
   // Enable heap profiling with specified |profiling_mode|.
   // Use kHeapProfilingModeDisabled to disable, but it can't be re-enabled then.
   // Returns true if mode has been *changed* to the desired |profiling_mode|.

base/trace_event/memory_dump_manager_unittest.cc

@@ -961,54 +961,6 @@ TEST_F(MemoryDumpManagerTestAsCoordinator, EnableHeapProfilingDisableDisabled) {
 }
 #endif  //  BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
 
-TEST_F(MemoryDumpManagerTest, EnableHeapProfilingIfNeeded) {
-  MockMemoryDumpProvider mdp1;
-  MemoryDumpProvider::Options supported_options;
-  supported_options.supports_heap_profiling = true;
-  RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get(), supported_options);
-
-  // Should be noop.
-  mdm_->EnableHeapProfilingIfNeeded();
-  ASSERT_EQ(AllocationContextTracker::CaptureMode::DISABLED,
-            AllocationContextTracker::capture_mode());
-  mdm_->EnableHeapProfilingIfNeeded();
-  ASSERT_EQ(AllocationContextTracker::CaptureMode::DISABLED,
-            AllocationContextTracker::capture_mode());
-
-#if BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
-  testing::InSequence sequence;
-  EXPECT_CALL(mdp1, OnHeapProfilingEnabled(true)).Times(1);
-  EXPECT_CALL(mdp1, OnHeapProfilingEnabled(false)).Times(1);
-
-  CommandLine* cmdline = CommandLine::ForCurrentProcess();
-  cmdline->AppendSwitchASCII(switches::kEnableHeapProfiling, "");
-  mdm_->EnableHeapProfilingIfNeeded();
-  RunLoop().RunUntilIdle();
-  ASSERT_EQ(AllocationContextTracker::CaptureMode::PSEUDO_STACK,
-            AllocationContextTracker::capture_mode());
-  EXPECT_TRUE(mdm_->EnableHeapProfiling(kHeapProfilingModeDisabled));
-  RunLoop().RunUntilIdle();
-  ASSERT_EQ(AllocationContextTracker::CaptureMode::DISABLED,
-            AllocationContextTracker::capture_mode());
-  EXPECT_FALSE(mdm_->EnableHeapProfiling(kHeapProfilingModeBackground));
-  ASSERT_EQ(AllocationContextTracker::CaptureMode::DISABLED,
-            AllocationContextTracker::capture_mode());
-#endif  //  BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
-}
-
-TEST_F(MemoryDumpManagerTest, EnableHeapProfilingIfNeededUnsupported) {
-#if BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
-  ASSERT_EQ(mdm_->GetHeapProfilingMode(), kHeapProfilingModeDisabled);
-  CommandLine* cmdline = CommandLine::ForCurrentProcess();
-  cmdline->AppendSwitchASCII(switches::kEnableHeapProfiling, "unsupported");
-  mdm_->EnableHeapProfilingIfNeeded();
-  EXPECT_EQ(mdm_->GetHeapProfilingMode(), kHeapProfilingModeInvalid);
-#else
-  mdm_->EnableHeapProfilingIfNeeded();
-  EXPECT_EQ(mdm_->GetHeapProfilingMode(), kHeapProfilingModeInvalid);
-#endif  //  BUILDFLAG(USE_ALLOCATOR_SHIM) && !defined(OS_NACL)
-}
-
 // Mock MDP class that tests if the number of OnMemoryDump() calls are expected.
 // It is implemented without gmocks since EXPECT_CALL implementation is slow
 // when there are 1000s of instances, as required in

chrome/app/generated_resources.grd

@@ -4884,11 +4884,6 @@ Keep your key file in a safe place. You will need it to create new versions of y
         usage statistics
       </message>
 
-      <!-- Unimplemented Flags Infobar-->
-      <message name="IDS_UNIMPLEMENTED_FLAGS_WARNING_MESSAGE" desc="Message shown when a command-line flag is used that is not implemented by this build. [Keep it short so it fits in the infobar.]">
-        <ph name="BAD_FLAG">$1<ex>--enable-heap-profiling</ex></ph> is not implemented in this build
-      </message>
-
       <!-- Bad Flags Infobar-->
       <message name="IDS_BAD_FLAGS_WARNING_MESSAGE" desc="Message shown when an unsupported command-line flag is used. [Keep it short so it fits in the infobar.]">
         You are using an unsupported command-line flag: <ph name="BAD_FLAG">$1<ex>--no-sandbox</ex></ph>. Stability and security will suffer.

chrome/browser/about_flags.cc

@@ -967,16 +967,6 @@ const FeatureEntry::FeatureVariation kDataReductionMainMenuFeatureVariations[] =
       arraysize(kPersistentMenuItemEnabled), nullptr}};
 #endif  // OS_ANDROID
 
-const FeatureEntry::Choice kEnableHeapProfilingChoices[] = {
-    {flags_ui::kGenericExperimentChoiceDisabled, "", ""},
-    {flag_descriptions::kEnableHeapProfilingModePseudo,
-     switches::kEnableHeapProfiling, switches::kEnableHeapProfilingModePseudo},
-    {flag_descriptions::kEnableHeapProfilingModeNative,
-     switches::kEnableHeapProfiling, switches::kEnableHeapProfilingModeNative},
-    {flag_descriptions::kEnableHeapProfilingTaskProfiler,
-     switches::kEnableHeapProfiling,
-     switches::kEnableHeapProfilingTaskProfiler}};
-
 const FeatureEntry::Choice kEnableOutOfProcessHeapProfilingChoices[] = {
     {flags_ui::kGenericExperimentChoiceDisabled, "", ""},
     {flag_descriptions::kEnableOutOfProcessHeapProfilingModeMinimal,
@@ -3196,10 +3186,6 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kSamplingHeapProfilerDescription, kOsAll,
      SINGLE_VALUE_TYPE(switches::kSamplingHeapProfiler)},
 
-    {"enable-heap-profiling", flag_descriptions::kEnableHeapProfilingName,
-     flag_descriptions::kEnableHeapProfilingDescription, kOsAll,
-     MULTI_VALUE_TYPE(kEnableHeapProfilingChoices)},
-
     {"memlog", flag_descriptions::kEnableOutOfProcessHeapProfilingName,
      flag_descriptions::kEnableOutOfProcessHeapProfilingDescription, kOsAll,
      MULTI_VALUE_TYPE(kEnableOutOfProcessHeapProfilingChoices)},

chrome/browser/flag_descriptions.cc

@@ -429,12 +429,6 @@ const char kEnableHDRName[] = "HDR mode";
 const char kEnableHDRDescription[] =
     "Enables HDR support on compatible displays.";
 
-const char kEnableHeapProfilingName[] = "Heap profiling";
-const char kEnableHeapProfilingDescription[] = "Enables heap profiling.";
-const char kEnableHeapProfilingModePseudo[] = "Enabled (pseudo mode)";
-const char kEnableHeapProfilingModeNative[] = "Enabled (native mode)";
-const char kEnableHeapProfilingTaskProfiler[] = "Enabled (task mode)";
-
 const char kEnableHttpFormWarningName[] =
     "Show in-form warnings for sensitive fields when the top-level page is not "
     "HTTPS";

chrome/browser/flag_descriptions.h

@@ -295,12 +295,6 @@ extern const char kEnableGenericSensorExtraClassesDescription[];
 extern const char kEnableHDRName[];
 extern const char kEnableHDRDescription[];
 
-extern const char kEnableHeapProfilingName[];
-extern const char kEnableHeapProfilingDescription[];
-extern const char kEnableHeapProfilingModePseudo[];
-extern const char kEnableHeapProfilingModeNative[];
-extern const char kEnableHeapProfilingTaskProfiler[];
-
 extern const char kEnableHttpFormWarningName[];
 extern const char kEnableHttpFormWarningDescription[];
 

chrome/browser/ui/startup/bad_flags_prompt.cc

@@ -141,25 +141,6 @@ void ShowBadFlagsPrompt(content::WebContents* web_contents) {
 // On Android, ShowBadFlagsPrompt doesn't show the warning notification
 // for flags which are not available in about:flags.
 #if !defined(OS_ANDROID)
-  // Flags only available in specific builds, for which to display a warning
-  // "the flag is not implemented in this build", if necessary.
-  struct {
-    const char* name;
-    bool is_invalid;
-  } conditional_flags[] = {
-      {switches::kEnableHeapProfiling,
-       base::trace_event::MemoryDumpManager::
-               GetHeapProfilingModeFromCommandLine() ==
-           base::trace_event::kHeapProfilingModeInvalid},
-  };
-  for (auto conditional_flag : conditional_flags) {
-    if (conditional_flag.is_invalid) {
-      ShowBadFlagsInfoBar(web_contents, IDS_UNIMPLEMENTED_FLAGS_WARNING_MESSAGE,
-                          conditional_flag.name);
-      return;
-    }
-  }
-
   for (const char* flag : kBadFlags) {
     if (base::CommandLine::ForCurrentProcess()->HasSwitch(flag)) {
       ShowBadFlagsInfoBar(web_contents, IDS_BAD_FLAGS_WARNING_MESSAGE, flag);

components/services/heap_profiling/public/cpp/settings.cc

@@ -42,17 +42,15 @@ bool RecordAllAllocationsForStartup() {
 Mode GetModeForStartup() {
   const base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();
 #if BUILDFLAG(USE_ALLOCATOR_SHIM)
+  if (cmdline->HasSwitch("enable-heap-profiling")) {
+    LOG(ERROR) << "--enable-heap-profiling is no longer supported. Use "
+                  "--memlog instead. See documentation at "
+                  "docs/memory/debugging_memory_issues.md";
+    return Mode::kNone;
+  }
+
   if (cmdline->HasSwitch(kMemlog) ||
       base::FeatureList::IsEnabled(kOOPHeapProfilingFeature)) {
-    if (cmdline->HasSwitch(switches::kEnableHeapProfiling)) {
-      // PartitionAlloc doesn't support chained allocation hooks so we can't
-      // run both heap profilers at the same time.
-      LOG(ERROR) << "--" << switches::kEnableHeapProfiling
-                 << " specified with --" << kMemlog
-                 << "which are not compatible. Memlog will be disabled.";
-      return Mode::kNone;
-    }
-
     std::string mode;
     // Respect the commandline switch above the field trial.
     if (cmdline->HasSwitch(kMemlog)) {

components/tracing/common/trace_startup.cc

@@ -22,10 +22,6 @@ void EnableStartupTracingIfNeeded() {
   // https://crbug.com/764357
   base::trace_event::TraceLog::GetInstance();
 
-  // Enables heap profiling if "--enable-heap-profiling" flag is passed.
-  base::trace_event::MemoryDumpManager::GetInstance()
-      ->EnableHeapProfilingIfNeeded();
-
   if (command_line.HasSwitch(switches::kTraceStartup)) {
     base::trace_event::TraceConfig trace_config(
         command_line.GetSwitchValueASCII(switches::kTraceStartup),

content/app/content_main_runner.cc

@@ -279,7 +279,6 @@ pid_t LaunchZygoteHelper(base::CommandLine* cmd_line,
   // to the zygote/renderers.
   static const char* const kForwardSwitches[] = {
       switches::kAndroidFontsPath, switches::kClearKeyCdmPathForTesting,
-      switches::kEnableHeapProfiling,
       switches::kEnableLogging,  // Support, e.g., --enable-logging=stderr.
       // Need to tell the zygote that it is headless so that we don't try to use
       // the wrong type of main delegate.

content/browser/browser_main_loop.cc

@@ -160,7 +160,6 @@
 #endif
 
 #if defined(OS_MACOSX)
-#include "base/allocator/allocator_interception_mac.h"
 #include "base/memory/memory_pressure_monitor_mac.h"
 #include "content/browser/cocoa/system_hotkey_helper_mac.h"
 #include "content/browser/mach_broker_mac.h"
@@ -774,14 +773,6 @@ int BrowserMainLoop::PreCreateThreads() {
 
   InitializeMemoryManagementComponent();
 
-#if defined(OS_MACOSX)
-  if (base::CommandLine::InitializedForCurrentProcess() &&
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableHeapProfiling)) {
-    base::allocator::PeriodicallyShimNewMallocZones();
-  }
-#endif
-
 #if BUILDFLAG(ENABLE_PLUGINS)
   // Prior to any processing happening on the IO thread, we create the
   // plugin service as it is predominantly used from the IO thread,

content/browser/gpu/gpu_process_host.cc

@@ -142,7 +142,6 @@ static const char* const kSwitchNames[] = {
     switches::kEnableAcceleratedVpxDecode,
 #endif
     switches::kEnableGpuRasterization,
-    switches::kEnableHeapProfiling,
     switches::kEnableLogging,
     switches::kEnableOOPRasterization,
     switches::kEnableVizDevTools,

content/browser/renderer_host/render_process_host_impl.cc

@@ -2698,7 +2698,6 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
     switches::kDomAutomationController,
     switches::kEnableAutomation,
     switches::kEnableExperimentalWebPlatformFeatures,
-    switches::kEnableHeapProfiling,
     switches::kEnableGPUClientLogging,
     switches::kEnableGpuClientTracing,
     switches::kEnableGpuMemoryBufferVideoFrames,

content/child/child_thread_impl.cc

@@ -74,10 +74,6 @@
 #include "content/public/common/content_descriptors.h"
 #endif
 
-#if defined(OS_MACOSX)
-#include "base/allocator/allocator_interception_mac.h"
-#endif
-
 namespace content {
 namespace {
 
@@ -562,14 +558,6 @@ void ChildThreadImpl::Init(const Options& options) {
       connection_timeout = temp;
   }
 
-#if defined(OS_MACOSX)
-  if (base::CommandLine::InitializedForCurrentProcess() &&
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableHeapProfiling)) {
-    base::allocator::PeriodicallyShimNewMallocZones();
-  }
-#endif
-
   message_loop_->task_runner()->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(&ChildThreadImpl::EnsureConnected,