Translate: move UMA related code in TranslateManager to TranslateManagerMetrics

chrome/browser/translate/translate_manager_metrics_unittest.cc

Index: chrome/browser/translate/translate_manager_metrics_unittest.cc
diff --git a/chrome/browser/translate/translate_manager_metrics_unittest.cc b/chrome/browser/translate/translate_manager_metrics_unittest.cc
index aa418d239dbfed5ee120c0daf6b7ea60a0a9c7d1..91b66aecc2e3c3cbacd809b841c7dca1bba663b8 100644
--- a/chrome/browser/translate/translate_manager_metrics_unittest.cc
+++ b/chrome/browser/translate/translate_manager_metrics_unittest.cc
@@ -65,6 +65,16 @@ class MetricsRecorder {
         TranslateManagerMetrics::INITIATION_STATUS_SHOW_INFOBAR));
   }
 
+  HistogramBase::Count GetTotalCount() {

[MAD, 2013/05/15 01:32:48]

Why not make get public, and explicitly get the count we are looking for?

+    Snapshot();
+    if (!samples_.get())
+      return 0;
+    HistogramBase::Count count = samples_->TotalCount();
+    if (!base_samples_.get())
+      return count;
+    return count - base_samples_->TotalCount();
+  }
+
  private:
   void Snapshot() {
     HistogramBase* histogram = StatisticsRecorder::FindHistogram(key_);
@@ -127,3 +137,19 @@ TEST(TranslateManagerMetricsTest, ReportInitiationStatus) {
       TranslateManagerMetrics::INITIATION_STATUS_SHOW_INFOBAR);
   recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 1, 1);
 }
+
+TEST(TranslateManagerMetricsTest, ReportLanguageDetectionError) {
+  MetricsRecorder recorder(TranslateManagerMetrics::GetMetricsName(
+      TranslateManagerMetrics::UMA_LANGUAGE_DETECTION_ERROR));
+  EXPECT_EQ(0, recorder.GetTotalCount());
+  TranslateManagerMetrics::ReportLanguageDetectionError();
+  EXPECT_EQ(1, recorder.GetTotalCount());

[MAD, 2013/05/15 01:32:48]

As suggested before, we would be better making sure we report the right metric.

[Takashi Toyoshima, 2013/05/15 07:35:04]

I'n not sure I understand your comments correctly here.

Here, recorder is initialized with UMA_LANGUAGE_DETECTION_ERROR so that it
checks UMA_LANGUAGE DETECTION_ERROR in following GetTotalCount().

Here, recorder take a snapshot to know the initial count for
UMA_LANGUAGE_DETECTION_ERROR. Then, calling GetTotalCount() takes another
snapshot and subtract initial count if needed.

Do you mean I should check the reported value for the UMA is always "1"?

Here is another question.
In renderer side, I use CheckTotalCount(int) in similar tests. But I feel
GetTotalCount() way is better here because it can provide more useful log in
failures. In CheckTotalCount(int), failures will be reported that it happens
always at the same line where CheckTotalCount(int) is implemented.
CheckInitiationStatus(...) and so on have the same problem. But there pros win
cons. It can reduce code size dramatically.

[MAD, 2013/05/15 12:22:24]

Ho! My goof... I forgot about the constructor receiving the base name and the
other tests validating the sub names only, so you are right, this is my
misunderstanding... Sorry... :-)

As for the other question, I think it is fine as is...

Thanks!

+}
+
+TEST(TranslateManagerMetricsTest, ReportedUnsupportedLanguage) {
+  MetricsRecorder recorder(TranslateManagerMetrics::GetMetricsName(
+      TranslateManagerMetrics::UMA_SERVER_REPORTED_UNSUPPORTED_LANGUAGE));
+  EXPECT_EQ(0, recorder.GetTotalCount());
+  TranslateManagerMetrics::ReportUnsupportedLanguage();
+  EXPECT_EQ(1, recorder.GetTotalCount());
+}