[tracing] Add method to create "weak" global dumps

base/trace_event/memory_allocator_dump.h

Index: base/trace_event/memory_allocator_dump.h
diff --git a/base/trace_event/memory_allocator_dump.h b/base/trace_event/memory_allocator_dump.h
index 6c514fa6a62380b9ec6180792451a40b3a9dbbf9..12ff847041c052aa4ec16b2513416c50b602789d 100644
--- a/base/trace_event/memory_allocator_dump.h
+++ b/base/trace_event/memory_allocator_dump.h
@@ -26,6 +26,14 @@ class TracedValue;
 // Data model for user-land memory allocator dumps.
 class BASE_EXPORT MemoryAllocatorDump {
  public:
+  enum Flags {

[petrcermak, 2016/01/13 17:46:14]

I think that this should be called "Flag" (singular, see
https://code.google.com/p/chromium/codesearch#chromium/src/base/logging.h&l=151)

[ssid, 2016/01/15 17:14:56]

Done.

+    // A weak dump is made invalid unless it is marked strong later, either by

[petrcermak, 2016/01/13 17:46:14]

It could also be 'marked' strong "earlier".

[petrcermak, 2016/01/13 17:46:14]

Please don't use the word "invalid" because we don't use it anywhere else and it
is unclear what it means. You could say something along the lines of the dump
being "ignored", "removed", ...

[ssid, 2016/01/15 17:14:56]

Done.

[ssid, 2016/01/15 17:14:56]

Done.

+    // the same process or at least one process in case of global dumps.
+    // All dumps that own an invalid dump and all its children will be marked

[petrcermak, 2016/01/13 17:46:14]

nit: I'd say "All owners and children of a removed dump will also be removed
transitively"

[ssid, 2016/01/15 17:14:56]

Done.

+    // invalid transitively. Default value is 0.

[petrcermak, 2016/01/13 17:46:14]

Rather than "Default value is 0", say "A dump is strong by default".

[ssid, 2016/01/15 17:14:56]

Done.

+    WEAK = 1 << 0
+  };
+
   // MemoryAllocatorDump is owned by ProcessMemoryDump.
   MemoryAllocatorDump(const std::string& absolute_name,
                       ProcessMemoryDump* process_memory_dump,
@@ -60,6 +68,11 @@ class BASE_EXPORT MemoryAllocatorDump {
   // Absolute name, unique within the scope of an entire ProcessMemoryDump.
   const std::string& absolute_name() const { return absolute_name_; }
 
+  // Use Flags to set values.
+  void set_flags(int flags) { flags_ |= flags; }
+  void reset_flags(int flags) { flags_ &= !flags; }

[petrcermak, 2016/01/13 17:46:14]

it this a naming convention? If not, "unset" might be less ambiguous.

[ssid, 2016/01/15 17:14:56]

Done.

+  bool flags() const { return flags_; }

[petrcermak, 2016/01/13 17:46:14]

this shouldn't be a bool...

+
   // Called at trace generation time to populate the TracedValue.
   void AsValueInto(TracedValue* value) const;
 
@@ -83,6 +96,7 @@ class BASE_EXPORT MemoryAllocatorDump {
   ProcessMemoryDump* const process_memory_dump_;  // Not owned (PMD owns this).
   scoped_refptr<TracedValue> attributes_;
   MemoryAllocatorDumpGuid guid_;
+  int flags_;  // See Flags.

[petrcermak, 2016/01/13 17:46:14]

I think that this should be unsigned or Flag (even though you can assign
combinations of flags to it)

[ssid, 2016/01/15 17:14:56]

Hm I made this integer because the traceValue accepts only integer or double.
Not unsigned. Fixed and added a cast there.

 
   // A local buffer for Sprintf conversion on fastpath. Avoids allocating
   // temporary strings on each AddScalar() call.