[Spellcheck] Prevent re-recording of non-changed values.
The values recorded in RecordWordCounts() are all strictly monotonic. As such,
re-recording them if they haven't changed distorts our metrics.
Also added framework for unittests for metrics object, plus some basic tests.
[email protected]
[email protected]
BUG=none
Review URL: https://chromiumcodereview.appspot.com/11419178
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@169804 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/spellchecker/spellcheck_host_metrics.cc b/chrome/browser/spellchecker/spellcheck_host_metrics.cc
index 67dd088..2eb6944 100644
--- a/chrome/browser/spellchecker/spellcheck_host_metrics.cc
+++ b/chrome/browser/spellchecker/spellcheck_host_metrics.cc
@@ -9,9 +9,14 @@
SpellCheckHostMetrics::SpellCheckHostMetrics()
: misspelled_word_count_(0),
+ last_misspelled_word_count_(-1),
spellchecked_word_count_(0),
+ last_spellchecked_word_count_(-1),
suggestion_show_count_(0),
+ last_suggestion_show_count_(-1),
replaced_word_count_(0),
+ last_replaced_word_count_(-1),
+ last_unique_word_count_(-1),
start_time_(base::TimeTicks::Now()) {
const uint64 kHistogramTimerDurationInMinutes = 30;
recording_timer_.Start(FROM_HERE,
@@ -105,9 +110,33 @@
}
void SpellCheckHostMetrics::RecordWordCounts() {
- UMA_HISTOGRAM_COUNTS("SpellCheck.CheckedWords", spellchecked_word_count_);
- UMA_HISTOGRAM_COUNTS("SpellCheck.MisspelledWords", misspelled_word_count_);
- UMA_HISTOGRAM_COUNTS("SpellCheck.ReplacedWords", replaced_word_count_);
- UMA_HISTOGRAM_COUNTS("SpellCheck.UniqueWords", checked_word_hashes_.size());
- UMA_HISTOGRAM_COUNTS("SpellCheck.ShownSuggestions", suggestion_show_count_);
+ if (spellchecked_word_count_ != last_spellchecked_word_count_) {
+ DCHECK(spellchecked_word_count_ > last_spellchecked_word_count_);
+ UMA_HISTOGRAM_COUNTS("SpellCheck.CheckedWords", spellchecked_word_count_);
+ last_spellchecked_word_count_ = spellchecked_word_count_;
+ }
+
+ if (misspelled_word_count_ != last_misspelled_word_count_) {
+ DCHECK(misspelled_word_count_ > last_misspelled_word_count_);
+ UMA_HISTOGRAM_COUNTS("SpellCheck.MisspelledWords", misspelled_word_count_);
+ last_misspelled_word_count_ = misspelled_word_count_;
+ }
+
+ if (replaced_word_count_ != last_replaced_word_count_) {
+ DCHECK(replaced_word_count_ > last_replaced_word_count_);
+ UMA_HISTOGRAM_COUNTS("SpellCheck.ReplacedWords", replaced_word_count_);
+ last_replaced_word_count_ = replaced_word_count_;
+ }
+
+ if (((int)checked_word_hashes_.size()) != last_unique_word_count_) {
+ DCHECK((int)checked_word_hashes_.size() > last_unique_word_count_);
+ UMA_HISTOGRAM_COUNTS("SpellCheck.UniqueWords", checked_word_hashes_.size());
+ last_unique_word_count_ = checked_word_hashes_.size();
+ }
+
+ if (suggestion_show_count_ != last_suggestion_show_count_) {
+ DCHECK(suggestion_show_count_ > last_suggestion_show_count_);
+ UMA_HISTOGRAM_COUNTS("SpellCheck.ShownSuggestions", suggestion_show_count_);
+ last_suggestion_show_count_ = suggestion_show_count_;
+ }
}
diff --git a/chrome/browser/spellchecker/spellcheck_host_metrics.h b/chrome/browser/spellchecker/spellcheck_host_metrics.h
index d8c9629..14644931 100644
--- a/chrome/browser/spellchecker/spellcheck_host_metrics.h
+++ b/chrome/browser/spellchecker/spellcheck_host_metrics.h
@@ -56,6 +56,7 @@
void RecordSuggestionStats(int delta);
private:
+ friend class SpellcheckHostMetricsTest;
void OnHistogramTimerExpired();
// Records various counters without changing their values.
@@ -63,12 +64,23 @@
// Number of corrected words of checked words.
int misspelled_word_count_;
+ int last_misspelled_word_count_;
+
// Number of checked words.
int spellchecked_word_count_;
+ int last_spellchecked_word_count_;
+
// Number of suggestion list showings.
int suggestion_show_count_;
+ int last_suggestion_show_count_;
+
// Number of misspelled words replaced by a user.
int replaced_word_count_;
+ int last_replaced_word_count_;
+
+ // Last recorded number of unique words.
+ int last_unique_word_count_;
+
// Time when first spellcheck happened.
base::TimeTicks start_time_;
// Set of checked words in the hashed form.
diff --git a/chrome/browser/spellchecker/spellcheck_host_metrics_unittest.cc b/chrome/browser/spellchecker/spellcheck_host_metrics_unittest.cc
new file mode 100644
index 0000000..a2b9bb3
--- /dev/null
+++ b/chrome/browser/spellchecker/spellcheck_host_metrics_unittest.cc
@@ -0,0 +1,126 @@
+// 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 "chrome/browser/spellchecker/spellcheck_host_metrics.h"
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_samples.h"
+#include "base/metrics/statistics_recorder.h"
+#include "base/utf_string_conversions.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using base::Histogram;
+using base::HistogramSamples;
+using base::StatisticsRecorder;
+
+class SpellcheckHostMetricsTest : public testing::Test {
+ public:
+ SpellcheckHostMetricsTest() : loop_(MessageLoop::TYPE_DEFAULT) {
+ }
+
+ virtual void SetUp() OVERRIDE {
+ base::StatisticsRecorder::Initialize();
+ metrics_.reset(new SpellCheckHostMetrics);
+ }
+
+ SpellCheckHostMetrics* metrics() { return metrics_.get(); }
+ void RecordWordCountsForTesting() { metrics_->RecordWordCounts(); }
+
+ private:
+ MessageLoop loop_;
+ scoped_ptr<SpellCheckHostMetrics> metrics_;
+};
+
+TEST_F(SpellcheckHostMetricsTest, RecordEnabledStats) {
+ scoped_ptr<HistogramSamples> baseline;
+ Histogram* histogram =
+ StatisticsRecorder::FindHistogram("SpellCheck.Enabled");
+ if (histogram)
+ baseline = histogram->SnapshotSamples();
+
+ metrics()->RecordEnabledStats(false);
+
+ histogram =
+ StatisticsRecorder::FindHistogram("SpellCheck.Enabled");
+ ASSERT_TRUE(histogram != NULL);
+ scoped_ptr<HistogramSamples> samples(histogram->SnapshotSamples());
+ if (baseline.get())
+ samples->Subtract(*baseline);
+ EXPECT_EQ(1, samples->GetCount(0));
+ EXPECT_EQ(0, samples->GetCount(1));
+
+ baseline.reset(samples.release());
+
+ metrics()->RecordEnabledStats(true);
+
+ histogram =
+ StatisticsRecorder::FindHistogram("SpellCheck.Enabled");
+ ASSERT_TRUE(histogram != NULL);
+ samples = histogram->SnapshotSamples();
+ samples->Subtract(*baseline);
+ EXPECT_EQ(0, samples->GetCount(0));
+ EXPECT_EQ(1, samples->GetCount(1));
+}
+
+TEST_F(SpellcheckHostMetricsTest, CustomWordStats) {
+ metrics()->RecordCustomWordCountStats(123);
+
+ Histogram* histogram =
+ StatisticsRecorder::FindHistogram("SpellCheck.CustomWords");
+ ASSERT_TRUE(histogram != NULL);
+ scoped_ptr<HistogramSamples> baseline = histogram->SnapshotSamples();
+
+ metrics()->RecordCustomWordCountStats(23);
+ histogram =
+ StatisticsRecorder::FindHistogram("SpellCheck.CustomWords");
+ ASSERT_TRUE(histogram != NULL);
+ scoped_ptr<HistogramSamples> samples = histogram->SnapshotSamples();
+
+ samples->Subtract(*baseline);
+ EXPECT_EQ(23,samples->sum());
+}
+
+TEST_F(SpellcheckHostMetricsTest, RecordWordCountsDiscardsDuplicates) {
+ // This test ensures that RecordWordCounts only records metrics if they
+ // have changed from the last invocation.
+ const char* histogramName[] = {
+ "SpellCheck.CheckedWords",
+ "SpellCheck.MisspelledWords",
+ "SpellCheck.ReplacedWords",
+ "SpellCheck.UniqueWords",
+ "SpellCheck.ShownSuggestions"
+ };
+
+ // Ensure all histograms exist.
+ metrics()->RecordCheckedWordStats(string16(ASCIIToUTF16("test")), false);
+ RecordWordCountsForTesting();
+
+ // Get baselines for all affected histograms.
+ scoped_ptr<HistogramSamples> baselines[arraysize(histogramName)];
+ for (size_t i = 0; i < arraysize(histogramName); ++i) {
+ Histogram* histogram =
+ StatisticsRecorder::FindHistogram(histogramName[i]);
+ if (histogram)
+ baselines[i] = histogram->SnapshotSamples();
+ }
+
+ // Nothing changed, so this invocation should not affect any histograms.
+ RecordWordCountsForTesting();
+
+ // Get samples for all affected histograms.
+ scoped_ptr<HistogramSamples> samples[arraysize(histogramName)];
+ for (size_t i = 0; i < arraysize(histogramName); ++i) {
+ Histogram* histogram =
+ StatisticsRecorder::FindHistogram(histogramName[i]);
+ ASSERT_TRUE(histogram != NULL);
+ samples[i] = histogram->SnapshotSamples();
+ if (baselines[i].get())
+ samples[i]->Subtract(*baselines[i]);
+
+ EXPECT_EQ(0, samples[i]->TotalCount());
+ }
+}
