Allow unregistering MemoryDumpProviders during dump

base/trace_event/memory_dump_manager.h

Index: base/trace_event/memory_dump_manager.h
diff --git a/base/trace_event/memory_dump_manager.h b/base/trace_event/memory_dump_manager.h
index 5c75e4e82e882b3e0f49b63f72ffc094496106e8..1530f489f3e3dfc744d776aa23977e024771f98d 100644
--- a/base/trace_event/memory_dump_manager.h
+++ b/base/trace_event/memory_dump_manager.h
@@ -103,6 +103,8 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   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.
@@ -123,6 +125,11 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
     // as can be safely changed without impacting the order within the set.
     mutable int consecutive_failures;
     mutable bool disabled;
+
+    // When unregistering, we do not remove the provider immediately because a

[Primiano Tucci (use gerrit), 2015/08/24 09:31:19]

Please remove the "we".As a general rule it usually doesn't add too much
informative content and is ambiguos (who is we? users? developers? tracing
owners?)
Also I'r reword this comment a bit, the main problem here is not that the dump
depends on the set order. Is that we want to avoid changing the dump_providers_
in the middle of a dump.
I'd probably say here

When a dump provider unregisters, it is flagged as |unregistered| and is removed
only upon the next memory dump. This is to avoid altering the |dump_providers_|
collection while a dump is in progress.

[Ruud van Asseldonk, 2015/08/24 10:08:37]

Done.

+    // dump may be happening which depends on the set order. Instead, we clean
+    // up the set after the dump.
+    mutable bool unregistered;
   };
 
   using MemoryDumpProviderInfoSet = std::set<MemoryDumpProviderInfo>;
@@ -189,10 +196,6 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   // affinity (MDPs belonging to the same thread are adjacent).
   MemoryDumpProviderInfoSet dump_providers_;
 
-  // Flag used to signal that some provider was removed from |dump_providers_|
-  // and therefore the current memory dump (if any) should be aborted.
-  bool did_unregister_dump_provider_;
-
   // Shared among all the PMDs to keep state scoped to the tracing session.
   scoped_refptr<MemoryDumpSessionState> session_state_;