diff --git a/base/BUILD.gn b/base/BUILD.gn index 3b25692cca571d..c5c8561a07da87 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -958,6 +958,8 @@ component("base") { "trace_event/heap_profiler_event_filter.h", "trace_event/heap_profiler_heap_dump_writer.cc", "trace_event/heap_profiler_heap_dump_writer.h", + "trace_event/heap_profiler_serialization_state.cc", + "trace_event/heap_profiler_serialization_state.h", "trace_event/heap_profiler_stack_frame_deduplicator.cc", "trace_event/heap_profiler_stack_frame_deduplicator.h", "trace_event/heap_profiler_type_name_deduplicator.cc", @@ -979,8 +981,6 @@ component("base") { "trace_event/memory_dump_request_args.h", "trace_event/memory_dump_scheduler.cc", "trace_event/memory_dump_scheduler.h", - "trace_event/memory_dump_session_state.cc", - "trace_event/memory_dump_session_state.h", "trace_event/memory_infra_background_whitelist.cc", "trace_event/memory_infra_background_whitelist.h", "trace_event/memory_peak_detector.cc", diff --git a/base/trace_event/heap_profiler_heap_dump_writer.cc b/base/trace_event/heap_profiler_heap_dump_writer.cc index 70bd87a44aae07..f06f64cde5174c 100644 --- a/base/trace_event/heap_profiler_heap_dump_writer.cc +++ b/base/trace_event/heap_profiler_heap_dump_writer.cc @@ -16,9 +16,9 @@ #include "base/logging.h" #include "base/macros.h" #include "base/strings/stringprintf.h" +#include "base/trace_event/heap_profiler_serialization_state.h" #include "base/trace_event/heap_profiler_stack_frame_deduplicator.h" #include "base/trace_event/heap_profiler_type_name_deduplicator.h" -#include "base/trace_event/memory_dump_session_state.h" #include "base/trace_event/trace_config.h" #include "base/trace_event/trace_event_argument.h" #include "base/trace_event/trace_log.h" @@ -311,11 +311,12 @@ std::unique_ptr Serialize(const std::set& entries) { std::unique_ptr ExportHeapDump( const std::unordered_map& metrics_by_context, - const MemoryDumpSessionState& session_state) { + const HeapProfilerSerializationState& heap_profiler_serialization_state) { internal::HeapDumpWriter writer( - session_state.stack_frame_deduplicator(), - session_state.type_name_deduplicator(), - session_state.heap_profiler_breakdown_threshold_bytes()); + heap_profiler_serialization_state.stack_frame_deduplicator(), + heap_profiler_serialization_state.type_name_deduplicator(), + heap_profiler_serialization_state + .heap_profiler_breakdown_threshold_bytes()); return Serialize(writer.Summarize(metrics_by_context)); } diff --git a/base/trace_event/heap_profiler_heap_dump_writer.h b/base/trace_event/heap_profiler_heap_dump_writer.h index 84639062602fde..3366c286f521ac 100644 --- a/base/trace_event/heap_profiler_heap_dump_writer.h +++ b/base/trace_event/heap_profiler_heap_dump_writer.h @@ -18,7 +18,7 @@ namespace base { namespace trace_event { -class MemoryDumpSessionState; +class HeapProfilerSerializationState; class StackFrameDeduplicator; class TracedValue; class TypeNameDeduplicator; @@ -30,7 +30,7 @@ class TypeNameDeduplicator; BASE_EXPORT std::unique_ptr ExportHeapDump( const std::unordered_map& metrics_by_context, - const MemoryDumpSessionState& session_state); + const HeapProfilerSerializationState& heap_profiler_serialization_state); namespace internal { diff --git a/base/trace_event/heap_profiler_heap_dump_writer_unittest.cc b/base/trace_event/heap_profiler_heap_dump_writer_unittest.cc index b090eee0d8e008..56da861847231d 100644 --- a/base/trace_event/heap_profiler_heap_dump_writer_unittest.cc +++ b/base/trace_event/heap_profiler_heap_dump_writer_unittest.cc @@ -16,7 +16,6 @@ #include "base/trace_event/heap_profiler_allocation_context.h" #include "base/trace_event/heap_profiler_stack_frame_deduplicator.h" #include "base/trace_event/heap_profiler_type_name_deduplicator.h" -#include "base/trace_event/memory_dump_session_state.h" #include "base/trace_event/trace_event_argument.h" #include "base/values.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/base/trace_event/memory_dump_session_state.cc b/base/trace_event/heap_profiler_serialization_state.cc similarity index 53% rename from base/trace_event/memory_dump_session_state.cc rename to base/trace_event/heap_profiler_serialization_state.cc index d26b82a5b74c65..d332d43c4710cf 100644 --- a/base/trace_event/memory_dump_session_state.cc +++ b/base/trace_event/heap_profiler_serialization_state.cc @@ -2,36 +2,26 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/trace_event/memory_dump_session_state.h" +#include "base/trace_event/heap_profiler_serialization_state.h" namespace base { namespace trace_event { -MemoryDumpSessionState::MemoryDumpSessionState() +HeapProfilerSerializationState::HeapProfilerSerializationState() : heap_profiler_breakdown_threshold_bytes_(0) {} -MemoryDumpSessionState::~MemoryDumpSessionState() {} +HeapProfilerSerializationState::~HeapProfilerSerializationState() {} -void MemoryDumpSessionState::SetStackFrameDeduplicator( +void HeapProfilerSerializationState::SetStackFrameDeduplicator( std::unique_ptr stack_frame_deduplicator) { DCHECK(!stack_frame_deduplicator_); stack_frame_deduplicator_ = std::move(stack_frame_deduplicator); } -void MemoryDumpSessionState::SetTypeNameDeduplicator( +void HeapProfilerSerializationState::SetTypeNameDeduplicator( std::unique_ptr type_name_deduplicator) { DCHECK(!type_name_deduplicator_); type_name_deduplicator_ = std::move(type_name_deduplicator); } -void MemoryDumpSessionState::SetAllowedDumpModes( - std::set allowed_dump_modes) { - allowed_dump_modes_ = allowed_dump_modes; -} - -bool MemoryDumpSessionState::IsDumpModeAllowed( - MemoryDumpLevelOfDetail dump_mode) const { - return allowed_dump_modes_.count(dump_mode) != 0; -} - } // namespace trace_event } // namespace base diff --git a/base/trace_event/memory_dump_session_state.h b/base/trace_event/heap_profiler_serialization_state.h similarity index 82% rename from base/trace_event/memory_dump_session_state.h rename to base/trace_event/heap_profiler_serialization_state.h index 46092cb4832d7d..3d388b209c5807 100644 --- a/base/trace_event/memory_dump_session_state.h +++ b/base/trace_event/heap_profiler_serialization_state.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef BASE_TRACE_EVENT_MEMORY_DUMP_SESSION_STATE_H_ -#define BASE_TRACE_EVENT_MEMORY_DUMP_SESSION_STATE_H_ +#ifndef BASE_TRACE_EVENT_HEAP_PROFILER_SERIALIZATION_STATE_H_ +#define BASE_TRACE_EVENT_HEAP_PROFILER_SERIALIZATION_STATE_H_ #include #include @@ -18,10 +18,10 @@ namespace trace_event { // Container for state variables that should be shared across all the memory // dumps in a tracing session. -class BASE_EXPORT MemoryDumpSessionState - : public RefCountedThreadSafe { +class BASE_EXPORT HeapProfilerSerializationState + : public RefCountedThreadSafe { public: - MemoryDumpSessionState(); + HeapProfilerSerializationState(); // Returns the stack frame deduplicator that should be used by memory dump // providers when doing a heap dump. @@ -55,8 +55,8 @@ class BASE_EXPORT MemoryDumpSessionState } private: - friend class RefCountedThreadSafe; - ~MemoryDumpSessionState(); + friend class RefCountedThreadSafe; + ~HeapProfilerSerializationState(); // Deduplicates backtraces in heap dumps so they can be written once when the // trace is finalized. @@ -66,12 +66,10 @@ class BASE_EXPORT MemoryDumpSessionState // trace is finalized. std::unique_ptr type_name_deduplicator_; - std::set allowed_dump_modes_; - uint32_t heap_profiler_breakdown_threshold_bytes_; }; } // namespace trace_event } // namespace base -#endif // BASE_TRACE_EVENT_MEMORY_DUMP_SESSION_STATE_H_ +#endif // BASE_TRACE_EVENT_HEAP_PROFILER_SERIALIZATION_STATE_H diff --git a/base/trace_event/memory_allocator_dump_unittest.cc b/base/trace_event/memory_allocator_dump_unittest.cc index e1818f6eeccdad..7cd8f516f2c10f 100644 --- a/base/trace_event/memory_allocator_dump_unittest.cc +++ b/base/trace_event/memory_allocator_dump_unittest.cc @@ -8,9 +8,9 @@ #include "base/format_macros.h" #include "base/strings/stringprintf.h" +#include "base/trace_event/heap_profiler_serialization_state.h" #include "base/trace_event/memory_allocator_dump_guid.h" #include "base/trace_event/memory_dump_provider.h" -#include "base/trace_event/memory_dump_session_state.h" #include "base/trace_event/process_memory_dump.h" #include "base/trace_event/trace_event_argument.h" #include "base/values.h" @@ -130,7 +130,7 @@ TEST(MemoryAllocatorDumpTest, GuidGeneration) { TEST(MemoryAllocatorDumpTest, DumpIntoProcessMemoryDump) { FakeMemoryAllocatorDumpProvider fmadp; MemoryDumpArgs dump_args = {MemoryDumpLevelOfDetail::DETAILED}; - ProcessMemoryDump pmd(new MemoryDumpSessionState, dump_args); + ProcessMemoryDump pmd(new HeapProfilerSerializationState, dump_args); fmadp.OnMemoryDump(dump_args, &pmd); @@ -174,7 +174,7 @@ TEST(MemoryAllocatorDumpTest, DumpIntoProcessMemoryDump) { TEST(MemoryAllocatorDumpTest, GetSize) { MemoryDumpArgs dump_args = {MemoryDumpLevelOfDetail::DETAILED}; - ProcessMemoryDump pmd(new MemoryDumpSessionState, dump_args); + ProcessMemoryDump pmd(new HeapProfilerSerializationState, dump_args); MemoryAllocatorDump* dump = pmd.CreateAllocatorDump("allocator_for_size"); dump->AddScalar(MemoryAllocatorDump::kNameSize, MemoryAllocatorDump::kUnitsBytes, 1); @@ -187,7 +187,7 @@ TEST(MemoryAllocatorDumpTest, GetSize) { TEST(MemoryAllocatorDumpTest, ForbidDuplicatesDeathTest) { FakeMemoryAllocatorDumpProvider fmadp; MemoryDumpArgs dump_args = {MemoryDumpLevelOfDetail::DETAILED}; - ProcessMemoryDump pmd(new MemoryDumpSessionState, dump_args); + ProcessMemoryDump pmd(new HeapProfilerSerializationState, dump_args); pmd.CreateAllocatorDump("foo_allocator"); pmd.CreateAllocatorDump("bar_allocator/heap"); ASSERT_DEATH(pmd.CreateAllocatorDump("foo_allocator"), ""); diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc index 1cb0e923d163b9..5272b2f09bd297 100644 --- a/base/trace_event/memory_dump_manager.cc +++ b/base/trace_event/memory_dump_manager.cc @@ -28,12 +28,12 @@ #include "base/trace_event/heap_profiler.h" #include "base/trace_event/heap_profiler_allocation_context_tracker.h" #include "base/trace_event/heap_profiler_event_filter.h" +#include "base/trace_event/heap_profiler_serialization_state.h" #include "base/trace_event/heap_profiler_stack_frame_deduplicator.h" #include "base/trace_event/heap_profiler_type_name_deduplicator.h" #include "base/trace_event/malloc_dump_provider.h" #include "base/trace_event/memory_dump_provider.h" #include "base/trace_event/memory_dump_scheduler.h" -#include "base/trace_event/memory_dump_session_state.h" #include "base/trace_event/memory_infra_background_whitelist.h" #include "base/trace_event/memory_peak_detector.h" #include "base/trace_event/memory_tracing_observer.h" @@ -83,29 +83,33 @@ void FillOsDumpFromProcessMemoryDump( } // Proxy class which wraps a ConvertableToTraceFormat owned by the -// |session_state| into a proxy object that can be added to the trace event log. -// This is to solve the problem that the MemoryDumpSessionState is refcounted -// but the tracing subsystem wants a std::unique_ptr. +// |heap_profiler_serialization_state| into a proxy object that can be added to +// the trace event log. This is to solve the problem that the +// HeapProfilerSerializationState is refcounted but the tracing subsystem wants +// a std::unique_ptr. template struct SessionStateConvertableProxy : public ConvertableToTraceFormat { - using GetterFunctPtr = T* (MemoryDumpSessionState::*)() const; + using GetterFunctPtr = T* (HeapProfilerSerializationState::*)() const; - SessionStateConvertableProxy( - scoped_refptr session_state, - GetterFunctPtr getter_function) - : session_state(session_state), getter_function(getter_function) {} + SessionStateConvertableProxy(scoped_refptr + heap_profiler_serialization_state, + GetterFunctPtr getter_function) + : heap_profiler_serialization_state(heap_profiler_serialization_state), + getter_function(getter_function) {} void AppendAsTraceFormat(std::string* out) const override { - return (session_state.get()->*getter_function)()->AppendAsTraceFormat(out); + return (heap_profiler_serialization_state.get()->*getter_function)() + ->AppendAsTraceFormat(out); } void EstimateTraceMemoryOverhead( TraceEventMemoryOverhead* overhead) override { - return (session_state.get()->*getter_function)() + return (heap_profiler_serialization_state.get()->*getter_function)() ->EstimateTraceMemoryOverhead(overhead); } - scoped_refptr session_state; + scoped_refptr + heap_profiler_serialization_state; GetterFunctPtr const getter_function; }; @@ -407,11 +411,9 @@ void MemoryDumpManager::RequestGlobalDump( const GlobalMemoryDumpCallback& callback) { // Bail out immediately if tracing is not enabled at all or if the dump mode // is not allowed. - if (!UNLIKELY(subtle::NoBarrier_Load(&is_enabled_)) || - !IsDumpModeAllowed(level_of_detail)) { + if (!UNLIKELY(subtle::NoBarrier_Load(&is_enabled_))) { VLOG(1) << kLogPrefix << " failed because " << kTraceCategory - << " tracing category is not enabled or the requested dump mode is " - "not allowed by trace config."; + << " tracing category is not enabled."; if (!callback.is_null()) callback.Run(0u /* guid */, false /* success */); return; @@ -500,15 +502,9 @@ void MemoryDumpManager::CreateProcessDump( AutoLock lock(lock_); pmd_async_state.reset(new ProcessMemoryDumpAsyncState( - args, dump_providers_, session_state_, callback, + args, dump_providers_, heap_profiler_serialization_state_, callback, GetOrCreateBgTaskRunnerLocked())); - // Safety check to prevent reaching here without calling RequestGlobalDump, - // with disallowed modes. If |session_state_| is null then tracing is - // disabled. - CHECK(!session_state_ || - session_state_->IsDumpModeAllowed(args.level_of_detail)); - // If enabled, holds back the peak detector resetting its estimation window. MemoryPeakDetector::GetInstance()->Throttle(); } @@ -775,50 +771,47 @@ void MemoryDumpManager::FinalizeDumpAndAddToTrace( void MemoryDumpManager::Enable( const TraceConfig::MemoryDumpConfig& memory_dump_config) { - - scoped_refptr session_state = - new MemoryDumpSessionState; - session_state->SetAllowedDumpModes(memory_dump_config.allowed_dump_modes); - session_state->set_heap_profiler_breakdown_threshold_bytes( - memory_dump_config.heap_profiler_options.breakdown_threshold_bytes); + scoped_refptr + heap_profiler_serialization_state = new HeapProfilerSerializationState; + heap_profiler_serialization_state + ->set_heap_profiler_breakdown_threshold_bytes( + memory_dump_config.heap_profiler_options.breakdown_threshold_bytes); if (heap_profiling_enabled_) { // If heap profiling is enabled, the stack frame deduplicator and type name // deduplicator will be in use. Add a metadata events to write the frames // and type IDs. - session_state->SetStackFrameDeduplicator( + heap_profiler_serialization_state->SetStackFrameDeduplicator( WrapUnique(new StackFrameDeduplicator)); - session_state->SetTypeNameDeduplicator( + heap_profiler_serialization_state->SetTypeNameDeduplicator( WrapUnique(new TypeNameDeduplicator)); TRACE_EVENT_API_ADD_METADATA_EVENT( TraceLog::GetCategoryGroupEnabled("__metadata"), "stackFrames", "stackFrames", MakeUnique>( - session_state, &MemoryDumpSessionState::stack_frame_deduplicator)); + heap_profiler_serialization_state, + &HeapProfilerSerializationState::stack_frame_deduplicator)); TRACE_EVENT_API_ADD_METADATA_EVENT( TraceLog::GetCategoryGroupEnabled("__metadata"), "typeNames", "typeNames", MakeUnique>( - session_state, &MemoryDumpSessionState::type_name_deduplicator)); + heap_profiler_serialization_state, + &HeapProfilerSerializationState::type_name_deduplicator)); } AutoLock lock(lock_); // At this point we must have the ability to request global dumps. DCHECK(!request_dump_function_.is_null()); - session_state_ = session_state; + heap_profiler_serialization_state_ = heap_profiler_serialization_state; subtle::NoBarrier_Store(&is_enabled_, 1); MemoryDumpScheduler::Config periodic_config; bool peak_detector_configured = false; for (const auto& trigger : memory_dump_config.triggers) { - if (!session_state_->IsDumpModeAllowed(trigger.level_of_detail)) { - NOTREACHED(); - continue; - } if (trigger.trigger_type == MemoryDumpType::PERIODIC_INTERVAL) { if (periodic_config.triggers.empty()) { periodic_config.callback = BindRepeating(&OnPeriodicSchedulerTick); @@ -869,25 +862,20 @@ void MemoryDumpManager::Disable() { AutoLock lock(lock_); MemoryDumpScheduler::GetInstance()->Stop(); MemoryPeakDetector::GetInstance()->TearDown(); - session_state_ = nullptr; + heap_profiler_serialization_state_ = nullptr; } } -bool MemoryDumpManager::IsDumpModeAllowed(MemoryDumpLevelOfDetail dump_mode) { - AutoLock lock(lock_); - if (!session_state_) - return false; - return session_state_->IsDumpModeAllowed(dump_mode); -} - MemoryDumpManager::ProcessMemoryDumpAsyncState::ProcessMemoryDumpAsyncState( MemoryDumpRequestArgs req_args, const MemoryDumpProviderInfo::OrderedSet& dump_providers, - scoped_refptr session_state, + scoped_refptr + heap_profiler_serialization_state, ProcessMemoryDumpCallback callback, scoped_refptr dump_thread_task_runner) : req_args(req_args), - session_state(std::move(session_state)), + heap_profiler_serialization_state( + std::move(heap_profiler_serialization_state)), callback(callback), dump_successful(true), callback_task_runner(ThreadTaskRunnerHandle::Get()), @@ -905,7 +893,7 @@ ProcessMemoryDump* MemoryDumpManager::ProcessMemoryDumpAsyncState:: auto iter = process_dumps.find(pid); if (iter == process_dumps.end()) { std::unique_ptr new_pmd( - new ProcessMemoryDump(session_state, dump_args)); + new ProcessMemoryDump(heap_profiler_serialization_state, dump_args)); iter = process_dumps.insert(std::make_pair(pid, std::move(new_pmd))).first; } return iter->second.get(); diff --git a/base/trace_event/memory_dump_manager.h b/base/trace_event/memory_dump_manager.h index 03192bfa91efd9..6c71a247a68aad 100644 --- a/base/trace_event/memory_dump_manager.h +++ b/base/trace_event/memory_dump_manager.h @@ -42,7 +42,7 @@ namespace trace_event { class MemoryTracingObserver; class MemoryDumpProvider; -class MemoryDumpSessionState; +class HeapProfilerSerializationState; // This is the interface exposed to the rest of the codebase to deal with // memory tracing. The main entry point for clients is represented by @@ -147,18 +147,12 @@ class BASE_EXPORT MemoryDumpManager { // Enable heap profiling if kEnableHeapProfiling is specified. void EnableHeapProfilingIfNeeded(); - // Returns true if the dump mode is allowed for current tracing session. - bool IsDumpModeAllowed(MemoryDumpLevelOfDetail dump_mode); - // Lets tests see if a dump provider is registered. bool IsDumpProviderRegisteredForTesting(MemoryDumpProvider*); - // Returns the MemoryDumpSessionState object, which is shared by all the - // ProcessMemoryDump and MemoryAllocatorDump instances through all the tracing - // session lifetime. - const scoped_refptr& session_state_for_testing() - const { - return session_state_; + const scoped_refptr& + heap_profiler_serialization_state_for_testing() const { + return heap_profiler_serialization_state_; } // Returns a unique id for identifying the processes. The id can be @@ -197,7 +191,8 @@ class BASE_EXPORT MemoryDumpManager { ProcessMemoryDumpAsyncState( MemoryDumpRequestArgs req_args, const MemoryDumpProviderInfo::OrderedSet& dump_providers, - scoped_refptr session_state, + scoped_refptr + heap_profiler_serialization_state, ProcessMemoryDumpCallback callback, scoped_refptr dump_thread_task_runner); ~ProcessMemoryDumpAsyncState(); @@ -221,8 +216,11 @@ class BASE_EXPORT MemoryDumpManager { // and becomes empty at the end, when all dump providers have been invoked. std::vector> pending_dump_providers; - // The trace-global session state. - scoped_refptr session_state; + // The HeapProfilerSerializationState object, which is shared by all + // the ProcessMemoryDump and MemoryAllocatorDump instances through all the + // tracing session lifetime. + scoped_refptr + heap_profiler_serialization_state; // Callback passed to the initial call to CreateProcessDump(). ProcessMemoryDumpCallback callback; @@ -293,7 +291,8 @@ class BASE_EXPORT MemoryDumpManager { MemoryDumpProviderInfo::OrderedSet dump_providers_; // Shared among all the PMDs to keep state scoped to the tracing session. - scoped_refptr session_state_; + scoped_refptr + heap_profiler_serialization_state_; std::unique_ptr tracing_observer_; diff --git a/base/trace_event/memory_dump_manager_unittest.cc b/base/trace_event/memory_dump_manager_unittest.cc index 48ae4547c7120f..a75eab2f7cabfd 100644 --- a/base/trace_event/memory_dump_manager_unittest.cc +++ b/base/trace_event/memory_dump_manager_unittest.cc @@ -157,14 +157,15 @@ class MockMemoryDumpProvider : public MemoryDumpProvider { MockMemoryDumpProvider() : enable_mock_destructor(false) { ON_CALL(*this, OnMemoryDump(_, _)) - .WillByDefault(Invoke([](const MemoryDumpArgs&, - ProcessMemoryDump* pmd) -> bool { - // |session_state| should not be null under any circumstances when - // invoking a memory dump. The problem might arise in race conditions - // like crbug.com/600570 . - EXPECT_TRUE(pmd->session_state().get() != nullptr); - return true; - })); + .WillByDefault( + Invoke([](const MemoryDumpArgs&, ProcessMemoryDump* pmd) -> bool { + // |heap_profiler_serialization_state| should not be null under + // any circumstances when invoking a memory dump. The problem + // might arise in race conditions like crbug.com/600570 . + EXPECT_TRUE(pmd->heap_profiler_serialization_state().get() != + nullptr); + return true; + })); ON_CALL(*this, PollFastMemoryTotal(_)) .WillByDefault( @@ -407,8 +408,9 @@ TEST_F(MemoryDumpManagerTest, CheckMemoryDumpArgs) { mdm_->UnregisterDumpProvider(&mdp); } -// Checks that the SharedSessionState object is acqually shared over time. -TEST_F(MemoryDumpManagerTest, SharedSessionState) { +// Checks that the HeapProfilerSerializationState object is actually +// shared over time. +TEST_F(MemoryDumpManagerTest, HeapProfilerSerializationState) { InitializeMemoryDumpManager(false /* is_coordinator */); MockMemoryDumpProvider mdp1; MockMemoryDumpProvider mdp2; @@ -416,23 +418,27 @@ TEST_F(MemoryDumpManagerTest, SharedSessionState) { RegisterDumpProvider(&mdp2, nullptr); EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory); - const MemoryDumpSessionState* session_state = - mdm_->session_state_for_testing().get(); + const HeapProfilerSerializationState* heap_profiler_serialization_state = + mdm_->heap_profiler_serialization_state_for_testing().get(); EXPECT_CALL(global_dump_handler_, RequestGlobalMemoryDump(_, _)).Times(2); EXPECT_CALL(mdp1, OnMemoryDump(_, _)) .Times(2) - .WillRepeatedly(Invoke([session_state](const MemoryDumpArgs&, - ProcessMemoryDump* pmd) -> bool { - EXPECT_EQ(session_state, pmd->session_state().get()); - return true; - })); + .WillRepeatedly( + Invoke([heap_profiler_serialization_state]( + const MemoryDumpArgs&, ProcessMemoryDump* pmd) -> bool { + EXPECT_EQ(heap_profiler_serialization_state, + pmd->heap_profiler_serialization_state().get()); + return true; + })); EXPECT_CALL(mdp2, OnMemoryDump(_, _)) .Times(2) - .WillRepeatedly(Invoke([session_state](const MemoryDumpArgs&, - ProcessMemoryDump* pmd) -> bool { - EXPECT_EQ(session_state, pmd->session_state().get()); - return true; - })); + .WillRepeatedly( + Invoke([heap_profiler_serialization_state]( + const MemoryDumpArgs&, ProcessMemoryDump* pmd) -> bool { + EXPECT_EQ(heap_profiler_serialization_state, + pmd->heap_profiler_serialization_state().get()); + return true; + })); for (int i = 0; i < 2; ++i) { RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED, @@ -1250,6 +1256,11 @@ TEST_F(MemoryDumpManagerTest, TestWhitelistingMDP) { TEST_F(MemoryDumpManagerTest, TestBackgroundTracingSetup) { InitializeMemoryDumpManager(true /* is_coordinator */); + // We now need an MDP to hit the code path where the dump will be rejected + // since this happens at the point you try to serialize a process dump. + MockMemoryDumpProvider mdp; + RegisterDumpProvider(&mdp, ThreadTaskRunnerHandle::Get()); + RunLoop run_loop; auto test_task_runner = ThreadTaskRunnerHandle::Get(); auto quit_closure = run_loop.QuitClosure(); @@ -1272,6 +1283,8 @@ TEST_F(MemoryDumpManagerTest, TestBackgroundTracingSetup) { TraceConfigMemoryTestUtil::GetTraceConfig_BackgroundTrigger( 1 /* period_ms */)); + run_loop.Run(); + // Only background mode dumps should be allowed with the trace config. last_callback_success_ = false; RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED, @@ -1283,7 +1296,6 @@ TEST_F(MemoryDumpManagerTest, TestBackgroundTracingSetup) { EXPECT_FALSE(last_callback_success_); ASSERT_TRUE(IsPeriodicDumpingEnabled()); - run_loop.Run(); DisableTracing(); } @@ -1323,8 +1335,8 @@ TEST_F(MemoryDumpManagerTest, TestSummaryComputation) { MockMemoryDumpProvider mdp; RegisterDumpProvider(&mdp, ThreadTaskRunnerHandle::Get()); - const MemoryDumpSessionState* session_state = - mdm_->session_state_for_testing().get(); + const HeapProfilerSerializationState* heap_profiler_serialization_state = + mdm_->heap_profiler_serialization_state_for_testing().get(); EXPECT_CALL(global_dump_handler_, RequestGlobalMemoryDump(_, _)) .WillOnce(Invoke([this](const MemoryDumpRequestArgs& args, @@ -1338,8 +1350,9 @@ TEST_F(MemoryDumpManagerTest, TestSummaryComputation) { EXPECT_CALL(mdp, OnMemoryDump(_, _)) .Times(1) - .WillRepeatedly(Invoke([session_state](const MemoryDumpArgs&, - ProcessMemoryDump* pmd) -> bool { + .WillRepeatedly(Invoke([heap_profiler_serialization_state]( + const MemoryDumpArgs&, + ProcessMemoryDump* pmd) -> bool { auto* size = MemoryAllocatorDump::kNameSize; auto* bytes = MemoryAllocatorDump::kUnitsBytes; const uint32_t kB = 1024; diff --git a/base/trace_event/memory_tracing_observer.cc b/base/trace_event/memory_tracing_observer.cc index a5b60e48fd7e0d..8847b8045cf35c 100644 --- a/base/trace_event/memory_tracing_observer.cc +++ b/base/trace_event/memory_tracing_observer.cc @@ -4,6 +4,7 @@ #include "base/trace_event/memory_tracing_observer.h" +#include "base/memory/ptr_util.h" #include "base/trace_event/memory_dump_manager.h" #include "base/trace_event/trace_event_argument.h" @@ -56,11 +57,15 @@ void MemoryTracingObserver::OnTraceLogEnabled() { const TraceConfig::MemoryDumpConfig& memory_dump_config = trace_config.memory_dump_config(); + memory_dump_config_ = + MakeUnique(memory_dump_config); + memory_dump_manager_->Enable(memory_dump_config); } void MemoryTracingObserver::OnTraceLogDisabled() { memory_dump_manager_->Disable(); + memory_dump_config_.reset(); } bool MemoryTracingObserver::AddDumpToTraceIfEnabled( @@ -71,6 +76,10 @@ bool MemoryTracingObserver::AddDumpToTraceIfEnabled( // dump then ignoring the result. if (!IsMemoryInfraTracingEnabled()) return false; + // If the dump mode is too detailed don't add to trace to avoid accidentally + // including PII. + if (!IsDumpModeAllowed(req_args->level_of_detail)) + return false; CHECK_NE(MemoryDumpType::SUMMARY_ONLY, req_args->dump_type); @@ -94,5 +103,12 @@ bool MemoryTracingObserver::AddDumpToTraceIfEnabled( return true; } +bool MemoryTracingObserver::IsDumpModeAllowed( + MemoryDumpLevelOfDetail dump_mode) const { + if (!memory_dump_config_) + return false; + return memory_dump_config_->allowed_dump_modes.count(dump_mode) != 0; +} + } // namespace trace_event } // namespace base diff --git a/base/trace_event/memory_tracing_observer.h b/base/trace_event/memory_tracing_observer.h index 1efea18c4d5525..11276763c1a782 100644 --- a/base/trace_event/memory_tracing_observer.h +++ b/base/trace_event/memory_tracing_observer.h @@ -33,8 +33,12 @@ class BASE_EXPORT MemoryTracingObserver const ProcessMemoryDump*); private: + // Returns true if the dump mode is allowed for current tracing session. + bool IsDumpModeAllowed(MemoryDumpLevelOfDetail) const; + MemoryDumpManager* const memory_dump_manager_; TraceLog* const trace_log_; + std::unique_ptr memory_dump_config_; DISALLOW_COPY_AND_ASSIGN(MemoryTracingObserver); }; diff --git a/base/trace_event/process_memory_dump.cc b/base/trace_event/process_memory_dump.cc index da7bcad0959f9e..0a2bf66620f4a1 100644 --- a/base/trace_event/process_memory_dump.cc +++ b/base/trace_event/process_memory_dump.cc @@ -12,6 +12,7 @@ #include "base/process/process_metrics.h" #include "base/strings/stringprintf.h" #include "base/trace_event/heap_profiler_heap_dump_writer.h" +#include "base/trace_event/heap_profiler_serialization_state.h" #include "base/trace_event/memory_infra_background_whitelist.h" #include "base/trace_event/process_memory_totals.h" #include "base/trace_event/trace_event_argument.h" @@ -151,11 +152,13 @@ size_t ProcessMemoryDump::CountResidentBytes(void* start_address, #endif // defined(COUNT_RESIDENT_BYTES_SUPPORTED) ProcessMemoryDump::ProcessMemoryDump( - scoped_refptr session_state, + scoped_refptr + heap_profiler_serialization_state, const MemoryDumpArgs& dump_args) : has_process_totals_(false), has_process_mmaps_(false), - session_state_(std::move(session_state)), + heap_profiler_serialization_state_( + std::move(heap_profiler_serialization_state)), dump_args_(dump_args) {} ProcessMemoryDump::~ProcessMemoryDump() {} @@ -252,7 +255,7 @@ void ProcessMemoryDump::DumpHeapUsage( if (!metrics_by_context.empty()) { DCHECK_EQ(0ul, heap_dumps_.count(allocator_name)); std::unique_ptr heap_dump = ExportHeapDump( - metrics_by_context, *session_state()); + metrics_by_context, *heap_profiler_serialization_state()); heap_dumps_[allocator_name] = std::move(heap_dump); } diff --git a/base/trace_event/process_memory_dump.h b/base/trace_event/process_memory_dump.h index 5a0f4c5aca9e8c..b16930fa6c2913 100644 --- a/base/trace_event/process_memory_dump.h +++ b/base/trace_event/process_memory_dump.h @@ -14,10 +14,10 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_vector.h" +#include "base/trace_event/heap_profiler_serialization_state.h" #include "base/trace_event/memory_allocator_dump.h" #include "base/trace_event/memory_allocator_dump_guid.h" #include "base/trace_event/memory_dump_request_args.h" -#include "base/trace_event/memory_dump_session_state.h" #include "base/trace_event/process_memory_maps.h" #include "base/trace_event/process_memory_totals.h" #include "build/build_config.h" @@ -31,7 +31,7 @@ namespace base { namespace trace_event { -class MemoryDumpSessionState; +class HeapProfilerSerializationState; class TracedValue; // ProcessMemoryDump is as a strongly typed container which holds the dumps @@ -67,7 +67,8 @@ class BASE_EXPORT ProcessMemoryDump { static size_t CountResidentBytes(void* start_address, size_t mapped_size); #endif - ProcessMemoryDump(scoped_refptr session_state, + ProcessMemoryDump(scoped_refptr + heap_profiler_serialization_state, const MemoryDumpArgs& dump_args); ~ProcessMemoryDump(); @@ -148,8 +149,9 @@ class BASE_EXPORT ProcessMemoryDump { void AddSuballocation(const MemoryAllocatorDumpGuid& source, const std::string& target_node_name); - const scoped_refptr& session_state() const { - return session_state_; + const scoped_refptr& + heap_profiler_serialization_state() const { + return heap_profiler_serialization_state_; } // Removes all the MemoryAllocatorDump(s) contained in this instance. This @@ -198,7 +200,8 @@ class BASE_EXPORT ProcessMemoryDump { HeapDumpsMap heap_dumps_; // State shared among all PMDs instances created in a given trace session. - scoped_refptr session_state_; + scoped_refptr + heap_profiler_serialization_state_; // Keeps track of relationships between MemoryAllocatorDump(s). std::vector allocator_dumps_edges_; diff --git a/base/trace_event/process_memory_dump_unittest.cc b/base/trace_event/process_memory_dump_unittest.cc index 4ee6508190c140..70f897c6b39dcc 100644 --- a/base/trace_event/process_memory_dump_unittest.cc +++ b/base/trace_event/process_memory_dump_unittest.cc @@ -94,22 +94,22 @@ TEST(ProcessMemoryDumpTest, TakeAllDumpsFrom) { metrics_by_context[AllocationContext()] = { 1, 1 }; TraceEventMemoryOverhead overhead; - scoped_refptr session_state = - new MemoryDumpSessionState; - session_state->SetStackFrameDeduplicator( + scoped_refptr + heap_profiler_serialization_state = new HeapProfilerSerializationState; + heap_profiler_serialization_state->SetStackFrameDeduplicator( WrapUnique(new StackFrameDeduplicator)); - session_state->SetTypeNameDeduplicator( + heap_profiler_serialization_state->SetTypeNameDeduplicator( WrapUnique(new TypeNameDeduplicator)); - std::unique_ptr pmd1( - new ProcessMemoryDump(session_state.get(), kDetailedDumpArgs)); + std::unique_ptr pmd1(new ProcessMemoryDump( + heap_profiler_serialization_state.get(), kDetailedDumpArgs)); auto* mad1_1 = pmd1->CreateAllocatorDump("pmd1/mad1"); auto* mad1_2 = pmd1->CreateAllocatorDump("pmd1/mad2"); pmd1->AddOwnershipEdge(mad1_1->guid(), mad1_2->guid()); pmd1->DumpHeapUsage(metrics_by_context, overhead, "pmd1/heap_dump1"); pmd1->DumpHeapUsage(metrics_by_context, overhead, "pmd1/heap_dump2"); - std::unique_ptr pmd2( - new ProcessMemoryDump(session_state.get(), kDetailedDumpArgs)); + std::unique_ptr pmd2(new ProcessMemoryDump( + heap_profiler_serialization_state.get(), kDetailedDumpArgs)); auto* mad2_1 = pmd2->CreateAllocatorDump("pmd2/mad1"); auto* mad2_2 = pmd2->CreateAllocatorDump("pmd2/mad2"); pmd2->AddOwnershipEdge(mad2_1->guid(), mad2_2->guid()); diff --git a/tools/gn/bootstrap/bootstrap.py b/tools/gn/bootstrap/bootstrap.py index 399cceaf38106d..3901dd4eb545c7 100755 --- a/tools/gn/bootstrap/bootstrap.py +++ b/tools/gn/bootstrap/bootstrap.py @@ -534,6 +534,7 @@ def write_gn_ninja(path, root_gen_dir, options): 'base/trace_event/heap_profiler_allocation_register.cc', 'base/trace_event/heap_profiler_event_filter.cc', 'base/trace_event/heap_profiler_heap_dump_writer.cc', + 'base/trace_event/heap_profiler_serialization_state.cc', 'base/trace_event/heap_profiler_stack_frame_deduplicator.cc', 'base/trace_event/heap_profiler_type_name_deduplicator.cc', 'base/trace_event/memory_allocator_dump.cc', @@ -542,7 +543,6 @@ def write_gn_ninja(path, root_gen_dir, options): 'base/trace_event/memory_dump_provider_info.cc', 'base/trace_event/memory_dump_request_args.cc', 'base/trace_event/memory_dump_scheduler.cc', - 'base/trace_event/memory_dump_session_state.cc', 'base/trace_event/memory_infra_background_whitelist.cc', 'base/trace_event/memory_peak_detector.cc', 'base/trace_event/memory_tracing_observer.cc',