memory-infra: Move dump level check to observer and rename session state

- Rename session_state to heap_profiler_serialization_state in MemoryDumpManager.
- Move IsDumpModeAllowed check from MemoryDumpManager to MemoryTracingObserver.

This further decouples MemoryDumpManager from tracing.

BUG=703184

Review-Url: https://codereview.chromium.org/2861133002
Cr-Commit-Position: refs/heads/master@{#469721}

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",

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<TracedValue> Serialize(const std::set<Entry>& entries) {
 std::unique_ptr<TracedValue> ExportHeapDump(
     const std::unordered_map<AllocationContext, AllocationMetrics>&
         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));
 }
 

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<TracedValue> ExportHeapDump(
     const std::unordered_map<AllocationContext, AllocationMetrics>&
         metrics_by_context,
-    const MemoryDumpSessionState& session_state);
+    const HeapProfilerSerializationState& heap_profiler_serialization_state);
 
 namespace internal {
 

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"

base/trace_event/memory_dump_session_state.cc → 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<StackFrameDeduplicator> stack_frame_deduplicator) {
   DCHECK(!stack_frame_deduplicator_);
   stack_frame_deduplicator_ = std::move(stack_frame_deduplicator);
 }
 
-void MemoryDumpSessionState::SetTypeNameDeduplicator(
+void HeapProfilerSerializationState::SetTypeNameDeduplicator(
     std::unique_ptr<TypeNameDeduplicator> type_name_deduplicator) {
   DCHECK(!type_name_deduplicator_);
   type_name_deduplicator_ = std::move(type_name_deduplicator);
 }
 
-void MemoryDumpSessionState::SetAllowedDumpModes(
-    std::set<MemoryDumpLevelOfDetail> 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

base/trace_event/memory_dump_session_state.h → 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 <memory>
 #include <set>
@@ -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<MemoryDumpSessionState> {
+class BASE_EXPORT HeapProfilerSerializationState
+    : public RefCountedThreadSafe<HeapProfilerSerializationState> {
  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>;
-  ~MemoryDumpSessionState();
+  friend class RefCountedThreadSafe<HeapProfilerSerializationState>;
+  ~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<TypeNameDeduplicator> type_name_deduplicator_;
 
-  std::set<MemoryDumpLevelOfDetail> 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

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"), "");

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<ConvertableToTraceFormat>.
+// |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<ConvertableToTraceFormat>.
 template <typename T>
 struct SessionStateConvertableProxy : public ConvertableToTraceFormat {
-  using GetterFunctPtr = T* (MemoryDumpSessionState::*)() const;
+  using GetterFunctPtr = T* (HeapProfilerSerializationState::*)() const;
 
-  SessionStateConvertableProxy(
-      scoped_refptr<MemoryDumpSessionState> session_state,
-      GetterFunctPtr getter_function)
-      : session_state(session_state), getter_function(getter_function) {}
+  SessionStateConvertableProxy(scoped_refptr<HeapProfilerSerializationState>
+                                   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<MemoryDumpSessionState> session_state;
+  scoped_refptr<HeapProfilerSerializationState>
+      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<MemoryDumpSessionState> 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<HeapProfilerSerializationState>
+      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<SessionStateConvertableProxy<StackFrameDeduplicator>>(
-            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<SessionStateConvertableProxy<TypeNameDeduplicator>>(
-            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<MemoryDumpSessionState> session_state,
+    scoped_refptr<HeapProfilerSerializationState>
+        heap_profiler_serialization_state,
     ProcessMemoryDumpCallback callback,
     scoped_refptr<SequencedTaskRunner> 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<ProcessMemoryDump> 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();