memory-infra: Start disentangling tracing from memory-infra

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 7e62064ec6a08e25fcdbc0f79cb3065ed36b1e4a..74a4dd727739192027bbf1d402389fcab6b470e2 100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -35,6 +35,7 @@
 #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"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_argument.h"
@@ -49,10 +50,6 @@ namespace trace_event {
 
 namespace {
 
-const int kTraceEventNumArgs = 1;
-const char* kTraceEventArgNames[] = {"dumps"};
-const unsigned char kTraceEventArgTypes[] = {TRACE_VALUE_TYPE_CONVERTABLE};
-
 StaticAtomicSequenceNumber g_next_guid;
 MemoryDumpManager* g_instance_for_testing = nullptr;
 
@@ -198,7 +195,6 @@ MemoryDumpManager::MemoryDumpManager()
 }
 
 MemoryDumpManager::~MemoryDumpManager() {
-  TraceLog::GetInstance()->RemoveEnabledStateObserver(this);
 }
 
 void MemoryDumpManager::EnableHeapProfilingIfNeeded() {
@@ -289,14 +285,8 @@ void MemoryDumpManager::Initialize(
                                         TraceLog::FILTERING_MODE);
   }
 
-  // If tracing was enabled before initializing MemoryDumpManager, we missed the
-  // OnTraceLogEnabled() event. Synthetize it so we can late-join the party.
-  // IsEnabled is called before adding observer to avoid calling
-  // OnTraceLogEnabled twice.
-  bool is_tracing_already_enabled = TraceLog::GetInstance()->IsEnabled();
-  TraceLog::GetInstance()->AddEnabledStateObserver(this);
-  if (is_tracing_already_enabled)
-    OnTraceLogEnabled();
+  trace_log_observer_ =
+      MakeUnique<MemoryTracingObserver>(TraceLog::GetInstance(), this);
 }
 
 void MemoryDumpManager::RegisterDumpProvider(
@@ -745,23 +735,9 @@ void MemoryDumpManager::FinalizeDumpAndAddToTrace(
   for (const auto& kv : pmd_async_state->process_dumps) {
     ProcessId pid = kv.first;  // kNullProcessId for the current process.
     ProcessMemoryDump* process_memory_dump = kv.second.get();
-    std::unique_ptr<TracedValue> traced_value(new TracedValue);
-    process_memory_dump->AsValueInto(traced_value.get());
-    traced_value->SetString("level_of_detail",
-                            MemoryDumpLevelOfDetailToString(
-                                pmd_async_state->req_args.level_of_detail));
-    const char* const event_name =
-        MemoryDumpTypeToString(pmd_async_state->req_args.dump_type);
-
-    std::unique_ptr<ConvertableToTraceFormat> event_value(
-        std::move(traced_value));
-    TRACE_EVENT_API_ADD_TRACE_EVENT_WITH_PROCESS_ID(
-        TRACE_EVENT_PHASE_MEMORY_DUMP,
-        TraceLog::GetCategoryGroupEnabled(kTraceCategory), event_name,
-        trace_event_internal::kGlobalScope, dump_guid, pid,
-        kTraceEventNumArgs, kTraceEventArgNames,
-        kTraceEventArgTypes, nullptr /* arg_values */, &event_value,
-        TRACE_EVENT_FLAG_HAS_ID);
+
+    MemoryTracingObserver::MaybeAddDumpToTrace(&pmd_async_state->req_args, pid,

[Primiano Tucci (use gerrit), 2017/04/21 10:10:26]

I'd call this AddDumpToTraceIf(Tracing)Enabled to make it sound a bit less
mysterious. I know that there is an extra subtlety about the max_dump_level, but
at least AddDumpToTraceIf(Tracing)Enabled gives a hint about the general use
case.

[hjd, 2017/04/21 11:51:57]

Done.

+                                               process_memory_dump);
 
     // TODO(hjd): Transitional until we send the full PMD. See crbug.com/704203
     // Don't try to fill the struct in detailed mode since it is hard to avoid
@@ -792,14 +768,6 @@ void MemoryDumpManager::FinalizeDumpAndAddToTrace(
     }
   }
 
-  bool tracing_still_enabled;
-  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, pmd_async_state->dump_successful);
     pmd_async_state->callback.Reset();
@@ -809,12 +777,8 @@ void MemoryDumpManager::FinalizeDumpAndAddToTrace(
                                   TRACE_ID_LOCAL(dump_guid));
 }
 
-void MemoryDumpManager::OnTraceLogEnabled() {
-  bool enabled;
-  TRACE_EVENT_CATEGORY_GROUP_ENABLED(kTraceCategory, &enabled);
-  if (!enabled)
-    return;
-
+void MemoryDumpManager::Enable(
+    const TraceConfig::MemoryDumpConfig& memory_dump_config) {

[Primiano Tucci (use gerrit), 2017/04/21 10:10:26]

okay for the moment, but I think that in the next CL we should rip this out
(i.e. the TraceConfig argument) and move the IsDumpModeAllowed to the observer.

[hjd, 2017/04/21 11:51:57]

Acknowledged.

   // Initialize the TraceLog for the current thread. This is to avoid that the
   // TraceLog memory dump provider is registered lazily in the PostTask() below
   // while the |lock_| is taken;
@@ -827,10 +791,6 @@ void MemoryDumpManager::OnTraceLogEnabled() {
     return;
   }
 
-  const TraceConfig& trace_config =
-      TraceLog::GetInstance()->GetCurrentTraceConfig();
-  const TraceConfig::MemoryDumpConfig& memory_dump_config =
-      trace_config.memory_dump_config();
   scoped_refptr<MemoryDumpSessionState> session_state =
       new MemoryDumpSessionState;
   session_state->SetAllowedDumpModes(memory_dump_config.allowed_dump_modes);
@@ -915,7 +875,7 @@ void MemoryDumpManager::OnTraceLogEnabled() {
   }
 }
 
-void MemoryDumpManager::OnTraceLogDisabled() {
+void MemoryDumpManager::Disable() {
   // There might be a memory dump in progress while this happens. Therefore,
   // ensure that the MDM state which depends on the tracing enabled / disabled
   // state is always accessed by the dumping methods holding the |lock_|.