[tracing] MemoryDumpManager delegate for upcoming inter-process dumps

This CL introduces an interface to the MemoryDumpManager to delegate
the generation of dump points. The MDM delegate here is an inversion
of dependency interface, to allow the MDM (which lives in //base) to
coordinate inter-process dumps without having to depend on //ipc and
other layers.
In the upcoming CLs this delegate will be concretely implemented by
TracingControllerImpl (browser side) and ChildTraceMessageFilter
(child process side) to handle the IPC dances.

More context and design doc are available in the attached BUG.

BUG=462930

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

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

3 files changed, 64 insertions, 8 deletions

diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc
index ff53b71..ed965fa 100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -60,7 +60,9 @@ void MemoryDumpManager::SetInstanceForTesting(MemoryDumpManager* instance) {
 }
 
 MemoryDumpManager::MemoryDumpManager()
-    : dump_provider_currently_active_(nullptr), memory_tracing_enabled_(0) {
+    : dump_provider_currently_active_(nullptr),
+      delegate_(nullptr),
+      memory_tracing_enabled_(0) {
   g_next_guid.GetNext();  // Make sure that first guid is not zero.
 }
 
@@ -73,6 +75,12 @@ void MemoryDumpManager::Initialize() {
   trace_event::TraceLog::GetInstance()->AddEnabledStateObserver(this);
 }
 
+void MemoryDumpManager::SetDelegate(MemoryDumpManagerDelegate* delegate) {
+  AutoLock lock(lock_);
+  DCHECK(delegate_ == nullptr);
+  delegate_ = delegate;
+}
+
 void MemoryDumpManager::RegisterDumpProvider(MemoryDumpProvider* mdp) {
   AutoLock lock(lock_);
   if (std::find(dump_providers_registered_.begin(),
@@ -103,9 +111,6 @@ void MemoryDumpManager::UnregisterDumpProvider(MemoryDumpProvider* mdp) {
 void MemoryDumpManager::RequestGlobalDump(
     MemoryDumpType dump_type,
     const MemoryDumpCallback& callback) {
-  // TODO(primiano): this will have more logic to coordinate memory dumps across
-  // multiple processes via IPC. See crbug.com/462930.
-
   // Bail out immediately if tracing is not enabled at all.
   if (!UNLIKELY(subtle::NoBarrier_Load(&memory_tracing_enabled_)))
     return;
@@ -113,8 +118,23 @@ void MemoryDumpManager::RequestGlobalDump(
   // TODO(primiano): Make guid actually unique (cross-process) by hashing it
   // with the PID. See crbug.com/462931 for details.
   const uint64 guid = g_next_guid.GetNext();
-  MemoryDumpRequestArgs args = {guid, dump_type};
-  CreateProcessDump(args);
+
+  // The delegate_ is supposed to be thread safe, immutable and long lived.
+  // No need to keep the lock after we ensure that a delegate has been set.
+  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};
+    delegate->RequestGlobalMemoryDump(args, callback);
+  } else if (!callback.is_null()) {
+    callback.Run(guid, false /* success */);
+  }
 }
 
 void MemoryDumpManager::RequestGlobalDump(MemoryDumpType dump_type) {
diff --git a/base/trace_event/memory_dump_manager.h b/base/trace_event/memory_dump_manager.h
index 1699be5..6415cef 100644
--- a/base/trace_event/memory_dump_manager.h
+++ b/base/trace_event/memory_dump_manager.h
@@ -16,6 +16,7 @@
 namespace base {
 namespace trace_event {
 
+class MemoryDumpManagerDelegate;
 class MemoryDumpProvider;
 
 // This is the interface exposed to the rest of the codebase to deal with
@@ -28,6 +29,10 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   // 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);
+
   // MemoryDumpManager does NOT take memory ownership of |mdp|, which is
   // expected to be a singleton.
   void RegisterDumpProvider(MemoryDumpProvider* mdp);
@@ -76,8 +81,10 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   // TODO(primiano): this is required only until crbug.com/466121 gets fixed.
   MemoryDumpProvider* dump_provider_currently_active_;  // Not owned.
 
-  // Protects from concurrent accesses to the |dump_providers_*|, e.g., tearing
-  // down logging while creating a memory dump on another thread.
+  MemoryDumpManagerDelegate* delegate_;  // Not owned.
+
+  // Protects from concurrent accesses to the |dump_providers_*| and |delegate_|
+  // to guard against disabling logging while dumping on another thread.
   Lock lock_;
 
   // Optimization to avoid attempting any memory dump (i.e. to not walk an empty
@@ -87,6 +94,21 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   DISALLOW_COPY_AND_ASSIGN(MemoryDumpManager);
 };
 
+// The delegate is supposed to be long lived (read: a Singleton) and thread
+// safe (i.e. should expect calls from any thread and handle thread hopping).
+class BASE_EXPORT MemoryDumpManagerDelegate {
+ public:
+  virtual void RequestGlobalMemoryDump(const MemoryDumpRequestArgs& args,
+                                       const MemoryDumpCallback& callback) = 0;
+
+ protected:
+  MemoryDumpManagerDelegate() {}
+  virtual ~MemoryDumpManagerDelegate() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MemoryDumpManagerDelegate);
+};
+
 }  // namespace trace_event
 }  // namespace base
 
diff --git a/base/trace_event/memory_dump_manager_unittest.cc b/base/trace_event/memory_dump_manager_unittest.cc
index f63e487..0ae9568 100644
--- a/base/trace_event/memory_dump_manager_unittest.cc
+++ b/base/trace_event/memory_dump_manager_unittest.cc
@@ -16,6 +16,17 @@ using testing::Return;
 namespace base {
 namespace trace_event {
 
+// Testing MemoryDumpManagerDelegate which short-circuits dump requests locally
+// instead of performing IPC dances.
+class MemoryDumpManagerDelegateForTesting : public MemoryDumpManagerDelegate {
+ public:
+  void RequestGlobalMemoryDump(
+      const base::trace_event::MemoryDumpRequestArgs& args,
+      const MemoryDumpCallback& callback) override {
+    MemoryDumpManager::GetInstance()->CreateProcessDump(args);
+  }
+};
+
 class MemoryDumpManagerTest : public testing::Test {
  public:
   void SetUp() override {
@@ -23,6 +34,7 @@ class MemoryDumpManagerTest : public testing::Test {
     MemoryDumpManager::SetInstanceForTesting(mdm_.get());
     ASSERT_EQ(mdm_, MemoryDumpManager::GetInstance());
     MemoryDumpManager::GetInstance()->Initialize();
+    MemoryDumpManager::GetInstance()->SetDelegate(&delegate_);
   }
 
   void TearDown() override {
@@ -44,6 +56,8 @@ class MemoryDumpManagerTest : public testing::Test {
   scoped_ptr<MemoryDumpManager> mdm_;
 
  private:
+  MemoryDumpManagerDelegateForTesting delegate_;
+
   // We want our singleton torn down after each test.
   ShadowingAtExitManager at_exit_manager_;
 };