[Tracing] Disable registration of regular dump providers during tests

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 dfaf3a1fc9e0e16a226a5f833c50d648e4629fde..240524d0f4a5580d286bde204874f770d4f19a87 100644
--- a/base/trace_event/memory_dump_manager.h
+++ b/base/trace_event/memory_dump_manager.h
@@ -17,6 +17,10 @@
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 
+namespace content {
+class MemoryTracingTest;

[Primiano Tucci (use gerrit), 2015/08/24 17:23:34]

Hmm no, base should never know anything about content, we have to find another
way to achieve this.

[Ruud van Asseldonk, 2015/08/25 09:51:17]

I made |ignore_dumper_registrations_for_testing()| public. The for_testing
suffix should prevent people from using this outside of tests anyway.

+}
+
 namespace base {
 
 class SingleThreadTaskRunner;
@@ -107,6 +111,7 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   friend struct DefaultSingletonTraits<MemoryDumpManager>;
   friend class MemoryDumpManagerDelegate;
   friend class MemoryDumpManagerTest;
+  friend class content::MemoryTracingTest;
   FRIEND_TEST_ALL_PREFIXES(MemoryDumpManagerTest, DisableFailingDumpers);
   FRIEND_TEST_ALL_PREFIXES(MemoryDumpManagerTest,
                            UnregisterDumperFromThreadWhileDumping);
@@ -208,7 +213,7 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
 
   // Protects from concurrent accesses to the |dump_providers_*| and |delegate_|
   // to guard against disabling logging while dumping on another thread.
-  Lock lock_;
+  mutable Lock lock_;

[Primiano Tucci (use gerrit), 2015/08/24 17:23:34]

See comment below, you don't need to use the lock at all.

 
   // Optimization to avoid attempting any memory dump (i.e. to not walk an empty
   // dump_providers_enabled_ list) when tracing is not enabled.
@@ -224,8 +229,18 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
   // Name of the allocated_objects dump.
   const char* system_allocator_pool_name_;
 
-  // Skips the auto-registration of the core dumpers during Initialize().
-  bool skip_core_dumpers_auto_registration_for_testing_;
+  // When true, calling RegisterMemoryDumpProvider is a no-op.
+  bool ignore_registrations_for_testing_;

[Primiano Tucci (use gerrit), 2015/08/24 17:23:34]

I'd name this (and the setter) ignore_dumper_registrations_for_testing_

[Ruud van Asseldonk, 2015/08/25 09:51:17]

Done.

+
+  bool ignore_registrations_for_testing() const {
+    AutoLock lock(lock_);

[Primiano Tucci (use gerrit), 2015/08/24 17:23:34]

If this was production code would have been fine.
But for testing code we can just safely assume that the setter is called early
enough and does not race.
Also, I think you just want a setter here. Are you ever going to use the getter?

[Ruud van Asseldonk, 2015/08/25 09:51:17]

It is used in |MemoryDumpManagerTest::EnableTracing| to enable tracing without
registering any extra dump provider, and without changing the state of whether
registrations are ignored (because the tests do register providers, and it would
be tedious to wrap every registration into a set ignore/unset ignore pair).

I think you are right about the lock, setting is only done from one thread so
even if a test starts up new threads this is not a problem.

+    return ignore_registrations_for_testing_;
+  }
+
+  void set_ignore_registrations_for_testing(bool ignore) {
+    AutoLock lock(lock_);
+    ignore_registrations_for_testing_ = ignore;
+  }
 
   // When true, the initialization phase does not start the periodic memory
   // dumps.