memory-infra: Move chrome tracing dumps into the service

Since we now have the Chrome memory dumps in the service, we can now
centralise the code which performs the trace dumps.

Bug: 728199
Change-Id: I7385a99782b9027a85eacd11a755a6d10bb3c8f1
Reviewed-on: https://chromium-review.googlesource.com/685854
Commit-Queue: Lalit Maganti <lalitm@chromium.org>
Reviewed-by: Hector Dearman <hjd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510274}

services/resource_coordinator/memory_instrumentation/coordinator_impl.cc

@@ -269,7 +269,7 @@ void CoordinatorImpl::RequestGlobalMemoryDumpInternal(
     }
   }
 
-  queued_memory_dump_requests_.emplace_back(args, callback);
+  queued_memory_dump_requests_.emplace_back(args, callback, add_to_trace);
 
   // If another dump is already in queued, this dump will automatically be
   // scheduled when the other dump finishes.
@@ -400,7 +400,14 @@ void CoordinatorImpl::OnChromeMemoryDumpResponse(
     return;
   }
 
-  request->responses[client].chrome_dump = std::move(chrome_memory_dump);
+  // Only add to trace if requested.
+  auto* response = &request->responses[client];
+  if (chrome_memory_dump && request->add_to_trace) {
+    bool added_to_trace = tracing_observer_->AddChromeDumpToTraceIfEnabled(
+        request->args, response->process_id, chrome_memory_dump.get());
+    success = success && added_to_trace;
+  }
+  response->chrome_dump = std::move(chrome_memory_dump);
 
   if (!success) {
     request->failed_memory_dump_count++;
@@ -541,8 +548,12 @@ void CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() {
       continue;
 
     mojom::OSMemDumpPtr os_dump = CreatePublicOSDump(*raw_dumps.raw_os_dump);
-    tracing_observer_->AddOsDumpToTraceIfEnabled(
-        request->args, pid, os_dump.get(), &raw_dumps.raw_os_dump->memory_maps);
+
+    if (request->add_to_trace) {
+      tracing_observer_->AddOsDumpToTraceIfEnabled(
+          request->args, pid, os_dump.get(),
+          &raw_dumps.raw_os_dump->memory_maps);
+    }
 
     if (request->args.dump_type == MemoryDumpType::VM_REGIONS_ONLY) {
       DCHECK(request->wants_mmaps());
@@ -598,8 +609,9 @@ CoordinatorImpl::QueuedMemoryDumpRequest::Response::~Response() {}
 
 CoordinatorImpl::QueuedMemoryDumpRequest::QueuedMemoryDumpRequest(
     const base::trace_event::MemoryDumpRequestArgs& args,
-    const RequestGlobalMemoryDumpInternalCallback& callback)
-    : args(args), callback(callback) {}
+    const RequestGlobalMemoryDumpInternalCallback& callback,
+    bool add_to_trace)
+    : args(args), callback(callback), add_to_trace(add_to_trace) {}
 
 CoordinatorImpl::QueuedMemoryDumpRequest::~QueuedMemoryDumpRequest() {}
 

services/resource_coordinator/memory_instrumentation/coordinator_impl.h

@@ -79,10 +79,12 @@ class CoordinatorImpl : public Coordinator, public mojom::Coordinator {
   struct QueuedMemoryDumpRequest {
     QueuedMemoryDumpRequest(
         const base::trace_event::MemoryDumpRequestArgs& args,
-        const RequestGlobalMemoryDumpInternalCallback& callback);
+        const RequestGlobalMemoryDumpInternalCallback& callback,
+        bool add_to_trace);
     ~QueuedMemoryDumpRequest();
     const base::trace_event::MemoryDumpRequestArgs args;
     const RequestGlobalMemoryDumpInternalCallback callback;
+    const bool add_to_trace;
 
     bool wants_mmaps() const {
       return args.level_of_detail ==

services/resource_coordinator/memory_instrumentation/coordinator_impl_unittest.cc

@@ -5,9 +5,12 @@
 #include "services/resource_coordinator/memory_instrumentation/coordinator_impl.h"
 
 #include "base/bind.h"
+#include "base/memory/ref_counted_memory.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/test/trace_event_analyzer.h"
 #include "base/trace_event/memory_dump_request_args.h"
+#include "base/trace_event/trace_buffer.h"
 #include "build/build_config.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
@@ -37,18 +40,56 @@ using ::testing::AllOf;
 
 using RequestGlobalMemoryDumpCallback =
     memory_instrumentation::CoordinatorImpl::RequestGlobalMemoryDumpCallback;
+using RequestGlobalMemoryDumpAndAppendToTraceCallback = memory_instrumentation::
+    CoordinatorImpl::RequestGlobalMemoryDumpAndAppendToTraceCallback;
 using GetVmRegionsForHeapProfilerCallback = memory_instrumentation::
     CoordinatorImpl::GetVmRegionsForHeapProfilerCallback;
 using memory_instrumentation::mojom::GlobalMemoryDumpPtr;
 using memory_instrumentation::mojom::GlobalMemoryDump;
+using base::trace_event::MemoryAllocatorDump;
 using base::trace_event::MemoryDumpArgs;
 using base::trace_event::MemoryDumpLevelOfDetail;
-using base::trace_event::ProcessMemoryDump;
-using base::trace_event::MemoryAllocatorDump;
+using base::trace_event::MemoryDumpManager;
 using base::trace_event::MemoryDumpRequestArgs;
+using base::trace_event::MemoryDumpType;
+using base::trace_event::ProcessMemoryDump;
+using base::trace_event::TraceConfig;
+using base::trace_event::TraceLog;
+using base::trace_event::TraceResultBuffer;
 
 namespace memory_instrumentation {
 
+namespace {
+
+std::unique_ptr<trace_analyzer::TraceAnalyzer> GetDeserializedTrace() {
+  // Flush the trace into JSON.
+  TraceResultBuffer buffer;
+  TraceResultBuffer::SimpleOutput trace_output;
+  buffer.SetOutputCallback(trace_output.GetCallback());
+  base::RunLoop run_loop;
+  buffer.Start();
+  auto on_trace_data_collected =
+      [](base::Closure quit_closure, TraceResultBuffer* buffer,
+         const scoped_refptr<base::RefCountedString>& json,
+         bool has_more_events) {
+        buffer->AddFragment(json->data());
+        if (!has_more_events)
+          quit_closure.Run();
+      };
+
+  TraceLog::GetInstance()->Flush(Bind(on_trace_data_collected,
+                                      run_loop.QuitClosure(),
+                                      base::Unretained(&buffer)));
+  run_loop.Run();
+  buffer.Finish();
+
+  // Analyze the JSON.
+  return base::WrapUnique(
+      trace_analyzer::TraceAnalyzer::Create(trace_output.json_output));
+}
+
+}  // namespace
+
 class FakeCoordinatorImpl : public CoordinatorImpl {
  public:
   FakeCoordinatorImpl() : CoordinatorImpl(nullptr) {}
@@ -89,6 +130,12 @@ class CoordinatorImplTest : public testing::Test {
     coordinator_->RequestGlobalMemoryDump(args, callback);
   }
 
+  void RequestGlobalMemoryDumpAndAppendToTrace(
+      MemoryDumpRequestArgs args,
+      RequestGlobalMemoryDumpAndAppendToTraceCallback callback) {
+    coordinator_->RequestGlobalMemoryDumpAndAppendToTrace(args, callback);
+  }
+
   void GetVmRegionsForHeapProfiler(
       GetVmRegionsForHeapProfilerCallback callback) {
     coordinator_->GetVmRegionsForHeapProfiler(callback);
@@ -173,6 +220,19 @@ class MockGlobalMemoryDumpCallback {
   }
 };
 
+class MockGlobalMemoryDumpAndAppendToTraceCallback {
+ public:
+  MockGlobalMemoryDumpAndAppendToTraceCallback() = default;
+  MOCK_METHOD2(OnCall, void(bool, uint64_t));
+
+  void Run(bool success, uint64_t dump_guid) { OnCall(success, dump_guid); }
+
+  RequestGlobalMemoryDumpAndAppendToTraceCallback Get() {
+    return base::Bind(&MockGlobalMemoryDumpAndAppendToTraceCallback::Run,
+                      base::Unretained(this));
+  }
+};
+
 class MockGetVmRegionsForHeapProfilerCallback {
  public:
   MockGetVmRegionsForHeapProfilerCallback() = default;
@@ -213,7 +273,7 @@ mojom::RawOSMemDumpPtr FillRawOSDump(int pid) {
 // Tests that the global dump is acked even in absence of clients.
 TEST_F(CoordinatorImplTest, NoClients) {
   base::trace_event::MemoryDumpRequestArgs args = {
-      0, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
+      0, base::trace_event::MemoryDumpType::SUMMARY_ONLY,
       MemoryDumpLevelOfDetail::DETAILED};
 
   MockGlobalMemoryDumpCallback callback;
@@ -228,11 +288,12 @@ TEST_F(CoordinatorImplTest, SeveralClients) {
   NiceMock<MockClientProcess> client_process_1(this, 1,
                                                mojom::ProcessType::BROWSER);
   NiceMock<MockClientProcess> client_process_2(this);
+
   EXPECT_CALL(client_process_1, RequestChromeMemoryDump(_, _)).Times(1);
   EXPECT_CALL(client_process_2, RequestChromeMemoryDump(_, _)).Times(1);
 
   base::trace_event::MemoryDumpRequestArgs args = {
-      0, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
+      0, base::trace_event::MemoryDumpType::SUMMARY_ONLY,
       MemoryDumpLevelOfDetail::DETAILED};
 
   MockGlobalMemoryDumpCallback callback;
@@ -246,7 +307,7 @@ TEST_F(CoordinatorImplTest, MissingChromeDump) {
   base::RunLoop run_loop;
 
   base::trace_event::MemoryDumpRequestArgs args = {
-      0, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
+      0, base::trace_event::MemoryDumpType::SUMMARY_ONLY,
       MemoryDumpLevelOfDetail::DETAILED};
 
   NiceMock<MockClientProcess> client_process(this, 1,
@@ -276,7 +337,7 @@ TEST_F(CoordinatorImplTest, MissingOsDump) {
   base::RunLoop run_loop;
 
   base::trace_event::MemoryDumpRequestArgs args = {
-      0, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
+      0, base::trace_event::MemoryDumpType::SUMMARY_ONLY,
       MemoryDumpLevelOfDetail::DETAILED};
 
   NiceMock<MockClientProcess> client_process(this, 1,
@@ -607,4 +668,96 @@ TEST_F(CoordinatorImplTest, VmRegionsForHeapProfiler) {
   run_loop.Run();
 }
 
+// RequestGlobalMemoryDump, as opposite to RequestGlobalMemoryDumpAndAddToTrace,
+// shouldn't add anything into the trace
+TEST_F(CoordinatorImplTest, DumpsArentAddedToTraceUnlessRequested) {
+  using trace_analyzer::Query;
+
+  base::RunLoop run_loop;
+
+  base::trace_event::MemoryDumpRequestArgs args = {
+      0, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
+      MemoryDumpLevelOfDetail::DETAILED};
+
+  NiceMock<MockClientProcess> client_process(this, 1,
+                                             mojom::ProcessType::BROWSER);
+
+  EXPECT_CALL(client_process, RequestChromeMemoryDump(_, _))
+      .WillOnce(
+          Invoke([](const MemoryDumpRequestArgs& args,
+                    const MockClientProcess::RequestChromeMemoryDumpCallback&
+                        callback) {
+            MemoryDumpArgs dump_args{MemoryDumpLevelOfDetail::DETAILED};
+            auto pmd = std::make_unique<ProcessMemoryDump>(nullptr, dump_args);
+            callback.Run(true, args.dump_guid, std::move(pmd));
+          }));
+
+  MockGlobalMemoryDumpCallback callback;
+  EXPECT_CALL(
+      callback,
+      OnCall(true, Pointee(Field(&mojom::GlobalMemoryDump::process_dumps,
+                                 IsEmpty()))))
+      .WillOnce(RunClosure(run_loop.QuitClosure()));
+
+  TraceLog::GetInstance()->SetEnabled(
+      TraceConfig(MemoryDumpManager::kTraceCategory, ""),
+      TraceLog::RECORDING_MODE);
+  RequestGlobalMemoryDump(args, callback.Get());
+  run_loop.Run();
+  TraceLog::GetInstance()->SetDisabled();
+
+  std::unique_ptr<trace_analyzer::TraceAnalyzer> analyzer =
+      GetDeserializedTrace();
+  trace_analyzer::TraceEventVector events;
+  analyzer->FindEvents(Query::EventPhaseIs(TRACE_EVENT_PHASE_MEMORY_DUMP),
+                       &events);
+
+  ASSERT_EQ(0u, events.size());
+}
+
+TEST_F(CoordinatorImplTest, DumpsAreAddedToTraceWhenRequested) {
+  using trace_analyzer::Query;
+
+  base::RunLoop run_loop;
+
+  base::trace_event::MemoryDumpRequestArgs args = {
+      0, base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
+      MemoryDumpLevelOfDetail::DETAILED};
+
+  NiceMock<MockClientProcess> client_process(this, 1,
+                                             mojom::ProcessType::BROWSER);
+
+  EXPECT_CALL(client_process, RequestChromeMemoryDump(_, _))
+      .WillOnce(
+          Invoke([](const MemoryDumpRequestArgs& args,
+                    const MockClientProcess::RequestChromeMemoryDumpCallback&
+                        callback) {
+            MemoryDumpArgs dump_args{MemoryDumpLevelOfDetail::DETAILED};
+            auto pmd = std::make_unique<ProcessMemoryDump>(nullptr, dump_args);
+            callback.Run(true, args.dump_guid, std::move(pmd));
+          }));
+
+  MockGlobalMemoryDumpAndAppendToTraceCallback callback;
+  EXPECT_CALL(callback, OnCall(true, Ne(0ul)))
+      .WillOnce(RunClosure(run_loop.QuitClosure()));
+
+  TraceLog::GetInstance()->SetEnabled(
+      TraceConfig(MemoryDumpManager::kTraceCategory, ""),
+      TraceLog::RECORDING_MODE);
+  RequestGlobalMemoryDumpAndAppendToTrace(args, callback.Get());
+  run_loop.Run();
+  TraceLog::GetInstance()->SetDisabled();
+
+  std::unique_ptr<trace_analyzer::TraceAnalyzer> analyzer =
+      GetDeserializedTrace();
+  trace_analyzer::TraceEventVector events;
+  analyzer->FindEvents(Query::EventPhaseIs(TRACE_EVENT_PHASE_MEMORY_DUMP),
+                       &events);
+
+  ASSERT_EQ(1u, events.size());
+  ASSERT_TRUE(trace_analyzer::CountMatches(
+      events, Query::EventNameIs(MemoryDumpTypeToString(
+                  MemoryDumpType::EXPLICITLY_TRIGGERED))));
+}
+
 }  // namespace memory_instrumentation

services/resource_coordinator/public/cpp/memory_instrumentation/client_process_impl.cc

@@ -91,15 +91,6 @@ void ClientProcessImpl::OnChromeMemoryDumpDone(
     callback.Run(false, dump_guid, nullptr);
     return;
   }
-  // SUMMARY_ONLY dumps should just return the summarized result in the
-  // ProcessMemoryDumpCallback. These shouldn't be added to the trace to
-  // avoid confusing trace consumers.
-  if (req_args.dump_type != base::trace_event::MemoryDumpType::SUMMARY_ONLY) {
-    bool added_to_trace = tracing_observer_->AddChromeDumpToTraceIfEnabled(
-        req_args, process_memory_dump.get());
-    success = success && added_to_trace;
-  }
-
   callback.Run(success, dump_guid, std::move(process_memory_dump));
 }
 

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_integration_unittest.cc

@@ -54,8 +54,6 @@ const char* kBackgroundButNotSummaryWhitelistedMDPName =
     "BackgroundButNotSummaryWhitelistedTestDumpProvider";
 const char* const kTestMDPWhitelist[] = {
     kWhitelistedMDPName, kBackgroundButNotSummaryWhitelistedMDPName, nullptr};
-const char* const kTestMDPWhitelistForSummary[] = {kWhitelistedMDPName,
-                                                   nullptr};
 const uint64_t kTestGuid = 0;
 
 // GTest matchers for MemoryDumpRequestArgs arguments.
@@ -101,33 +99,6 @@ class MockMemoryDumpProvider : public MemoryDumpProvider {
   bool enable_mock_destructor;
 };
 
-std::unique_ptr<trace_analyzer::TraceAnalyzer> GetDeserializedTrace() {
-  // Flush the trace into JSON.
-  TraceResultBuffer buffer;
-  TraceResultBuffer::SimpleOutput trace_output;
-  buffer.SetOutputCallback(trace_output.GetCallback());
-  base::RunLoop run_loop;
-  buffer.Start();
-  auto on_trace_data_collected =
-      [](base::Closure quit_closure, TraceResultBuffer* buffer,
-         const scoped_refptr<base::RefCountedString>& json,
-         bool has_more_events) {
-        buffer->AddFragment(json->data());
-        if (!has_more_events)
-          quit_closure.Run();
-      };
-
-  TraceLog::GetInstance()->Flush(Bind(on_trace_data_collected,
-                                      run_loop.QuitClosure(),
-                                      base::Unretained(&buffer)));
-  run_loop.Run();
-  buffer.Finish();
-
-  // Analyze the JSON.
-  return base::WrapUnique(
-      trace_analyzer::TraceAnalyzer::Create(trace_output.json_output));
-}
-
 }  // namespace
 
 class MemoryTracingIntegrationTest;
@@ -279,87 +250,6 @@ void MockCoordinator::RequestGlobalMemoryDumpAndAppendToTrace(
   callback.Run(args.dump_guid, true);
 }
 
-// Tests that the MemoryDumpProvider(s) are invoked even if tracing has not
-// been initialized.
-TEST_F(MemoryTracingIntegrationTest, DumpWithoutInitializingTracing) {
-  InitializeClientProcess(mojom::ProcessType::RENDERER);
-  MockMemoryDumpProvider mdp;
-  RegisterDumpProvider(&mdp, base::ThreadTaskRunnerHandle::Get(),
-                       MemoryDumpProvider::Options());
-  DisableTracing();
-  EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(3);
-  for (int i = 0; i < 3; ++i) {
-    // A non-SUMMARY_ONLY dump was requested when tracing was not enabled. So,
-    // the request should fail because dump was not added to the trace.
-    EXPECT_FALSE(RequestChromeDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                                          MemoryDumpLevelOfDetail::DETAILED));
-  }
-  mdm_->UnregisterDumpProvider(&mdp);
-}
-
-// Similar to DumpWithoutInitializingTracing. Tracing is initialized but not
-// enabled.
-TEST_F(MemoryTracingIntegrationTest,
-       DumpWithMDMInitializedForTracingButDisabled) {
-  InitializeClientProcess(mojom::ProcessType::RENDERER);
-  MockMemoryDumpProvider mdp;
-  RegisterDumpProvider(&mdp, base::ThreadTaskRunnerHandle::Get(),
-                       MemoryDumpProvider::Options());
-
-  DisableTracing();
-
-  const TraceConfig& trace_config =
-      TraceConfig(base::trace_event::TraceConfigMemoryTestUtil::
-                      GetTraceConfig_NoTriggers());
-  const TraceConfig::MemoryDumpConfig& memory_dump_config =
-      trace_config.memory_dump_config();
-  mdm_->SetupForTracing(memory_dump_config);
-
-  EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(3);
-  for (int i = 0; i < 3; ++i) {
-    // Same as the above. Even if the MDP(s) are invoked, this will return false
-    // while attempting to add the dump into the trace.
-    EXPECT_FALSE(RequestChromeDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                                          MemoryDumpLevelOfDetail::DETAILED));
-  }
-  mdm_->TeardownForTracing();
-  mdm_->UnregisterDumpProvider(&mdp);
-}
-
-TEST_F(MemoryTracingIntegrationTest, SummaryOnlyDumpsArentAddedToTrace) {
-  InitializeClientProcess(mojom::ProcessType::RENDERER);
-  using trace_analyzer::Query;
-
-  base::trace_event::SetDumpProviderSummaryWhitelistForTesting(
-      kTestMDPWhitelistForSummary);
-  base::trace_event::SetDumpProviderWhitelistForTesting(kTestMDPWhitelist);
-
-  // Standard provider with default options (create dump for current process).
-  MockMemoryDumpProvider mdp;
-  RegisterDumpProvider(&mdp, nullptr, MemoryDumpProvider::Options(),
-                       kWhitelistedMDPName);
-
-  EnableMemoryInfraTracing();
-
-  EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(2);
-  EXPECT_TRUE(RequestChromeDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                                       MemoryDumpLevelOfDetail::BACKGROUND));
-  EXPECT_TRUE(RequestChromeDumpAndWait(MemoryDumpType::SUMMARY_ONLY,
-                                       MemoryDumpLevelOfDetail::BACKGROUND));
-  DisableTracing();
-
-  std::unique_ptr<trace_analyzer::TraceAnalyzer> analyzer =
-      GetDeserializedTrace();
-  trace_analyzer::TraceEventVector events;
-  analyzer->FindEvents(Query::EventPhaseIs(TRACE_EVENT_PHASE_MEMORY_DUMP),
-                       &events);
-
-  ASSERT_EQ(1u, events.size());
-  ASSERT_TRUE(trace_analyzer::CountMatches(
-      events, Query::EventNameIs(MemoryDumpTypeToString(
-                  MemoryDumpType::EXPLICITLY_TRIGGERED))));
-}
-
 // Checks that is the ClientProcessImpl is initialized after tracing already
 // began, it will still late-join the party (real use case: startup tracing).
 TEST_F(MemoryTracingIntegrationTest, InitializedAfterStartOfTracing) {
@@ -415,15 +305,14 @@ TEST_F(MemoryTracingIntegrationTest, TestBackgroundTracingSetup) {
 
   run_loop.Run();
 
-  // When requesting non-BACKGROUND dumps the MDP will be invoked but the
-  // data is expected to be dropped on the floor, hence the EXPECT_FALSE.
+  // When requesting non-BACKGROUND dumps the MDP will be invoked.
   EXPECT_CALL(*mdp, OnMemoryDump(IsLightDump(), _));
-  EXPECT_FALSE(RequestChromeDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                                        MemoryDumpLevelOfDetail::LIGHT));
+  EXPECT_TRUE(RequestChromeDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                                       MemoryDumpLevelOfDetail::LIGHT));
 
   EXPECT_CALL(*mdp, OnMemoryDump(IsDetailedDump(), _));
-  EXPECT_FALSE(RequestChromeDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
-                                        MemoryDumpLevelOfDetail::DETAILED));
+  EXPECT_TRUE(RequestChromeDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                                       MemoryDumpLevelOfDetail::DETAILED));
 
   ASSERT_TRUE(IsPeriodicDumpingEnabled());
   DisableTracing();

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.cc

@@ -96,13 +96,6 @@ bool TracingObserver::ShouldAddToTrace(
   // including PII.
   if (!IsDumpModeAllowed(args.level_of_detail))
     return false;
-
-  // SUMMARY_ONLY dumps are just return the summarized result in the
-  // ProcessMemoryDumpCallback. These shouldn't be added to the trace to
-  // avoid confusing trace consumers.
-  if (args.dump_type == base::trace_event::MemoryDumpType::SUMMARY_ONLY)
-    return false;
-
   return true;
 }
 
@@ -131,6 +124,7 @@ void TracingObserver::AddToTrace(
 
 bool TracingObserver::AddChromeDumpToTraceIfEnabled(
     const base::trace_event::MemoryDumpRequestArgs& args,
+    const base::ProcessId pid,
     const ProcessMemoryDump* process_memory_dump) {
   if (!ShouldAddToTrace(args))
     return false;
@@ -138,7 +132,7 @@ bool TracingObserver::AddChromeDumpToTraceIfEnabled(
   std::unique_ptr<TracedValue> traced_value = base::MakeUnique<TracedValue>();
   process_memory_dump->SerializeAllocatorDumpsInto(traced_value.get());
 
-  AddToTrace(args, base::kNullProcessId, std::move(traced_value));
+  AddToTrace(args, pid, std::move(traced_value));
 
   return true;
 }

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_observer.h

@@ -29,6 +29,7 @@ class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT TracingObserver
 
   bool AddChromeDumpToTraceIfEnabled(
       const base::trace_event::MemoryDumpRequestArgs&,
+      const base::ProcessId pid,
       const base::trace_event::ProcessMemoryDump*);
   bool AddOsDumpToTraceIfEnabled(
       const base::trace_event::MemoryDumpRequestArgs&,