MemoryInfra: Do not call OnHeapProfilingEnabled() for all providers

To call OnHeapProfilingEnabled() on all providers, it is required to
post task for each of them. This sometimes creates issues because the
task runner may not be available when enabling profiling. So, only call
on providers that support profiling.

BUG=762994

Change-Id: Ic428f7c772e982167d3eb2260197e11a1f8685cf
Reviewed-on: https://chromium-review.googlesource.com/657894Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501501}

base/trace_event/memory_dump_manager.cc

@@ -331,7 +331,10 @@ void MemoryDumpManager::Initialize(
 
 // Enable the core dump providers.
 #if defined(MALLOC_MEMORY_TRACING_SUPPORTED)
-  RegisterDumpProvider(MallocDumpProvider::GetInstance(), "Malloc", nullptr);
+  base::trace_event::MemoryDumpProvider::Options options;
+  options.supports_heap_profiling = true;
+  RegisterDumpProvider(MallocDumpProvider::GetInstance(), "Malloc", nullptr,
+                       options);
 #endif
 
 #if defined(OS_ANDROID)
@@ -849,13 +852,8 @@ void MemoryDumpManager::NotifyHeapProfilingEnabledLocked(
     scoped_refptr<MemoryDumpProviderInfo> mdpinfo,
     bool enabled) {
   lock_.AssertAcquired();
-
-  // If we do not have ability to request global dumps, then a dump cannot be in
-  // progress. So, the notification can be sent on any thread. This is done not
-  // to create thread early during startup since sandbox initialization that
-  // happens later requires no thread to be created.
-  if (!can_request_global_dumps())
-    return mdpinfo->dump_provider->OnHeapProfilingEnabled(enabled);
+  if (!mdpinfo->options.supports_heap_profiling)
+    return;
 
   const auto& task_runner = mdpinfo->task_runner
                                 ? mdpinfo->task_runner

base/trace_event/memory_dump_manager_unittest.cc

@@ -840,7 +840,10 @@ TEST_F(MemoryDumpManagerTest, EnableHeapProfilingPseudoStack) {
   InitializeMemoryDumpManagerForInProcessTesting(false /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
   MockMemoryDumpProvider mdp2;
-  RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get());
+  MockMemoryDumpProvider mdp3;
+  MemoryDumpProvider::Options supported_options;
+  supported_options.supports_heap_profiling = true;
+  RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get(), supported_options);
   {
     testing::InSequence sequence;
     EXPECT_CALL(mdp1, OnHeapProfilingEnabled(true)).Times(1);
@@ -851,6 +854,8 @@ TEST_F(MemoryDumpManagerTest, EnableHeapProfilingPseudoStack) {
     EXPECT_CALL(mdp2, OnHeapProfilingEnabled(true)).Times(1);
     EXPECT_CALL(mdp2, OnHeapProfilingEnabled(false)).Times(1);
   }
+  RegisterDumpProvider(&mdp3, ThreadTaskRunnerHandle::Get());
+  EXPECT_CALL(mdp3, OnHeapProfilingEnabled(_)).Times(0);
 
   EXPECT_TRUE(mdm_->EnableHeapProfiling(kHeapProfilingModePseudo));
   RunLoop().RunUntilIdle();
@@ -858,7 +863,7 @@ TEST_F(MemoryDumpManagerTest, EnableHeapProfilingPseudoStack) {
             AllocationContextTracker::capture_mode());
   EXPECT_EQ(mdm_->GetHeapProfilingMode(), kHeapProfilingModePseudo);
   EXPECT_EQ(TraceLog::FILTERING_MODE, TraceLog::GetInstance()->enabled_modes());
-  RegisterDumpProvider(&mdp2, ThreadTaskRunnerHandle::Get());
+  RegisterDumpProvider(&mdp2, ThreadTaskRunnerHandle::Get(), supported_options);
 
   TraceConfig::MemoryDumpConfig config;
   config.heap_profiler_options.breakdown_threshold_bytes = 100;
@@ -891,7 +896,9 @@ TEST_F(MemoryDumpManagerTest, EnableHeapProfilingPseudoStack) {
 TEST_F(MemoryDumpManagerTest, EnableHeapProfilingNoStack) {
   InitializeMemoryDumpManagerForInProcessTesting(true /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
-  RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get());
+  MemoryDumpProvider::Options supported_options;
+  supported_options.supports_heap_profiling = true;
+  RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get(), supported_options);
   testing::InSequence sequence;
   EXPECT_CALL(mdp1, OnHeapProfilingEnabled(true)).Times(1);
   EXPECT_CALL(mdp1, OnHeapProfilingEnabled(false)).Times(1);
@@ -939,7 +946,9 @@ TEST_F(MemoryDumpManagerTest, EnableHeapProfilingTask) {
   InitializeMemoryDumpManagerForInProcessTesting(true /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
   MockMemoryDumpProvider mdp2;
-  RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get());
+  MemoryDumpProvider::Options supported_options;
+  supported_options.supports_heap_profiling = true;
+  RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get(), supported_options);
   EXPECT_CALL(mdp1, OnHeapProfilingEnabled(_)).Times(0);
   EXPECT_CALL(mdp2, OnHeapProfilingEnabled(_)).Times(0);
 
@@ -948,7 +957,7 @@ TEST_F(MemoryDumpManagerTest, EnableHeapProfilingTask) {
   RunLoop().RunUntilIdle();
   ASSERT_EQ(AllocationContextTracker::CaptureMode::DISABLED,
             AllocationContextTracker::capture_mode());
-  RegisterDumpProvider(&mdp2, ThreadTaskRunnerHandle::Get());
+  RegisterDumpProvider(&mdp2, ThreadTaskRunnerHandle::Get(), supported_options);
   EXPECT_EQ(mdm_->GetHeapProfilingMode(), kHeapProfilingModeTaskProfiler);
   ASSERT_TRUE(debug::ThreadHeapUsageTracker::IsHeapTrackingEnabled());
   TestingThreadHeapUsageTracker::DisableHeapTrackingForTesting();
@@ -966,7 +975,9 @@ TEST_F(MemoryDumpManagerTest, EnableHeapProfilingDisableDisabled) {
 TEST_F(MemoryDumpManagerTest, EnableHeapProfilingIfNeeded) {
   InitializeMemoryDumpManagerForInProcessTesting(false /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
-  RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get());
+  MemoryDumpProvider::Options supported_options;
+  supported_options.supports_heap_profiling = true;
+  RegisterDumpProvider(&mdp1, ThreadTaskRunnerHandle::Get(), supported_options);
 
   // Should be noop.
   mdm_->EnableHeapProfilingIfNeeded();

base/trace_event/memory_dump_provider.h

@@ -22,7 +22,8 @@ class BASE_EXPORT MemoryDumpProvider {
   struct Options {
     Options()
         : dumps_on_single_thread_task_runner(false),
-          is_fast_polling_supported(false) {}
+          is_fast_polling_supported(false),
+          supports_heap_profiling(false) {}
 
     // |dumps_on_single_thread_task_runner| is true if the dump provider runs on
     // a SingleThreadTaskRunner, which is usually the case. It is faster to run
@@ -33,6 +34,10 @@ class BASE_EXPORT MemoryDumpProvider {
     // polling. Only providers running without task runner affinity are
     // supported.
     bool is_fast_polling_supported;
+
+    // Set to true when the dump provider supports heap profiling. MDM sends
+    // OnHeapProfiling() notifications only if this is set to true.
+    bool supports_heap_profiling;
   };
 
   virtual ~MemoryDumpProvider() {}
@@ -48,9 +53,8 @@ class BASE_EXPORT MemoryDumpProvider {
                             ProcessMemoryDump* pmd) = 0;
 
   // Called by the MemoryDumpManager when an allocator should start or stop
-  // collecting extensive allocation data, if supported. Can be called on any
-  // thread and MDM guarantees OnMemoryDump() and OnHeapProfilingEnabled() are
-  // not called at the same time.
+  // collecting extensive allocation data, if supported. Called only when
+  // |supports_heap_profiling| is set to true.
   virtual void OnHeapProfilingEnabled(bool enabled) {}
 
   // Quickly record the total memory usage in |memory_total|. This method will

third_party/WebKit/Source/platform/exported/Platform.cpp

@@ -123,10 +123,13 @@ void Platform::Initialize(Platform* platform) {
 
   ProcessHeap::Init();
   MemoryCoordinator::Initialize();
-  if (base::ThreadTaskRunnerHandle::IsSet())
+  if (base::ThreadTaskRunnerHandle::IsSet()) {
+    base::trace_event::MemoryDumpProvider::Options options;
+    options.supports_heap_profiling = true;
     base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
         BlinkGCMemoryDumpProvider::Instance(), "BlinkGC",
-        base::ThreadTaskRunnerHandle::Get());
+        base::ThreadTaskRunnerHandle::Get(), options);
+  }
 
   ThreadState::AttachMainThread();
 
@@ -140,9 +143,11 @@ void Platform::Initialize(Platform* platform) {
   if (g_platform->main_thread_) {
     DCHECK(!g_gc_task_runner);
     g_gc_task_runner = new GCTaskRunner(g_platform->main_thread_);
+    base::trace_event::MemoryDumpProvider::Options heap_profiling_options;
+    heap_profiling_options.supports_heap_profiling = true;
     base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
         PartitionAllocMemoryDumpProvider::Instance(), "PartitionAlloc",
-        base::ThreadTaskRunnerHandle::Get());
+        base::ThreadTaskRunnerHandle::Get(), heap_profiling_options);
     base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
         FontCacheMemoryDumpProvider::Instance(), "FontCaches",
         base::ThreadTaskRunnerHandle::Get());