Store alloc history data in memory leak report protobuf

chrome/browser/metrics/leak_detector_controller.cc

 // Copyright 2016 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.
 
 #include "chrome/browser/metrics/leak_detector_controller.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 87 matching lines @@
                  proto.analysis_interval_bytes(),
                  proto.size_suspicion_threshold(),
                  proto.call_stack_suspicion_threshold());
 }
 
 LeakDetectorController::~LeakDetectorController() {
   DCHECK(thread_checker_.CalledOnValidThread());
   LeakDetector::GetInstance()->RemoveObserver(this);
 }
 
-void LeakDetectorController::OnLeakFound(
-    const LeakDetector::LeakReport& report) {
+void LeakDetectorController::OnLeaksFound(
+    const std::vector<MemoryLeakReportProto>& reports) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  // Initialize the new report with the protobuf template, which contains some
-  // pre-filled data (parameter values).
-  stored_reports_.push_back(leak_report_proto_template_);
-  MemoryLeakReportProto* proto = &stored_reports_.back();
+  for (const auto& report : reports) {
+    // Initialize the new report with the protobuf template, which contains some
+    // pre-filled data (parameter values).
+    stored_reports_.push_back(leak_report_proto_template_);
+    MemoryLeakReportProto* proto = &stored_reports_.back();
 
-  // Fill in the remaining fields of the protobuf with data from |report|.
-  proto->set_size_bytes(report.alloc_size_bytes);
-  proto->mutable_call_stack()->Reserve(report.call_stack.size());
-  for (uintptr_t call_stack_entry : report.call_stack)
-    proto->mutable_call_stack()->Add(call_stack_entry);
+    // Merge in the other fields.
+    proto->CheckTypeAndMergeFrom(report);

[Ilya Sherman, 2016/04/14 01:08:45]

Hmm, is the "CheckTypeAnd" prefix necessary?  That is, could you just call
MergeFrom()?

[Simon Que, 2016/04/14 01:24:30]

I thought it was not available because MemoryLeakReportProto uses LITE_RUNTIME
and I didn't think MergeFrom() was part of MessageLite's API:
https://developers.google.com/protocol-buffers/docs/reference/cpp/google.prot...

It turns out that replacing CheckTypeAndMergeFrom() with MergeFrom() still
works, so maybe I'm missing something.

I also wonder if the params in MemoryLeakReportProto should be in their own
separate nested message field. This way we can just store
MemoryLeakReportProto_Params and copy it to the |params| field in each
MemoryLeakReportProto.

[Ilya Sherman, 2016/04/14 02:30:04]

Yeah, the API docs on that site are not the clearest.  I think the message.h
docs include a bunch of docs about related classes as well, which are available
even when using MessageLite.  (I inferred that MergeFrom() would be usable here
because it was documented as being called from CheckTyepAndMergeFrom().)
Sure, that seems like a reasonable design choice.

+  }
 }
 
 void LeakDetectorController::GetLeakReports(
     std::vector<MemoryLeakReportProto>* reports) {
   DCHECK(thread_checker_.CalledOnValidThread());
   *reports = std::move(stored_reports_);
 }
 
 }  // namespace metrics