[tracing] Switch to new heap dump format.

base/trace_event/memory_dump_manager.cc

Index: base/trace_event/memory_dump_manager.cc
diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc
index 008b22a2d30be84f4e79db9b2c20de715c7c95ce..4a0d1c2e576f337c6ca7204aad8ae38549978785 100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -29,9 +29,7 @@
 #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/heap_profiler_event_writer.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"
@@ -83,37 +81,6 @@ void FillOsDumpFromProcessMemoryDump(
   }
 }
 
-// Proxy class which wraps a ConvertableToTraceFormat owned by the
-// |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* (HeapProfilerSerializationState::*)() const;
-
-  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 (heap_profiler_serialization_state.get()->*getter_function)()
-        ->AppendAsTraceFormat(out);
-  }
-
-  void EstimateTraceMemoryOverhead(
-      TraceEventMemoryOverhead* overhead) override {
-    return (heap_profiler_serialization_state.get()->*getter_function)()
-        ->EstimateTraceMemoryOverhead(overhead);
-  }
-
-  scoped_refptr<HeapProfilerSerializationState>
-      heap_profiler_serialization_state;
-  GetterFunctPtr const getter_function;
-};
-
 void OnPeakDetected(MemoryDumpLevelOfDetail level_of_detail) {
   MemoryDumpManager::GetInstance()->RequestGlobalDump(
       MemoryDumpType::PEAK_MEMORY_USAGE, level_of_detail);
@@ -780,6 +747,14 @@ void MemoryDumpManager::FinalizeDumpAndAddToTrace(
     }
   }
 
+  bool tracing_still_enabled;

[Primiano Tucci (use gerrit), 2017/05/30 18:50:53]

I think this is a rebase leftover. we don't want this code anymore, this will
break some UMA stuff now.

[DmitrySkiba, 2017/06/05 00:50:18]

Nice catch! Done.

+  TRACE_EVENT_CATEGORY_GROUP_ENABLED(kTraceCategory, &tracing_still_enabled);
+  if (!tracing_still_enabled) {
+    pmd_async_state->dump_successful = false;
+    VLOG(1) << kLogPrefix << " failed because tracing was disabled before"
+            << " the dump was completed";
+  }
+
   if (!pmd_async_state->callback.is_null()) {
     pmd_async_state->callback.Run(dump_guid, dump_successful, result);
     pmd_async_state->callback.Reset();
@@ -797,28 +772,13 @@ void MemoryDumpManager::SetupForTracing(
       ->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.
-    heap_profiler_serialization_state->SetStackFrameDeduplicator(
-        WrapUnique(new StackFrameDeduplicator));
-
-    heap_profiler_serialization_state->SetTypeNameDeduplicator(
-        WrapUnique(new TypeNameDeduplicator));
-
-    TRACE_EVENT_API_ADD_METADATA_EVENT(
-        TraceLog::GetCategoryGroupEnabled("__metadata"), "stackFrames",
-        "stackFrames",
-        MakeUnique<SessionStateConvertableProxy<StackFrameDeduplicator>>(
-            heap_profiler_serialization_state,
-            &HeapProfilerSerializationState::stack_frame_deduplicator));
-
-    TRACE_EVENT_API_ADD_METADATA_EVENT(
-        TraceLog::GetCategoryGroupEnabled("__metadata"), "typeNames",
-        "typeNames",
-        MakeUnique<SessionStateConvertableProxy<TypeNameDeduplicator>>(
-            heap_profiler_serialization_state,
-            &HeapProfilerSerializationState::type_name_deduplicator));
+    heap_profiler_serialization_state->CreateDeduplicators();
+    const TraceConfig& trace_config =
+        TraceLog::GetInstance()->GetCurrentTraceConfig();
+    // TODO(dskiba): support continuous mode (crbug.com/701052)
+    LOG_IF(ERROR, trace_config.GetTraceRecordMode() != RECORD_CONTINUOUSLY)

[Primiano Tucci (use gerrit), 2017/05/30 18:50:53]

FATAL (which causes a crash) or D/VLOG. any other logging is just spam.

Also I think the condition is reversed, shouldn't this be
trace_config.GetTraceRecordMode() == RECORD_CONTINUOUSLY ?
(You see, LOG(ERROR) are useless even for us two)

[DmitrySkiba, 2017/06/05 00:50:18]

I think it was your idea to use LOG_IF :)

"
Primiano Tucci 2017/03/09 11:47:45
I think  this should be a LOG_IF(ERROR, trace_config.GetTraceRecordMode() !=
RECORD_CONTINUOUSLY) not a dcheck.
"

Anyway, I'll make it a DLOG_IF and fix the condition.

+        << "Heap profile format is incremental and doesn't yet fully support "
+        << "continuous mode.";
   }
 
   AutoLock lock(lock_);