[Translate] Integrate TranslateEventProto UMA logging into TranslateManager.

components/translate/core/browser/translate_ui_delegate.cc

Index: components/translate/core/browser/translate_ui_delegate.cc
diff --git a/components/translate/core/browser/translate_ui_delegate.cc b/components/translate/core/browser/translate_ui_delegate.cc
index 7caffe1ef1b088fdf578cc99be49aebca85543ef..d0e7cfe3d4902a4f436ccffd5a147b021b8a6be8 100644
--- a/components/translate/core/browser/translate_ui_delegate.cc
+++ b/components/translate/core/browser/translate_ui_delegate.cc
@@ -149,6 +149,10 @@ void TranslateUIDelegate::UpdateOriginalLanguage(
   for (size_t i = 0; i < languages_.size(); ++i) {
     if (languages_[i].first.compare(language_code) == 0) {
       UpdateOriginalLanguageIndex(i);
+      if (translate_manager_) {

[groby-ooo-7-16, 2016/10/25 18:53:11]

Looks like it's an invariant as well - no need for if, here and elsewhere.

[Roger McFarlane (Chromium), 2016/10/25 20:07:56]

Acknowledged.

+        translate_manager_->mutable_translate_event()
+            ->set_modified_source_language(language_code);

[groby-ooo-7-16, 2016/10/25 18:53:11]

Do you really want to call this every time the language is changed, as opposed
to when the user has decided on a language? My suspicion is that you'll record a
lot of accidental misclicks that way.

[Roger McFarlane (Chromium), 2016/10/25 20:07:56]

This just tracks the state. We only record once the user has decided.

[groby-ooo-7-16, 2016/10/25 23:17:21]

You still could pull that at decision time instead. (It's not a big deal, but
that might get it out of the UI delegate)

+      }
       return;
     }
   }
@@ -172,6 +176,10 @@ void TranslateUIDelegate::UpdateTargetLanguage(
   for (size_t i = 0; i < languages_.size(); ++i) {
     if (languages_[i].first.compare(language_code) == 0) {
       UpdateTargetLanguageIndex(i);
+      if (translate_manager_) {
+        translate_manager_->mutable_translate_event()

[groby-ooo-7-16, 2016/10/25 18:53:11]

see above

[Roger McFarlane (Chromium), 2016/10/25 20:07:56]

Acknowledged.

+            ->set_modified_target_language(language_code);
+      }
       return;
     }
   }
@@ -209,6 +217,8 @@ void TranslateUIDelegate::Translate() {
   }
 
   if (translate_manager_) {
+    translate_manager_->RecordTranslateEvent(
+        metrics::TranslateEventProto::USER_ACCEPT);
     translate_manager_->TranslatePage(GetOriginalLanguageCode(),
                                       GetTargetLanguageCode(), false);
     UMA_HISTOGRAM_BOOLEAN(kPerformTranslate, true);
@@ -239,12 +249,17 @@ void TranslateUIDelegate::TranslationDeclined(bool explicitly_closed) {
   // when getting a LANGUAGE_DETERMINED from the page, which happens when a load
   // stops. That could happen multiple times, including after the user already
   // declined the translation.)
-  if (explicitly_closed && translate_manager_) {
-    translate_manager_->GetLanguageState().set_translation_declined(true);
-    UMA_HISTOGRAM_BOOLEAN(kDeclineTranslate, true);
+  if (translate_manager_) {
+    translate_manager_->RecordTranslateEvent(
+        explicitly_closed ? metrics::TranslateEventProto::USER_DECLINE
+                          : metrics::TranslateEventProto::USER_IGNORE);
+    if (explicitly_closed)
+      translate_manager_->GetLanguageState().set_translation_declined(true);
   }
 
-  if (!explicitly_closed) {
+  if (explicitly_closed) {
+    UMA_HISTOGRAM_BOOLEAN(kDeclineTranslate, true);
+  } else {
     UMA_HISTOGRAM_BOOLEAN(kDeclineTranslateDismissUI, true);
   }
 }
@@ -258,6 +273,11 @@ void TranslateUIDelegate::SetLanguageBlocked(bool value) {
     prefs_->BlockLanguage(GetOriginalLanguageCode());
     if (translate_manager_) {
       translate_manager_->GetLanguageState().SetTranslateEnabled(false);
+      // Translation has been blocked for this language. Capture that in the
+      // metrics. Note that we don't capture a language being unblocked... which
+      // is not the same as accepting a given translation for this language.
+      translate_manager_->RecordTranslateEvent(
+          metrics::TranslateEventProto::USER_NEVER_TRANSLATE_LANGUAGE);
     }
   } else {
     prefs_->UnblockLanguage(GetOriginalLanguageCode());
@@ -280,6 +300,11 @@ void TranslateUIDelegate::SetSiteBlacklist(bool value) {
     prefs_->BlacklistSite(host);
     if (translate_manager_) {
       translate_manager_->GetLanguageState().SetTranslateEnabled(false);
+      // Translation has been blocked for this site. Capture that in the metrics
+      // Note that we don't capture a language being unblocked... which is not

[groby-ooo-7-16, 2016/10/25 18:53:11]

Super-annoying nit: Usually, Chrome comments avoid "we" or "I" statements
because it's unclear whom/what they refer to. Can you change that comment to
make it more clear? 

[Roger McFarlane (Chromium), 2016/10/25 20:07:56]

Will do, here, above and below... and will keep in mind for future CLs.  :)

+      // the same as accepting a given translation for this site.
+      translate_manager_->RecordTranslateEvent(
+          metrics::TranslateEventProto::USER_NEVER_TRANSLATE_SITE);
     }
   } else {
     prefs_->RemoveSiteFromBlacklist(host);
@@ -320,10 +345,19 @@ bool TranslateUIDelegate::ShouldAlwaysTranslateBeCheckedByDefault() {
 void TranslateUIDelegate::SetAlwaysTranslate(bool value) {
   const std::string& original_lang = GetOriginalLanguageCode();
   const std::string& target_lang = GetTargetLanguageCode();
-  if (value)
+  if (value) {
     prefs_->WhitelistLanguagePair(original_lang, target_lang);
-  else
+    // A default translation mapping has been accepted for this language.
+    // Capture that in the metrics. Note that we don't capture a language being

[groby-ooo-7-16, 2016/10/25 18:53:11]

see above re: "we" 

[Roger McFarlane (Chromium), 2016/10/25 20:07:56]

Acknowledged.

+    // unmapped... which is not the same as accepting some other translation
+    // for this language.
+    if (translate_manager_) {
+      translate_manager_->RecordTranslateEvent(
+          metrics::TranslateEventProto::USER_ALWAYS_TRANSLATE_LANGUAGE);
+    }
+  } else {
     prefs_->RemoveLanguagePairFromWhitelist(original_lang, target_lang);
+  }
 
   UMA_HISTOGRAM_BOOLEAN(kAlwaysTranslateLang, value);
 }