[tracing] Allow asynchronous unregistration of unbound dump providers

base/trace_event/memory_dump_manager_unittest.cc

Index: base/trace_event/memory_dump_manager_unittest.cc
diff --git a/base/trace_event/memory_dump_manager_unittest.cc b/base/trace_event/memory_dump_manager_unittest.cc
index 898f003807eb92ddb5f9f6a4f1865a335a5b1296..03b3afa32e5201f3239a2c41e3d0fca8414fd1ed 100644
--- a/base/trace_event/memory_dump_manager_unittest.cc
+++ b/base/trace_event/memory_dump_manager_unittest.cc
@@ -17,6 +17,7 @@
 #include "base/test/test_io_thread.h"
 #include "base/test/trace_event_analyzer.h"
 #include "base/thread_task_runner_handle.h"
+#include "base/threading/platform_thread.h"
 #include "base/threading/thread.h"
 #include "base/trace_event/memory_dump_provider.h"
 #include "base/trace_event/process_memory_dump.h"
@@ -93,8 +94,17 @@ class MemoryDumpManagerDelegateForTesting : public MemoryDumpManagerDelegate {
 
 class MockMemoryDumpProvider : public MemoryDumpProvider {
  public:
+  MOCK_METHOD0(Destructor, void());
   MOCK_METHOD2(OnMemoryDump,
                bool(const MemoryDumpArgs& args, ProcessMemoryDump* pmd));
+
+  MockMemoryDumpProvider() : enable_mock_destructor(false) {}
+  ~MockMemoryDumpProvider() override {
+    if (enable_mock_destructor)
+      Destructor();
+  }
+
+  bool enable_mock_destructor;
 };
 
 class MemoryDumpManagerTest : public testing::Test {
@@ -785,7 +795,7 @@ TEST_F(MemoryDumpManagerTest, DisableTracingWhileDumping) {
   // Register also an unbound dump provider. Unbound dump providers are always
   // invoked after bound ones.
   MockMemoryDumpProvider unbound_mdp;
-  RegisterDumpProvider(&unbound_mdp);
+  RegisterDumpProvider(&unbound_mdp, nullptr, kDefaultOptions);
 
   EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
   EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(1);
@@ -877,5 +887,72 @@ TEST_F(MemoryDumpManagerTest, DumpOnBehalfOfOtherProcess) {
   ASSERT_EQ(events[0]->id, events[2]->id);
 }
 
+// Tests the basics of the UnregisterAndDeleteDumpProviderSoon(): the
+// unregistration should actually delete the providers and not leak them.
+TEST_F(MemoryDumpManagerTest, UnregisterAndDeleteDumpProviderSoon) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
+  static const int kNumProviders = 3;
+  int dtor_count = 0;
+  std::vector<scoped_ptr<MemoryDumpProvider>> mdps;
+  for (int i = 0; i < kNumProviders; ++i) {
+    scoped_ptr<MockMemoryDumpProvider> mdp(new MockMemoryDumpProvider);
+    mdp->enable_mock_destructor = true;
+    EXPECT_CALL(*mdp, Destructor())
+        .WillOnce(Invoke([&dtor_count]() { dtor_count++; }));
+    RegisterDumpProvider(mdp.get(), nullptr, kDefaultOptions);
+    mdps.push_back(std::move(mdp));
+  }
+
+  while (!mdps.empty()) {
+    mdm_->UnregisterAndDeleteDumpProviderSoon(std::move(mdps.back()));
+    mdps.pop_back();
+  }
+
+  ASSERT_EQ(kNumProviders, dtor_count);
+}
+
+// This test checks against races when unregistering an unbound dump provider
+// from another thread while dumping. It registers one MDP and, when
+// OnMemoryDump() is called, it invokes UnregisterAndDeleteDumpProviderSoon()
+// from another thread. The OnMemoryDump() and the dtor call are expected to
+// happen on the same thread (the MemoryDumpManager utility thread).
+TEST_F(MemoryDumpManagerTest, UnregisterAndDeleteDumpProviderSoonDuringDump) {
+  InitializeMemoryDumpManager(false /* is_coordinator */);
+  scoped_ptr<MockMemoryDumpProvider> mdp(new MockMemoryDumpProvider);
+  mdp->enable_mock_destructor = true;
+  RegisterDumpProvider(mdp.get(), nullptr, kDefaultOptions);
+
+  base::PlatformThreadRef thread_ref;
+  auto self_unregister_from_another_thread = [&mdp, &thread_ref](
+      const MemoryDumpArgs&, ProcessMemoryDump*) -> bool {
+    thread_ref = PlatformThread::CurrentRef();
+    TestIOThread thread_for_unregistration(TestIOThread::kAutoStart);
+    thread_for_unregistration.PostTaskAndWait(
+        FROM_HERE,
+        base::Bind(
+            &MemoryDumpManager::UnregisterAndDeleteDumpProviderSoon,
+            base::Unretained(MemoryDumpManager::GetInstance()),
+            base::Passed(scoped_ptr<MemoryDumpProvider>(std::move(mdp)))));
+    thread_for_unregistration.Stop();
+    return true;
+  };
+  EXPECT_CALL(*mdp, OnMemoryDump(_, _))
+      .Times(1)
+      .WillOnce(Invoke(self_unregister_from_another_thread));
+  EXPECT_CALL(*mdp, Destructor())
+      .Times(1)
+      .WillOnce(Invoke([&thread_ref]() {
+        EXPECT_EQ(thread_ref, PlatformThread::CurrentRef());
+      }));
+
+  EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
+  EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(2);
+  for (int i = 0; i < 2; ++i) {
+    RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                             MemoryDumpLevelOfDetail::DETAILED);
+  }
+  DisableTracing();
+}
+
 }  // namespace trace_event
 }  // namespace base