[tracing] Sanitize process memory dumps for background mode

base/trace_event/process_memory_dump.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_TRACE_EVENT_PROCESS_MEMORY_DUMP_H_
 #define BASE_TRACE_EVENT_PROCESS_MEMORY_DUMP_H_
 
 #include <stddef.h>
 
 #include <unordered_map>
@@ skipping 49 matching lines @@
   // In most cases, the two are the same.
   static size_t GetSystemPageSize();
 
   // Returns the total bytes resident for a virtual address range, with given
   // |start_address| and |mapped_size|. |mapped_size| is specified in bytes. The
   // value returned is valid only if the given range is currently mmapped by the
   // process. The |start_address| must be page-aligned.
   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
   // empty object back to the caller.
   // Arguments:
   //   absolute_name: a name that uniquely identifies allocator dumps produced
   //       by this provider. It is possible to specify nesting by using a
   //       path-like string (e.g., v8/isolate1/heap1, v8/isolate1/heap2).
   //       Leading or trailing slashes are not allowed.
   //   guid: an optional identifier, unique among all processes within the
@@ skipping 95 matching lines @@
   ProcessMemoryTotals* process_totals() { return &process_totals_; }
   bool has_process_totals() const { return has_process_totals_; }
   void set_has_process_totals() { has_process_totals_ = true; }
 
   ProcessMemoryMaps* process_mmaps() { return &process_mmaps_; }
   bool has_process_mmaps() const { return has_process_mmaps_; }
   void set_has_process_mmaps() { has_process_mmaps_ = true; }
 
   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);
 
   ProcessMemoryTotals process_totals_;
   bool has_process_totals_;
 
   ProcessMemoryMaps process_mmaps_;
   bool has_process_mmaps_;
 
   AllocatorDumpsMap allocator_dumps_;
   HeapDumpsMap heap_dumps_;
 
   // State shared among all PMDs instances created in a given trace session.
   scoped_refptr<MemoryDumpSessionState> session_state_;
 
   // 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);
 };
 
 }  // namespace trace_event
 }  // namespace base
 
 #endif  // BASE_TRACE_EVENT_PROCESS_MEMORY_DUMP_H_