Translate: move UMA related code in TranslateManager to TranslateManagerMetrics

chrome/browser/translate/translate_manager_metrics_unittest.cc

 // Copyright 2013 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/translate/translate_manager_metrics.h"
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_samples.h"
@@ skipping 47 matching lines @@
     EXPECT_EQ(expected_accept_languages, GetCount(
         TranslateManagerMetrics::INITIATION_STATUS_ACCEPT_LANGUAGES));
     EXPECT_EQ(expected_auto_by_config, GetCount(
         TranslateManagerMetrics::INITIATION_STATUS_AUTO_BY_CONFIG));
     EXPECT_EQ(expected_auto_by_link, GetCount(
         TranslateManagerMetrics::INITIATION_STATUS_AUTO_BY_LINK));
     EXPECT_EQ(expected_show_infobar, GetCount(
         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_);
     if (!histogram)
       return;
     samples_ = histogram->SnapshotSamples();
   }
 
   HistogramBase::Count GetCount(HistogramBase::Sample value) {
     if (!samples_.get())
@@ skipping 42 matching lines @@
   TranslateManagerMetrics::ReportInitiationStatus(
       TranslateManagerMetrics::INITIATION_STATUS_AUTO_BY_CONFIG);
   recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 0, 0);
   TranslateManagerMetrics::ReportInitiationStatus(
       TranslateManagerMetrics::INITIATION_STATUS_AUTO_BY_LINK);
   recorder.CheckInitiationStatus(1, 1, 1, 1, 1, 1, 1, 1, 1, 0);
   TranslateManagerMetrics::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());
+}