Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4693)

Unified Diff: base/metrics/histogram.cc

Issue 141393002: Return a NULL histogram pointer on construction error (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: add unit test, move impl out of .h Created 6 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | base/metrics/histogram_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/metrics/histogram.cc
diff --git a/base/metrics/histogram.cc b/base/metrics/histogram.cc
index fbe66d05d29fd3c2896f8499dd1dc07d9793d095..0a56174bfeeac60ecdb4488b3e58984005d5e700 100644
--- a/base/metrics/histogram.cc
+++ b/base/metrics/histogram.cc
@@ -76,6 +76,27 @@ bool ValidateRangeChecksum(const HistogramBase& histogram,
return casted_histogram.bucket_ranges()->checksum() == range_checksum;
}
+// DummyHistogram is an object that overrides Histogram's Add* functions
+// to provide no functionality. This can be returned in the case of mismatched
+// construction arguments to any of the other Histograms' FactoryGet calls.
+class DummyHistogram : public Histogram {
+ public:
+ virtual void Add(Sample value) OVERRIDE {}
+ virtual void AddSamples(const HistogramSamples& samples) OVERRIDE {}
+ virtual bool AddSamplesFromPickle(PickleIterator* iter) OVERRIDE;
+
+ DummyHistogram();
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(DummyHistogram);
+};
+
+bool DummyHistogram::AddSamplesFromPickle(PickleIterator* iter) {
Alexei Svitkine (slow) 2014/01/17 20:06:31 Nit: You can probably define these inline above.
elijahtaylor1 2014/01/17 20:25:34 I got an error on some windows trybots before for
Alexei Svitkine (slow) 2014/01/17 22:14:27 I think it should work - there's code elsewhere th
+ return false;
+}
+
+DummyHistogram::DummyHistogram() : Histogram("Dummy", 0, 1, NULL) {}
+
} // namespace
typedef HistogramBase::Count Count;
@@ -110,7 +131,16 @@ HistogramBase* Histogram::FactoryGet(const string& name,
}
DCHECK_EQ(HISTOGRAM, histogram->GetHistogramType());
- CHECK(histogram->HasConstructionArguments(minimum, maximum, bucket_count));
+ if (!histogram->HasConstructionArguments(minimum, maximum, bucket_count)) {
+ // The construction arguments do not match the existing histogram. This can
+ // come about if an extension updates in the middle of a chrome run and has
+ // changed one of them. We return a dummy object here instead of crashing
+ // so poor use of extension/Pepper APIs do not cause crashes in Chrome.
+ DLOG(ERROR) << "Histogram " << name << " has bad construction arguments";
+ DummyHistogram* tentative_histogram = new DummyHistogram;
+ histogram =
+ StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
+ }
return histogram;
}
@@ -567,7 +597,16 @@ HistogramBase* LinearHistogram::FactoryGetWithRangeDescription(
}
DCHECK_EQ(LINEAR_HISTOGRAM, histogram->GetHistogramType());
- CHECK(histogram->HasConstructionArguments(minimum, maximum, bucket_count));
+ if (!histogram->HasConstructionArguments(minimum, maximum, bucket_count)) {
+ // The construction arguments do not match the existing histogram. This can
+ // come about if an extension updates in the middle of a chrome run and has
+ // changed one of them. We return a dummy object here instead of crashing
+ // so poor use of extension/Pepper APIs do not cause crashes in Chrome.
+ DLOG(ERROR) << "Histogram " << name << " has bad construction arguments";
+ DummyHistogram* tentative_histogram = new DummyHistogram;
+ histogram =
+ StatisticsRecorder::RegisterOrDeleteDuplicate(tentative_histogram);
+ }
return histogram;
}
« no previous file with comments | « no previous file | base/metrics/histogram_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698