summaryrefslogtreecommitdiffstats
path: root/base/metrics
diff options
context:
space:
mode:
authorelijahtaylor@chromium.org <elijahtaylor@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-13 14:08:18 +0000
committerelijahtaylor@chromium.org <elijahtaylor@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-13 14:08:18 +0000
commit65796dc249d7d5f31f07dd2e3f3af7eeede3e2d8 (patch)
treeefff40093156198701c7468f7e1e1451c3f494cc /base/metrics
parent1c2f808c894cc923b0cda477f95edfc20523eb90 (diff)
downloadchromium_src-65796dc249d7d5f31f07dd2e3f3af7eeede3e2d8.zip
chromium_src-65796dc249d7d5f31f07dd2e3f3af7eeede3e2d8.tar.gz
chromium_src-65796dc249d7d5f31f07dd2e3f3af7eeede3e2d8.tar.bz2
Return a NULL histogram pointer on construction error
Check the pointer returned for the metricsPrivate API to keep from crashing Chrome with bad data. BUG=334135 Review URL: https://codereview.chromium.org/141393002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@256822 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base/metrics')
-rw-r--r--base/metrics/histogram.cc22
-rw-r--r--base/metrics/histogram_unittest.cc26
2 files changed, 46 insertions, 2 deletions
diff --git a/base/metrics/histogram.cc b/base/metrics/histogram.cc
index fbe66d0..beb9b9e 100644
--- a/base/metrics/histogram.cc
+++ b/base/metrics/histogram.cc
@@ -110,7 +110,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, or simply by bad code within Chrome itself. We
+ // return NULL here with the expectation that bad code in Chrome will crash
+ // on dereference, but extension/Pepper APIs will guard against NULL and not
+ // crash.
+ DLOG(ERROR) << "Histogram " << name << " has bad construction arguments";
+ return NULL;
+ }
return histogram;
}
@@ -567,7 +576,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, or simply by bad code within Chrome itself. We
+ // return NULL here with the expectation that bad code in Chrome will crash
+ // on dereference, but extension/Pepper APIs will guard against NULL and not
+ // crash.
+ DLOG(ERROR) << "Histogram " << name << " has bad construction arguments";
+ return NULL;
+ }
return histogram;
}
diff --git a/base/metrics/histogram_unittest.cc b/base/metrics/histogram_unittest.cc
index e7a01aa5..5fbd28a 100644
--- a/base/metrics/histogram_unittest.cc
+++ b/base/metrics/histogram_unittest.cc
@@ -443,6 +443,32 @@ TEST_F(HistogramTest, CustomHistogramSerializeInfo) {
EXPECT_FALSE(iter.SkipBytes(1));
}
+TEST_F(HistogramTest, BadConstruction) {
+ HistogramBase* histogram = Histogram::FactoryGet(
+ "BadConstruction", 0, 100, 8, HistogramBase::kNoFlags);
+ EXPECT_TRUE(histogram->HasConstructionArguments(1, 100, 8));
+
+ // Try to get the same histogram name with different arguments.
+ HistogramBase* bad_histogram = Histogram::FactoryGet(
+ "BadConstruction", 0, 100, 7, HistogramBase::kNoFlags);
+ EXPECT_EQ(NULL, bad_histogram);
+ bad_histogram = Histogram::FactoryGet(
+ "BadConstruction", 0, 99, 8, HistogramBase::kNoFlags);
+ EXPECT_EQ(NULL, bad_histogram);
+
+ HistogramBase* linear_histogram = LinearHistogram::FactoryGet(
+ "BadConstructionLinear", 0, 100, 8, HistogramBase::kNoFlags);
+ EXPECT_TRUE(linear_histogram->HasConstructionArguments(1, 100, 8));
+
+ // Try to get the same histogram name with different arguments.
+ bad_histogram = LinearHistogram::FactoryGet(
+ "BadConstructionLinear", 0, 100, 7, HistogramBase::kNoFlags);
+ EXPECT_EQ(NULL, bad_histogram);
+ bad_histogram = LinearHistogram::FactoryGet(
+ "BadConstructionLinear", 10, 100, 8, HistogramBase::kNoFlags);
+ EXPECT_EQ(NULL, bad_histogram);
+}
+
#if GTEST_HAS_DEATH_TEST
// For Histogram, LinearHistogram and CustomHistogram, the minimum for a
// declared range is 1, while the maximum is (HistogramBase::kSampleType_MAX -