[tracing] Sanitize process memory dumps for background mode

base/trace_event/process_memory_dump.h

Index: base/trace_event/process_memory_dump.h
diff --git a/base/trace_event/process_memory_dump.h b/base/trace_event/process_memory_dump.h
index 51e4b5f5150a00f9fd563f8bfe6a3c827db2e3a7..cd555f3a2fb8855f191cf367618ee257182bb1c9 100644
--- a/base/trace_event/process_memory_dump.h
+++ b/base/trace_event/process_memory_dump.h
@@ -67,7 +67,8 @@ class BASE_EXPORT ProcessMemoryDump {
   static size_t CountResidentBytes(void* start_address, size_t mapped_size);
 #endif
 
-  ProcessMemoryDump(scoped_refptr<MemoryDumpSessionState> session_state);
+  ProcessMemoryDump(scoped_refptr<MemoryDumpSessionState> session_state,
+                    MemoryDumpLevelOfDetail level_of_detail);

[Primiano Tucci (use gerrit), 2016/05/27 17:23:35]

Oh right now you need the args here, makes sense.
Can you just pass all the MemoryDumpArgs instead of just level_of_detail, just
for sake of consistency?

It's going to be a bit duplicated because now we pass the args both as arugment
to OnMemoryDump(args, pmd) and within PMD but ...whatever. It is still
conceptually clean and I don't see a better solution.
In theory we could refactor all the existing dump providers to take the args
form the pmd() and remove the extra argument to OnMemoryDump but... let's not go
there it's not worth it.

[ssid, 2016/05/27 17:42:48]

Yes I was thinking the same. Removing the args from OnMemoryDump is a big
change.

Having MemoryDumpArgs here means that I would have to include
memory_dump_provider.h in this file. But that is a wrong include.
memory_dump_provider should depend on process_memory_dump. Not the other way
around.
So, I would have to move MemoryDumpArgs from memory_dump_provider into
process_memory_dump.h. Which will also mean that now provider should actually
include process_memory_dump.
Does that sound good?

[ssid, 2016/05/31 22:33:20]

I think this change warrents for removal of the args from OnMemoryDump. Do you
think I sholuld add a TODO here?

   ~ProcessMemoryDump();
 
   // Creates a new MemoryAllocatorDump with the given name and returns the
@@ -183,7 +184,10 @@ class BASE_EXPORT ProcessMemoryDump {
 
   const HeapDumpsMap& heap_dumps() const { return heap_dumps_; }
 
+  MemoryDumpLevelOfDetail level_of_detail() const { return level_of_detail_; }
+
  private:
+  FRIEND_TEST_ALL_PREFIXES(ProcessMemoryDumpTest, BackgroundModeTest);
   MemoryAllocatorDump* AddAllocatorDumpInternal(
       std::unique_ptr<MemoryAllocatorDump> mad);
 
@@ -202,6 +206,17 @@ class BASE_EXPORT ProcessMemoryDump {
   // Keeps track of relationships between MemoryAllocatorDump(s).
   std::vector<MemoryAllocatorDumpEdge> allocator_dumps_edges_;
 
+  // Level of detail of the current dump.
+  const MemoryDumpLevelOfDetail level_of_detail_;
+
+  // This dummy allocator dump is returned when an invalid dump is created in
+  // background mode. The attributes of the dump are not added to the trace.
+  std::unique_ptr<MemoryAllocatorDump> dummy_mad_;

[Primiano Tucci (use gerrit), 2016/05/27 17:23:35]

dummy feels it is for test, it's not the case.
Maybe, dunno, noop_mad_, or black_hole_mad_ (inspired from
https://en.wikipedia.org/wiki/Black_hole_(networking) )

[ssid, 2016/05/31 22:33:20]

Done.

+
+  // When non-null, only the strings matching this name are whitelisted for
+  // background mode.
+  const char* whitelisted_name_for_testing_;

[Primiano Tucci (use gerrit), 2016/05/27 17:23:35]

can we move this to the memory_infra_background_whitelist.h file (see other
comment)

[ssid, 2016/05/27 17:42:48]

Okay I will make this change in this CL, while the first one still uses a local
list.

[ssid, 2016/05/31 22:33:20]

Done.

+
   DISALLOW_COPY_AND_ASSIGN(ProcessMemoryDump);
 };