Revert "Remove "--enable-heap-profiling""

This reverts commit 51e2344.

Reason for revert: Sid points out:
"""
We just figured that memlog doesn't work in WebView. WebView does not build chrome/ folders and we used to have the legacy heap profiling working on it. We might have to revert this change that removes the old flag while we figure out webview case.
Sorry about this late finding..
"""

Original change's description:
> Remove "--enable-heap-profiling"
> 
> 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}

TBR=avi@chromium.org,dcheng@chromium.org,primiano@chromium.org,erikchen@chromium.org,perezju@chromium.org,ssid@chromium.org,dskiba@chromium.org,tmartino@chromium.org

Change-Id: Iaac5d628ff248d70830b0c19403780df9ec0cdef
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 758739
Reviewed-on: https://chromium-review.googlesource.com/982693
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546308}

base/base_switches.cc

@@ -21,6 +21,25 @@ 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

@@ -16,6 +16,10 @@ 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,25 +15,11 @@
 namespace base {
 namespace trace_event {
 
-// 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
+// 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.
 class BASE_EXPORT AllocationContextTracker {
  public:
   enum class CaptureMode : int32_t {

base/trace_event/memory_dump_manager.cc

@@ -201,6 +201,44 @@ 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)
@@ -291,6 +329,7 @@ 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,6 +131,15 @@ 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,6 +961,54 @@ 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,6 +4884,11 @@ 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

@@ -959,6 +959,16 @@ 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,
@@ -3146,6 +3156,10 @@ 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

@@ -390,6 +390,12 @@ 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

@@ -271,6 +271,12 @@ 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/profiling_host/profiling_process_host.cc

@@ -273,15 +273,16 @@ void ProfilingProcessHost::AddClientToProfilingService(
 ProfilingProcessHost::Mode ProfilingProcessHost::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(switches::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 --" << switches::kMemlog
+                 << "which are not compatible. Memlog will be disabled.";
+      return Mode::kNone;
+    }
 
     std::string mode;
     // Respect the commandline switch above the field trial.

chrome/browser/ui/startup/bad_flags_prompt.cc

@@ -119,6 +119,25 @@ void ShowBadFeatureFlagsInfoBar(content::WebContents* web_contents,
 }  // namespace
 
 void ShowBadFlagsPrompt(content::WebContents* web_contents) {
+  // 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/tracing/common/trace_startup.cc

@@ -22,6 +22,10 @@ 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),

components/tracing/docs/heap_profiler_internals.md

@@ -0,0 +1,2 @@
+This document has moved to [//docs/memory-infra/heap_profiler_internals.md](/docs/memory-infra/heap_profiler_internals.md).
+

content/browser/browser_main_loop.cc

@@ -167,6 +167,7 @@
 #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"
@@ -267,6 +268,7 @@ 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.
@@ -841,6 +843,14 @@ 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,6 +142,7 @@ 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

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

content/child/child_thread_impl.cc

@@ -75,6 +75,10 @@
 #include "content/public/common/content_descriptors.h"
 #endif
 
+#if defined(OS_MACOSX)
+#include "base/allocator/allocator_interception_mac.h"
+#endif
+
 namespace content {
 namespace {
 
@@ -553,6 +557,14 @@ 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,