[tracing] Fix MemoryDumpManager to support startup tracing

This CL simplifies a lot the design of the MemoryDumpManager (MDM)
initialization (\o/). Before this change the MDM had a two-stage
initialization: 1. Initialize() that was registering to the
TraceLog enabled stateobserver; 2. SetDelegate() that was setting
the inversion-of-control delegate.
This design was fragile as it required to think about the two possible
sequences of events (1 -> 2 or 2 -> 1) and not really necessary.
The simplification herein introduced consists in:
 - Drop the IsCoordinatorProcess() method from the delegate. That
   information is not supposed to change over time and is known
   at MDM initialization time.
 - Merge the SetDelegate() into the Initialize() call. There is no
   point allowing to initialize the MDM without a delegate, so
   why bother? Moved all the initialization to the point where the
   delegate (concretelly the TracingControllerImpl) is ready.

As a side effect, startup tracing now works properly (tests are
coming in a separate CL).

BUG=524057
TEST=--trace-startup=-*,disabled-by-default-memory-infra --trace-startup-file=/tmp/startup.json

Review URL: https://codereview.chromium.org/1333873002

Cr-Commit-Position: refs/heads/master@{#348168}

10 files changed, 109 insertions, 105 deletions

diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc
index 55c4748..7fb8825 100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -106,6 +106,7 @@ void MemoryDumpManager::SetInstanceForTesting(MemoryDumpManager* instance) {
 
 MemoryDumpManager::MemoryDumpManager()
     : delegate_(nullptr),
+      is_coordinator_(false),
       memory_tracing_enabled_(0),
       tracing_process_id_(kInvalidTracingProcessId),
       skip_core_dumpers_auto_registration_for_testing_(false),
@@ -117,10 +118,15 @@ MemoryDumpManager::~MemoryDumpManager() {
   TraceLog::GetInstance()->RemoveEnabledStateObserver(this);
 }
 
-void MemoryDumpManager::Initialize() {
-  TRACE_EVENT0(kTraceCategory, "init");  // Add to trace-viewer category list.
-
-  TraceLog::GetInstance()->AddEnabledStateObserver(this);
+void MemoryDumpManager::Initialize(MemoryDumpManagerDelegate* delegate,
+                                   bool is_coordinator) {
+  {
+    AutoLock lock(lock_);
+    DCHECK(delegate);
+    DCHECK(!delegate_);
+    delegate_ = delegate;
+    is_coordinator_ = is_coordinator;
+  }
 
   // Enable the core dump providers.
   if (!skip_core_dumpers_auto_registration_for_testing_) {
@@ -141,12 +147,14 @@ void MemoryDumpManager::Initialize() {
     RegisterDumpProvider(WinHeapDumpProvider::GetInstance());
 #endif
   }  // !skip_core_dumpers_auto_registration_for_testing_
-}
 
-void MemoryDumpManager::SetDelegate(MemoryDumpManagerDelegate* delegate) {
-  AutoLock lock(lock_);
-  DCHECK_EQ(static_cast<MemoryDumpManagerDelegate*>(nullptr), delegate_);
-  delegate_ = delegate;
+  // If tracing was enabled before initializing MemoryDumpManager, we missed the
+  // OnTraceLogEnabled() event. Synthetize it so we can late-join the party.
+  bool is_tracing_already_enabled = TraceLog::GetInstance()->IsEnabled();
+  TRACE_EVENT0(kTraceCategory, "init");  // Add to trace-viewer category list.
+  TraceLog::GetInstance()->AddEnabledStateObserver(this);
+  if (is_tracing_already_enabled)
+    OnTraceLogEnabled();
 }
 
 void MemoryDumpManager::RegisterDumpProvider(
@@ -210,22 +218,21 @@ void MemoryDumpManager::RequestGlobalDump(MemoryDumpType dump_type,
   const uint64 guid =
       TraceLog::GetInstance()->MangleEventId(g_next_guid.GetNext());
 
-  // The delegate_ is supposed to be thread safe, immutable and long lived.
-  // No need to keep the lock after we ensured that a delegate has been set.
+  // Technically there is no need to grab the |lock_| here as the delegate is
+  // long-lived and can only be set by Initialize(), which is locked and
+  // necessarily happens before memory_tracing_enabled_ == true.
+  // Not taking the |lock_|, though, is lakely make TSan barf and, at this point
+  // (memory-infra is enabled) we're not in the fast-path anymore.
   MemoryDumpManagerDelegate* delegate;
   {
     AutoLock lock(lock_);
     delegate = delegate_;
   }
 
-  if (delegate) {
-    // The delegate is in charge to coordinate the request among all the
-    // processes and call the CreateLocalDumpPoint on the local process.
-    MemoryDumpRequestArgs args = {guid, dump_type, dump_args};
-    delegate->RequestGlobalMemoryDump(args, callback);
-  } else if (!callback.is_null()) {
-    callback.Run(guid, false /* success */);
-  }
+  // The delegate will coordinate the IPC broadcast and at some point invoke
+  // CreateProcessDump() to get a dump for the current process.
+  MemoryDumpRequestArgs args = {guid, dump_type, dump_args};
+  delegate->RequestGlobalMemoryDump(args, callback);
 }
 
 void MemoryDumpManager::RequestGlobalDump(MemoryDumpType dump_type,
@@ -394,11 +401,10 @@ void MemoryDumpManager::AbortDumpLocked(
 }
 
 void MemoryDumpManager::OnTraceLogEnabled() {
-  // TODO(primiano): at this point we query  TraceLog::GetCurrentCategoryFilter
-  // to figure out (and cache) which dumpers should be enabled or not.
-  // For the moment piggy back everything on the generic "memory" category.
   bool enabled;
   TRACE_EVENT_CATEGORY_GROUP_ENABLED(kTraceCategory, &enabled);
+  if (!enabled)
+    return;
 
   // Initialize the TraceLog for the current thread. This is to avoid that the
   // TraceLog memory dump provider is registered lazily in the PostTask() below
@@ -407,13 +413,7 @@ void MemoryDumpManager::OnTraceLogEnabled() {
 
   AutoLock lock(lock_);
 
-  // There is no point starting the tracing without a delegate.
-  if (!enabled || !delegate_) {
-    // Disable all the providers.
-    for (auto it = dump_providers_.begin(); it != dump_providers_.end(); ++it)
-      it->disabled = true;
-    return;
-  }
+  DCHECK(delegate_);  // At this point we must have a delegate.
 
   session_state_ = new MemoryDumpSessionState();
   for (auto it = dump_providers_.begin(); it != dump_providers_.end(); ++it) {
@@ -425,9 +425,9 @@ void MemoryDumpManager::OnTraceLogEnabled() {
 
   // TODO(primiano): This is a temporary hack to disable periodic memory dumps
   // when running memory benchmarks until telemetry uses TraceConfig to
-  // enable/disable periodic dumps.
+  // enable/disable periodic dumps. See crbug.com/529184 .
   // The same mechanism should be used to disable periodic dumps in tests.
-  if (!delegate_->IsCoordinatorProcess() ||
+  if (!is_coordinator_ ||
       CommandLine::ForCurrentProcess()->HasSwitch(
           "enable-memory-benchmarking") ||
       disable_periodic_dumps_for_testing_) {
diff --git a/base/trace_event/memory_dump_manager.h b/base/trace_event/memory_dump_manager.h
index 760ad7b..1092a26 100644
--- a/base/trace_event/memory_dump_manager.h
+++ b/base/trace_event/memory_dump_manager.h
@@ -40,12 +40,19 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
 
   static MemoryDumpManager* GetInstance();
 
-  // Invoked once per process to register the TraceLog observer.
-  void Initialize();
-
-  // See the lifetime and thread-safety requirements on the delegate below in
-  // the |MemoryDumpManagerDelegate| docstring.
-  void SetDelegate(MemoryDumpManagerDelegate* delegate);
+  // Invoked once per process to listen to trace begin / end events.
+  // Initialization can happen after (Un)RegisterMemoryDumpProvider() calls
+  // and the MemoryDumpManager guarantees to support this.
+  // On the other side, the MemoryDumpManager will not be fully operational
+  // (i.e. will NACK any RequestGlobalMemoryDump()) until initialized.
+  // Arguments:
+  //  is_coordinator: if true this MemoryDumpManager instance will act as a
+  //      coordinator and schedule periodic dumps (if enabled via TraceConfig);
+  //      false when the MemoryDumpManager is initialized in a slave process.
+  //  delegate: inversion-of-control interface for embedder-specific behaviors
+  //      (multiprocess handshaking). See the lifetime and thread-safety
+  //      requirements in the |MemoryDumpManagerDelegate| docstring.
+  void Initialize(MemoryDumpManagerDelegate* delegate, bool is_coordinator);
 
   // MemoryDumpManager does NOT take memory ownership of |mdp|, which is
   // expected to either be a singleton or unregister itself.
@@ -108,9 +115,6 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   friend struct DefaultSingletonTraits<MemoryDumpManager>;
   friend class MemoryDumpManagerDelegate;
   friend class MemoryDumpManagerTest;
-  FRIEND_TEST_ALL_PREFIXES(MemoryDumpManagerTest, DisableFailingDumpers);
-  FRIEND_TEST_ALL_PREFIXES(MemoryDumpManagerTest,
-                           UnregisterDumperFromThreadWhileDumping);
 
   // Descriptor struct used to hold information about registered MDPs. It is
   // deliberately copyable, in order to allow it to be used as std::set value.
@@ -208,6 +212,9 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
 
   MemoryDumpManagerDelegate* delegate_;  // Not owned.
 
+  // When true, this instance is in charge of coordinating periodic dumps.
+  bool is_coordinator_;
+
   // Protects from concurrent accesses to the |dump_providers_*| and |delegate_|
   // to guard against disabling logging while dumping on another thread.
   Lock lock_;
@@ -241,10 +248,6 @@ class BASE_EXPORT MemoryDumpManagerDelegate {
   virtual void RequestGlobalMemoryDump(const MemoryDumpRequestArgs& args,
                                        const MemoryDumpCallback& callback) = 0;
 
-  // Determines whether the MemoryDumpManager instance should be the master
-  // (the ones which initiates and coordinates the multiprocess dumps) or not.
-  virtual bool IsCoordinatorProcess() const = 0;
-
   // Returns tracing process id of the current process. This is used by
   // MemoryDumpManager::GetTracingProcessId.
   virtual uint64 GetTracingProcessId() const = 0;
diff --git a/base/trace_event/memory_dump_manager_unittest.cc b/base/trace_event/memory_dump_manager_unittest.cc
index 62a5c62..4510cff 100644
--- a/base/trace_event/memory_dump_manager_unittest.cc
+++ b/base/trace_event/memory_dump_manager_unittest.cc
@@ -29,6 +29,10 @@ namespace trace_event {
 namespace {
 MemoryDumpArgs g_high_detail_args = {MemoryDumpArgs::LevelOfDetail::HIGH};
 MemoryDumpArgs g_low_detail_args = {MemoryDumpArgs::LevelOfDetail::LOW};
+const char kTraceConfigWithTriggersFmt[] =
+    "{\"included_categories\":[\"%s\"],\"memory_dump_config\":{\"triggers\":["
+    "{\"mode\":\"light\", \"periodic_interval_ms\":1},"
+    "{\"mode\":\"detailed\", \"periodic_interval_ms\":5}]}}";
 }  // namespace
 
 // GTest matchers for MemoryDumpRequestArgs arguments.
@@ -54,7 +58,6 @@ MATCHER(IsLowDetailArgs, "") {
 class MemoryDumpManagerDelegateForTesting : public MemoryDumpManagerDelegate {
  public:
   MemoryDumpManagerDelegateForTesting() {
-    EXPECT_CALL(*this, IsCoordinatorProcess()).WillRepeatedly(Return(false));
     ON_CALL(*this, RequestGlobalMemoryDump(_, _))
         .WillByDefault(Invoke(
             this, &MemoryDumpManagerDelegateForTesting::CreateProcessDump));
@@ -64,8 +67,6 @@ class MemoryDumpManagerDelegateForTesting : public MemoryDumpManagerDelegate {
                void(const MemoryDumpRequestArgs& args,
                     const MemoryDumpCallback& callback));
 
-  MOCK_CONST_METHOD0(IsCoordinatorProcess, bool());
-
   uint64 GetTracingProcessId() const override {
     NOTREACHED();
     return MemoryDumpManager::kInvalidTracingProcessId;
@@ -86,9 +87,7 @@ class MemoryDumpManagerTest : public testing::Test {
     mdm_.reset(new MemoryDumpManager());
     MemoryDumpManager::SetInstanceForTesting(mdm_.get());
     ASSERT_EQ(mdm_, MemoryDumpManager::GetInstance());
-    mdm_->Initialize();
     delegate_.reset(new MemoryDumpManagerDelegateForTesting);
-    mdm_->SetDelegate(delegate_.get());
   }
 
   void TearDown() override {
@@ -108,7 +107,10 @@ class MemoryDumpManagerTest : public testing::Test {
   }
 
  protected:
-  // This enables tracing using the legacy category filter string.
+  void InitializeMemoryDumpManager(bool is_coordinator) {
+    mdm_->Initialize(delegate_.get(), is_coordinator);
+  }
+
   void EnableTracingWithLegacyCategories(const char* category) {
     TraceLog::GetInstance()->SetEnabled(TraceConfig(category, ""),
                                         TraceLog::RECORDING_MODE);
@@ -125,6 +127,10 @@ class MemoryDumpManagerTest : public testing::Test {
     return mdm_->periodic_dump_timer_.IsRunning();
   }
 
+  int GetMaxConsecutiveFailuresCount() const {
+    return MemoryDumpManager::kMaxConsecutiveFailuresCount;
+  }
+
   scoped_ptr<MemoryDumpManager> mdm_;
   scoped_ptr<MemoryDumpManagerDelegateForTesting> delegate_;
   bool last_callback_success_;
@@ -139,6 +145,7 @@ class MemoryDumpManagerTest : public testing::Test {
 // Basic sanity checks. Registers a memory dump provider and checks that it is
 // called, but only when memory-infra is enabled.
 TEST_F(MemoryDumpManagerTest, SingleDumper) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MockMemoryDumpProvider mdp;
   mdm_->RegisterDumpProvider(&mdp);
 
@@ -175,6 +182,7 @@ TEST_F(MemoryDumpManagerTest, SingleDumper) {
 // Checks that requesting dumps with high level of detail actually propagates
 // the level of the detail properly to OnMemoryDump() call on dump providers.
 TEST_F(MemoryDumpManagerTest, CheckMemoryDumpArgs) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MockMemoryDumpProvider mdp;
 
   mdm_->RegisterDumpProvider(&mdp);
@@ -200,6 +208,7 @@ TEST_F(MemoryDumpManagerTest, CheckMemoryDumpArgs) {
 
 // Checks that the SharedSessionState object is acqually shared over time.
 TEST_F(MemoryDumpManagerTest, SharedSessionState) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
   MockMemoryDumpProvider mdp2;
   mdm_->RegisterDumpProvider(&mdp1);
@@ -232,6 +241,7 @@ TEST_F(MemoryDumpManagerTest, SharedSessionState) {
 
 // Checks that the (Un)RegisterDumpProvider logic behaves sanely.
 TEST_F(MemoryDumpManagerTest, MultipleDumpers) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
   MockMemoryDumpProvider mdp2;
 
@@ -270,6 +280,7 @@ TEST_F(MemoryDumpManagerTest, MultipleDumpers) {
 // Checks that the dump provider invocations depend only on the current
 // registration state and not on previous registrations and dumps.
 TEST_F(MemoryDumpManagerTest, RegistrationConsistency) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MockMemoryDumpProvider mdp;
 
   mdm_->RegisterDumpProvider(&mdp);
@@ -325,6 +336,7 @@ TEST_F(MemoryDumpManagerTest, RegistrationConsistency) {
 // threads and registering a MemoryDumpProvider on each of them. At each
 // iteration, one thread is removed, to check the live unregistration logic.
 TEST_F(MemoryDumpManagerTest, RespectTaskRunnerAffinity) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   const uint32 kNumInitialThreads = 8;
 
   ScopedVector<Thread> threads;
@@ -391,6 +403,7 @@ TEST_F(MemoryDumpManagerTest, RespectTaskRunnerAffinity) {
 // Checks that providers get disabled after 3 consecutive failures, but not
 // otherwise (e.g., if interleaved).
 TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
   MockMemoryDumpProvider mdp2;
 
@@ -398,11 +411,11 @@ TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) {
   mdm_->RegisterDumpProvider(&mdp2);
   EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
 
-  const int kNumDumps = 2 * MemoryDumpManager::kMaxConsecutiveFailuresCount;
+  const int kNumDumps = 2 * GetMaxConsecutiveFailuresCount();
   EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(kNumDumps);
 
   EXPECT_CALL(mdp1, OnMemoryDump(_, _))
-      .Times(MemoryDumpManager::kMaxConsecutiveFailuresCount)
+      .Times(GetMaxConsecutiveFailuresCount())
       .WillRepeatedly(Return(false));
 
   EXPECT_CALL(mdp2, OnMemoryDump(_, _))
@@ -424,6 +437,7 @@ TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) {
 // Sneakily registers an extra memory dump provider while an existing one is
 // dumping and expect it to take part in the already active tracing session.
 TEST_F(MemoryDumpManagerTest, RegisterDumperWhileDumping) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
   MockMemoryDumpProvider mdp2;
 
@@ -458,6 +472,7 @@ TEST_F(MemoryDumpManagerTest, RegisterDumperWhileDumping) {
 
 // Like RegisterDumperWhileDumping, but unregister the dump provider instead.
 TEST_F(MemoryDumpManagerTest, UnregisterDumperWhileDumping) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
   MockMemoryDumpProvider mdp2;
 
@@ -494,6 +509,7 @@ TEST_F(MemoryDumpManagerTest, UnregisterDumperWhileDumping) {
 // Checks that the dump does not abort when unregistering a provider while
 // dumping from a different thread than the dumping thread.
 TEST_F(MemoryDumpManagerTest, UnregisterDumperFromThreadWhileDumping) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   ScopedVector<TestIOThread> threads;
   ScopedVector<MockMemoryDumpProvider> mdps;
 
@@ -534,9 +550,10 @@ TEST_F(MemoryDumpManagerTest, UnregisterDumperFromThreadWhileDumping) {
            MessageLoop::current()->task_runner(), run_loop.QuitClosure());
 
   EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
-  MemoryDumpRequestArgs request_args = {0, MemoryDumpType::EXPLICITLY_TRIGGERED,
-                                        g_high_detail_args};
-  mdm_->CreateProcessDump(request_args, callback);
+
+  EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(1);
+  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                          g_high_detail_args, callback);
 
   run_loop.Run();
 
@@ -549,6 +566,7 @@ TEST_F(MemoryDumpManagerTest, UnregisterDumperFromThreadWhileDumping) {
 // Checks that a NACK callback is invoked if RequestGlobalDump() is called when
 // tracing is not enabled.
 TEST_F(MemoryDumpManagerTest, CallbackCalledOnFailure) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MockMemoryDumpProvider mdp1;
   mdm_->RegisterDumpProvider(&mdp1);
 
@@ -568,10 +586,12 @@ TEST_F(MemoryDumpManagerTest, CallbackCalledOnFailure) {
   EXPECT_FALSE(last_callback_success_);
 }
 
-// This test crystallizes the expectations of the chrome://tracing UI and
-// chrome telemetry w.r.t. periodic dumps in memory-infra, handling gracefully
-// the transition between the legacy and the new-style (JSON-based) TraceConfig.
+// This test (and the MemoryDumpManagerTestCoordinator below) crystallizes the
+// expectations of the chrome://tracing UI and chrome telemetry w.r.t. periodic
+// dumps in memory-infra, handling gracefully the transition between the legacy
+// and the new-style (JSON-based) TraceConfig.
 TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
   MemoryDumpManagerDelegateForTesting& delegate = *delegate_;
 
   // Don't trigger the default behavior of the mock delegate in this test,
@@ -583,14 +603,27 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) {
 
   // Enabling memory-infra in a non-coordinator process should not trigger any
   // periodic dumps.
-  EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(false));
   EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
   EXPECT_FALSE(IsPeriodicDumpingEnabled());
   DisableTracing();
 
+  // Enabling memory-infra with the new (JSON) TraceConfig in a non-coordinator
+  // process with a fully defined trigger config should NOT enable any periodic
+  // dumps.
+  const std::string kTraceConfigWithTriggers = StringPrintf(
+      kTraceConfigWithTriggersFmt, MemoryDumpManager::kTraceCategory);
+  EnableTracingWithTraceConfig(kTraceConfigWithTriggers.c_str());
+  EXPECT_FALSE(IsPeriodicDumpingEnabled());
+  DisableTracing();
+}
+
+TEST_F(MemoryDumpManagerTest, TraceConfigExpectationsWhenIsCoordinator) {
+  InitializeMemoryDumpManager(true /* is_coordinator */);
+  MemoryDumpManagerDelegateForTesting& delegate = *delegate_;
+  ON_CALL(delegate, RequestGlobalMemoryDump(_, _)).WillByDefault(Return());
+
   // Enabling memory-infra with the legacy TraceConfig (category filter) in
   // a coordinator process should enable periodic dumps.
-  EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(true));
   EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
   EXPECT_TRUE(IsPeriodicDumpingEnabled());
   DisableTracing();
@@ -599,7 +632,6 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) {
   // process without specifying any "memory_dump_config" section should enable
   // periodic dumps. This is to preserve the behavior chrome://tracing UI, that
   // is: ticking memory-infra should dump periodically with the default config.
-  EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(true));
   const std::string kTraceConfigWithNoMemorDumpConfig = StringPrintf(
       "{\"included_categories\":[\"%s\"]}", MemoryDumpManager::kTraceCategory);
   EnableTracingWithTraceConfig(kTraceConfigWithNoMemorDumpConfig.c_str());
@@ -610,7 +642,6 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) {
   // process with an empty "memory_dump_config" should NOT enable periodic
   // dumps. This is the way telemetry is supposed to use memory-infra with
   // only explicitly triggered dumps.
-  EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(true));
   const std::string kTraceConfigWithNoTriggers = StringPrintf(
       "{\"included_categories\":[\"%s\"], \"memory_dump_config\":{}",
       MemoryDumpManager::kTraceCategory);
@@ -618,28 +649,9 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) {
   EXPECT_FALSE(IsPeriodicDumpingEnabled());
   DisableTracing();
 
-  // Enabling memory-infra with the new (JSON) TraceConfig in a non-coordinator
-  // process with a fully defined trigger config should NOT enable any periodic
-  // dumps.
-  EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(false));
-  const std::string kTraceConfigWithTriggers = StringPrintf(
-    "{\"included_categories\":[\"%s\"],"
-     "\"memory_dump_config\":{"
-        "\"triggers\":["
-          "{\"mode\":\"light\", \"periodic_interval_ms\":1},"
-          "{\"mode\":\"detailed\", \"periodic_interval_ms\":5}"
-        "]"
-      "}"
-    "}", MemoryDumpManager::kTraceCategory);
-  EnableTracingWithTraceConfig(kTraceConfigWithTriggers.c_str());
-  EXPECT_FALSE(IsPeriodicDumpingEnabled());
-  DisableTracing();
-
   // Enabling memory-infra with the new (JSON) TraceConfig in a coordinator
   // process with a fully defined trigger config should cause periodic dumps to
   // be performed in the correct order.
-  EXPECT_CALL(delegate, IsCoordinatorProcess()).WillRepeatedly(Return(true));
-
   RunLoop run_loop;
   auto quit_closure = run_loop.QuitClosure();
 
@@ -658,6 +670,8 @@ TEST_F(MemoryDumpManagerTest, TraceConfigExpectations) {
   // Swallow all the final spurious calls until tracing gets disabled.
   EXPECT_CALL(delegate, RequestGlobalMemoryDump(_, _)).Times(AnyNumber());
 
+  const std::string kTraceConfigWithTriggers = StringPrintf(
+      kTraceConfigWithTriggersFmt, MemoryDumpManager::kTraceCategory);
   EnableTracingWithTraceConfig(kTraceConfigWithTriggers.c_str());
   run_loop.Run();
   DisableTracing();
diff --git a/components/tracing/child_memory_dump_manager_delegate_impl.cc b/components/tracing/child_memory_dump_manager_delegate_impl.cc
index c32222d..2b537c9 100644
--- a/components/tracing/child_memory_dump_manager_delegate_impl.cc
+++ b/components/tracing/child_memory_dump_manager_delegate_impl.cc
@@ -21,11 +21,11 @@ ChildMemoryDumpManagerDelegateImpl::ChildMemoryDumpManagerDelegateImpl()
     : ctmf_(nullptr),
       tracing_process_id_(
           base::trace_event::MemoryDumpManager::kInvalidTracingProcessId) {
-  base::trace_event::MemoryDumpManager::GetInstance()->SetDelegate(this);
+  base::trace_event::MemoryDumpManager::GetInstance()->Initialize(
+      this /* delegate */, false /* is_coordinator */);
 }
 
-ChildMemoryDumpManagerDelegateImpl::~ChildMemoryDumpManagerDelegateImpl() {
-}
+ChildMemoryDumpManagerDelegateImpl::~ChildMemoryDumpManagerDelegateImpl() {}
 
 void ChildMemoryDumpManagerDelegateImpl::SetChildTraceMessageFilter(
     ChildTraceMessageFilter* ctmf) {
@@ -70,10 +70,6 @@ void ChildMemoryDumpManagerDelegateImpl::RequestGlobalMemoryDump(
   ctmf_->SendGlobalMemoryDumpRequest(args, callback);
 }
 
-bool ChildMemoryDumpManagerDelegateImpl::IsCoordinatorProcess() const {
-  return false;
-}
-
 uint64 ChildMemoryDumpManagerDelegateImpl::GetTracingProcessId() const {
   return tracing_process_id_;
 }
diff --git a/components/tracing/child_memory_dump_manager_delegate_impl.h b/components/tracing/child_memory_dump_manager_delegate_impl.h
index c83fce2..099948e 100644
--- a/components/tracing/child_memory_dump_manager_delegate_impl.h
+++ b/components/tracing/child_memory_dump_manager_delegate_impl.h
@@ -33,7 +33,6 @@ class ChildMemoryDumpManagerDelegateImpl
   void RequestGlobalMemoryDump(
       const base::trace_event::MemoryDumpRequestArgs& args,
       const base::trace_event::MemoryDumpCallback& callback) override;
-  bool IsCoordinatorProcess() const override;
   uint64 GetTracingProcessId() const override;
 
   void SetChildTraceMessageFilter(ChildTraceMessageFilter* ctmf);
diff --git a/content/browser/browser_main_loop.cc b/content/browser/browser_main_loop.cc
index b8c41e0..eb294ef 100644
--- a/content/browser/browser_main_loop.cc
+++ b/content/browser/browser_main_loop.cc
@@ -642,9 +642,7 @@ void BrowserMainLoop::PostMainMessageLoopStart() {
     DOMStorageArea::EnableAggressiveCommitDelay();
   }
 
-  base::trace_event::MemoryDumpManager::GetInstance()->Initialize();
-
-  // Enable the dump providers.
+  // Enable memory-infra dump providers.
   base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
       HostSharedBitmapManager::current());
   base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
diff --git a/content/browser/tracing/memory_tracing_browsertest.cc b/content/browser/tracing/memory_tracing_browsertest.cc
index 90ddb5b..8b0a1a9 100644
--- a/content/browser/tracing/memory_tracing_browsertest.cc
+++ b/content/browser/tracing/memory_tracing_browsertest.cc
@@ -68,13 +68,14 @@ class MemoryTracingTest : public ContentBrowserTest {
     callback_call_count_ = 0;
     last_callback_dump_guid_ = 0;
     last_callback_success_ = false;
-    MemoryDumpManager::GetInstance()->Initialize();
-    mock_dump_provider_.reset(new MockDumpProvider());
-    MemoryDumpManager::GetInstance()->RegisterDumpProvider(
-        mock_dump_provider_.get());
+
     // TODO(primiano): This should be done via TraceConfig.
     // See https://goo.gl/5Hj3o0.
     MemoryDumpManager::GetInstance()->DisablePeriodicDumpsForTesting();
+
+    mock_dump_provider_.reset(new MockDumpProvider());
+    MemoryDumpManager::GetInstance()->RegisterDumpProvider(
+        mock_dump_provider_.get());
     ContentBrowserTest::SetUp();
   }
 
diff --git a/content/browser/tracing/tracing_controller_impl.cc b/content/browser/tracing/tracing_controller_impl.cc
index d69cab9..708841b 100644
--- a/content/browser/tracing/tracing_controller_impl.cc
+++ b/content/browser/tracing/tracing_controller_impl.cc
@@ -59,7 +59,8 @@ TracingControllerImpl::TracingControllerImpl()
       is_recording_(TraceLog::GetInstance()->IsEnabled()),
       is_monitoring_(false),
       is_power_tracing_(false) {
-  base::trace_event::MemoryDumpManager::GetInstance()->SetDelegate(this);
+  base::trace_event::MemoryDumpManager::GetInstance()->Initialize(
+      this /* delegate */, true /* is_coordinator */);
 
   // Deliberately leaked, like this class.
   base::FileTracing::SetProvider(new FileTracingProviderImpl);
@@ -846,10 +847,6 @@ void TracingControllerImpl::RequestGlobalMemoryDump(
     tmf->SendProcessMemoryDumpRequest(args);
 }
 
-bool TracingControllerImpl::IsCoordinatorProcess() const {
-  return true;
-}
-
 uint64 TracingControllerImpl::GetTracingProcessId() const {
   return ChildProcessHost::kBrowserTracingProcessId;
 }
diff --git a/content/browser/tracing/tracing_controller_impl.h b/content/browser/tracing/tracing_controller_impl.h
index ad6126a..9af728e 100644
--- a/content/browser/tracing/tracing_controller_impl.h
+++ b/content/browser/tracing/tracing_controller_impl.h
@@ -59,7 +59,6 @@ class TracingControllerImpl
   void RequestGlobalMemoryDump(
       const base::trace_event::MemoryDumpRequestArgs& args,
       const base::trace_event::MemoryDumpCallback& callback) override;
-  bool IsCoordinatorProcess() const override;
   uint64 GetTracingProcessId() const override;
 
   class TraceMessageFilterObserver {
diff --git a/content/child/child_thread_impl.cc b/content/child/child_thread_impl.cc
index 8f0d93c..08c2075 100644
--- a/content/child/child_thread_impl.cc
+++ b/content/child/child_thread_impl.cc
@@ -28,7 +28,6 @@
 #include "base/synchronization/lock.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/thread_local.h"
-#include "base/trace_event/memory_dump_manager.h"
 #include "base/tracked_objects.h"
 #include "components/tracing/child_trace_message_filter.h"
 #include "content/child/child_discardable_shared_memory_manager.h"
@@ -495,8 +494,6 @@ void ChildThreadImpl::Init(const Options& options) {
       ::HeapProfilerStop, ::GetHeapProfile));
 #endif
 
-  base::trace_event::MemoryDumpManager::GetInstance()->Initialize();
-
   shared_bitmap_manager_.reset(
       new ChildSharedBitmapManager(thread_safe_sender()));