Merge multiple histogram snapshots into single one for reporting.

base/metrics/statistics_recorder.cc

 // Copyright (c) 2012 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 "base/metrics/statistics_recorder.h"
 
 #include "base/at_exit.h"
 #include "base/debug/leak_annotations.h"
 #include "base/json/string_escape.h"
 #include "base/logging.h"
@@ skipping 374 matching lines @@
   base::AutoLock auto_lock(*lock_);
   histograms_ = new HistogramMap;
   callbacks_ = new CallbackMap;
   ranges_ = new RangesMap;
 
   if (VLOG_IS_ON(1))
     AtExitManager::RegisterCallback(&DumpHistogramsToVlog, this);
 }
 
 // static
+void StatisticsRecorder::ResetForTesting() {

[Alexei Svitkine (slow), 2016/02/17 16:21:12]

Please order this same as in the .h file - i.e. above the ctor. Also, can you
move DumpHistogramsToVlog() to below the dtor?

[bcwhite, 2016/02/17 17:58:20]

This is being moved into another CL.

[Alexei Svitkine (slow), 2016/02/17 18:15:16]

So should it be deleted from this CL? Or are you planning to land the other one
first and rebase this one on top?

[bcwhite, 2016/02/17 19:39:50]

The latter.  I think the other one is ready to land now.

+  if (histograms_) {
+    STLDeleteValues(histograms_);
+    delete histograms_;
+    histograms_= nullptr;
+  }
+  if (ranges_) {
+    for (auto& iter : *ranges_) {
+      STLDeleteElements(iter.second);
+      delete iter.second;
+    }
+    delete ranges_;
+    ranges_ = nullptr;
+  }
+  delete callbacks_;
+  callbacks_ = nullptr;

[Alexei Svitkine (slow), 2016/02/17 16:21:12]

A lot of this code is similar to what's done in the dtor.

Can you make a private helper function? I know one difference is whether the
container values are deleted - but you can make a boolean param in the helper
for this.

I would also keep the structure of the dtor and have separate if (histograms_)
STLDelete* stmts for when the boolean specify to delete the contents.

(The reason to consolidate them is to make it less likely for someone update one
but forget the other.)

[bcwhite, 2016/02/17 17:58:20]

This is being moved into another CL.  It turns out that trying to clear the
histograms from the map can actually cause problems during testing because of
static pointers to histograms created in other code.

+}
+
+// static
 void StatisticsRecorder::DumpHistogramsToVlog(void* instance) {
   std::string output;
   StatisticsRecorder::WriteGraph(std::string(), &output);
   VLOG(1) << output;
 }
 
 StatisticsRecorder::~StatisticsRecorder() {
   DCHECK(histograms_ && ranges_ && lock_);
 
   // Clean up.
@@ skipping 18 matching lines @@
 // static
 StatisticsRecorder::HistogramMap* StatisticsRecorder::histograms_ = NULL;
 // static
 StatisticsRecorder::CallbackMap* StatisticsRecorder::callbacks_ = NULL;
 // static
 StatisticsRecorder::RangesMap* StatisticsRecorder::ranges_ = NULL;
 // static
 base::Lock* StatisticsRecorder::lock_ = NULL;
 
 }  // namespace base