Leak reports collect information about the memory usage

chrome/browser/metrics/metrics_memory_details.cc

 // 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.
 
 #include "chrome/browser/metrics/metrics_memory_details.h"
 
 #include <stddef.h>
 
 #include <vector>
 
@@ skipping 38 matching lines @@
     // Skip if a last record is found less than 30 minutes ago.
   } else {
     // Not reporting if it's the first record for |pid|.
     times_[pid] = current_time;
     memory_sizes_[pid] = sample;
   }
   return false;
 }
 
 MetricsMemoryDetails::MetricsMemoryDetails(
-    const base::Closure& callback,
-    MemoryGrowthTracker* memory_growth_tracker)
-    : callback_(callback), memory_growth_tracker_(memory_growth_tracker) {
-  memory_growth_tracker_ = memory_growth_tracker;
-}
+    const base::Closure& callback, MemoryGrowthTracker* memory_growth_tracker)

[Alexei Svitkine (slow), 2016/10/12 21:46:12]

Nit: I think convention is 1 param per line if first param is on a new line. But
if this is what "git cl format" generates, I'm ok with this too.

[mwlodar, 2016/10/12 22:45:39]

Done.

+    : callback_(callback), memory_growth_tracker_(memory_growth_tracker),
+      generate_histograms_(true) {}
+
+MetricsMemoryDetails::MetricsMemoryDetails(
+    const base::Closure& callback, MemoryGrowthTracker* memory_growth_tracker,
+    bool generate_histograms)
+    : callback_(callback), memory_growth_tracker_(memory_growth_tracker),
+      generate_histograms_(generate_histograms) {}

[Alexei Svitkine (slow), 2016/10/12 21:46:12]

Hmm, instead of a separate constructor, why not just make a setter?

[mwlodar, 2016/10/12 22:45:39]

Done.

 
 MetricsMemoryDetails::~MetricsMemoryDetails() {
 }
 
 void MetricsMemoryDetails::OnDetailsAvailable() {
-  UpdateHistograms();
+  if (generate_histograms_)
+    UpdateHistograms();
+  AnalyzeMemoryGrowth();
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback_);
 }
 
 void MetricsMemoryDetails::UpdateHistograms() {
   // Reports a set of memory metrics to UMA.
 
   const ProcessData& browser = *ChromeBrowser();
   size_t aggregate_memory = 0;
   int chrome_count = 0;
   int extension_count = 0;
@@ skipping 33 matching lines @@
           case ProcessMemoryInformation::RENDERER_UNKNOWN:
             NOTREACHED() << "Unknown renderer process type.";
             continue;
           case ProcessMemoryInformation::RENDERER_NORMAL:
           default:
             // TODO(erikkay): Should we bother splitting out the other subtypes?
             UMA_HISTOGRAM_MEMORY_LARGE_MB("Memory.Renderer.Large2",
                                           sample / 1024);
             UMA_HISTOGRAM_MEMORY_LARGE_MB("Memory.Renderer.Committed",
                                           committed / 1024);
-            int diff;
-            if (memory_growth_tracker_ &&
-                memory_growth_tracker_->UpdateSample(
-                    browser.processes[index].pid, sample, &diff)) {
-              if (diff < 0)
-                UMA_HISTOGRAM_MEMORY_KB("Memory.RendererShrinkIn30Min", -diff);
-              else
-                UMA_HISTOGRAM_MEMORY_KB("Memory.RendererGrowthIn30Min", diff);
-            }
             renderer_count++;
             continue;
         }
       }
       case content::PROCESS_TYPE_UTILITY:
         UMA_HISTOGRAM_MEMORY_KB("Memory.Utility", sample);
         other_count++;
         continue;
       case content::PROCESS_TYPE_ZYGOTE:
         UMA_HISTOGRAM_MEMORY_KB("Memory.Zygote", sample);
@@ skipping 162 matching lines @@
   UMA_HISTOGRAM_CUSTOM_COUNTS("Memory.Swap.NumWrites", swap_info().num_writes,
                               1, 100000000, 100);
 
   if (swap_info().orig_data_size > 0 && swap_info().compr_data_size > 0) {
     UMA_HISTOGRAM_CUSTOM_COUNTS(
         "Memory.Swap.CompressionRatio",
         swap_info().orig_data_size / swap_info().compr_data_size, 1, 20, 20);
   }
 }
 #endif  // defined(OS_CHROMEOS)
+
+void MetricsMemoryDetails::AnalyzeMemoryGrowth() {
+  const ProcessData& browser = *ChromeBrowser();
+  for (size_t index = 0; index < browser.processes.size(); index++) {

[Alexei Svitkine (slow), 2016/10/12 21:46:12]

Nit: use a C++ for loop:

for (const auto& process_entry : GetBrowser()->processes) {

}

[mwlodar, 2016/10/12 22:45:39]

Done.

+    int sample = static_cast<int>(browser.processes[index].working_set.priv);
+
+    int diff;
+    if (memory_growth_tracker_ && memory_growth_tracker_->UpdateSample(
+          browser.processes[index].pid, sample, &diff)) {
+      if (generate_histograms_) {

[Alexei Svitkine (slow), 2016/10/12 21:46:12]

Nit: Merge this if with the one you have above.

[mwlodar, 2016/10/12 22:45:39]

Done. This part is non-trivial though and I added a comment to warn against
changing the order of conditions.

+        if (diff < 0)
+          UMA_HISTOGRAM_MEMORY_KB("Memory.RendererShrinkIn30Min", -diff);
+        else
+          UMA_HISTOGRAM_MEMORY_KB("Memory.RendererGrowthIn30Min", diff);
+      }
+    }
+  }
+}