Handle zero-size allocations properly in heap profiler

base/trace_event/heap_profiler_heap_dump_writer.cc

Index: base/trace_event/heap_profiler_heap_dump_writer.cc
diff --git a/base/trace_event/heap_profiler_heap_dump_writer.cc b/base/trace_event/heap_profiler_heap_dump_writer.cc
index b94c31246b9d347d42c285d04878c586ed7f9163..dadaacf8d32183d098ea529caf6970ef6f6d73c3 100644
--- a/base/trace_event/heap_profiler_heap_dump_writer.cc
+++ b/base/trace_event/heap_profiler_heap_dump_writer.cc
@@ -70,6 +70,7 @@ std::vector<Bucket> GetSubbuckets(const Bucket& bucket, BreakDownMode breakBy) {
 
   if (breakBy == BreakDownMode::kByBacktrace) {
     for (const auto& context_and_size : bucket.bytes_by_context) {
+      DCHECK(context_and_size.second > 0);

[Primiano Tucci (use gerrit), 2016/04/05 18:38:48]

I think you don't need these ones. These are internal private functions.

       const Backtrace& backtrace = context_and_size.first->backtrace;
       const char* const* begin = std::begin(backtrace.frames);
       const char* const* end = std::end(backtrace.frames);
@@ -96,6 +97,7 @@ std::vector<Bucket> GetSubbuckets(const Bucket& bucket, BreakDownMode breakBy) {
   } else if (breakBy == BreakDownMode::kByTypeName) {
     if (!bucket.is_broken_down_by_type_name) {
       for (const auto& context_and_size : bucket.bytes_by_context) {
+        DCHECK(context_and_size.second > 0);

[Primiano Tucci (use gerrit), 2016/04/05 18:38:48]

ditto

         const AllocationContext* context = context_and_size.first;
         Bucket& subbucket = breakdown[context->type_name];
         subbucket.size += context_and_size.second;
@@ -141,8 +143,7 @@ std::vector<Bucket> BreakDownBy(const Bucket& bucket, BreakDownMode breakBy) {
     // descending size, so later buckets cannot have a larger contribution than
     // this one.
     accounted_for += buckets.front().size;
-    size_t contribution = buckets.front().size * 125 / accounted_for;

[Dmitry Skiba, 2016/04/05 18:54:13]

With new checks, do we still need this change? Version with 'contribution'
variable was clearer, and inlined with the comment above.

-    if (contribution == 0)
+    if (buckets.front().size < (accounted_for / 125))
       break;
 
     // Put the largest bucket in [begin, it) at |it - 1| and max-heapify
@@ -234,6 +235,7 @@ const std::set<Entry>& HeapDumpWriter::Summarize(
   // contexts stored in |bytes_by_context|.
   Bucket root_bucket;
   for (const auto& context_and_size : bytes_by_context) {
+    DCHECK(context_and_size.second > 0);

[Primiano Tucci (use gerrit), 2016/04/05 18:38:48]

Right you just need this one.
Nit: DCHECK_GT(context_and_size.second, 0) is more Chromey

     const AllocationContext* context = &context_and_size.first;
     const size_t size = context_and_size.second;
     root_bucket.bytes_by_context.push_back(std::make_pair(context, size));