diff options
author | elijahtaylor@chromium.org <elijahtaylor@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-13 14:08:18 +0000 |
---|---|---|
committer | elijahtaylor@chromium.org <elijahtaylor@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-13 14:08:18 +0000 |
commit | 65796dc249d7d5f31f07dd2e3f3af7eeede3e2d8 (patch) | |
tree | efff40093156198701c7468f7e1e1451c3f494cc /base/metrics | |
parent | 1c2f808c894cc923b0cda477f95edfc20523eb90 (diff) | |
download | chromium_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.cc | 22 | ||||
-rw-r--r-- | base/metrics/histogram_unittest.cc | 26 |
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 - |