[memory-infra] Add method to override importance of ownership edges

base/trace_event/process_memory_dump.cc

Index: base/trace_event/process_memory_dump.cc
diff --git a/base/trace_event/process_memory_dump.cc b/base/trace_event/process_memory_dump.cc
index 9f710936e1405f2b4215761ab0d357d75b7dbd8d..427a458210aea917a83bef01e07c69a363066db1 100644
--- a/base/trace_event/process_memory_dump.cc
+++ b/base/trace_event/process_memory_dump.cc
@@ -286,8 +286,7 @@ void ProcessMemoryDump::TakeAllDumpsFrom(ProcessMemoryDump* other) {
   other->allocator_dumps_.clear();
 
   // Move all the edges.
-  allocator_dumps_edges_.insert(allocator_dumps_edges_.end(),
-                                other->allocator_dumps_edges_.begin(),
+  allocator_dumps_edges_.insert(other->allocator_dumps_edges_.begin(),
                                 other->allocator_dumps_edges_.end());
   other->allocator_dumps_edges_.clear();
 
@@ -326,12 +325,12 @@ void ProcessMemoryDump::AsValueInto(TracedValue* value) const {
   }
 
   value->BeginArray("allocators_graph");
-  for (const MemoryAllocatorDumpEdge& edge : allocator_dumps_edges_) {
+  for (const auto& edge : allocator_dumps_edges_) {
     value->BeginDictionary();
-    value->SetString("source", edge.source.ToString());
-    value->SetString("target", edge.target.ToString());
-    value->SetInteger("importance", edge.importance);
-    value->SetString("type", edge.type);
+    value->SetString("source", edge.first.ToString());

[Primiano Tucci (use gerrit), 2017/06/05 15:23:39]

since the edge has still the |source| I think this code would stay more readable
if you:

- name the argument of the loop an explict iterator (for const auto& it : ...)
- add a const MemoryAllocatorDumpEdge& edge = it.second; here
- keep doing edge.(like it was before)

[ssid, 2017/06/05 17:03:29]

Fixed.

+    value->SetString("target", edge.second.target.ToString());
+    value->SetInteger("importance", edge.second.importance);
+    value->SetString("type", edge.second.type);
     value->EndDictionary();
   }
   value->EndArray();
@@ -340,8 +339,11 @@ void ProcessMemoryDump::AsValueInto(TracedValue* value) const {
 void ProcessMemoryDump::AddOwnershipEdge(const MemoryAllocatorDumpGuid& source,
                                          const MemoryAllocatorDumpGuid& target,
                                          int importance) {
-  allocator_dumps_edges_.push_back(
-      {source, target, importance, kEdgeTypeOwnership});
+  DCHECK(allocator_dumps_edges_.find(source) == allocator_dumps_edges_.end() ||

[Primiano Tucci (use gerrit), 2017/06/05 15:23:39]

allocator_dump_edges.count(source) == 0 is more concise (and performance I
think, as it doesn't have to return  an iterator)

[ssid, 2017/06/05 17:03:29]

Done.

+         allocator_dumps_edges_[source].overridable)
+      << "Cannot add multiple ownership edges between same allocator dumps";
+  allocator_dumps_edges_[source] = {target, importance, kEdgeTypeOwnership,
+                                    false /* overridable */};
 }
 
 void ProcessMemoryDump::AddOwnershipEdge(
@@ -350,6 +352,18 @@ void ProcessMemoryDump::AddOwnershipEdge(
   AddOwnershipEdge(source, target, 0 /* importance */);
 }
 
+void ProcessMemoryDump::AddOverridableOwnershipEdge(
+    const MemoryAllocatorDumpGuid& source,
+    const MemoryAllocatorDumpGuid& target,
+    int importance) {
+  if (allocator_dumps_edges_.find(source) == allocator_dumps_edges_.end()) {
+    allocator_dumps_edges_[source] = {target, importance, kEdgeTypeOwnership,
+                                      true /* overridable */};
+  } else {
+    DCHECK(!allocator_dumps_edges_[source].overridable);

[Primiano Tucci (use gerrit), 2017/06/05 15:23:39]

isn't this branch forgetting to actually override the entry? 
I mean, how is this supposed to work to override an edge that has been already
created?

[ssid, 2017/06/05 17:03:29]

Actually I went for the model where we specify the edges that can be overridden
rather than a API that does "OverrideOwnershipEdge". I just didn't want to make
all edges overridable and we will be confused when reading the code. Added
comment here.

+  }
+}
+
 void ProcessMemoryDump::AddSuballocation(const MemoryAllocatorDumpGuid& source,
                                          const std::string& target_node_name) {
   // Do not create new dumps for suballocations in background mode.