[Translate] Integrate TranslateEventProto UMA logging into TranslateManager.

components/translate/core/browser/translate_ui_delegate.cc

 // Copyright 2014 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 "components/translate/core/browser/translate_ui_delegate.h"
 
 #include "base/i18n/string_compare.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "components/translate/core/browser/language_state.h"
@@ skipping 131 matching lines @@
 
   UMA_HISTOGRAM_BOOLEAN(kModifyOriginalLang, true);
   original_language_index_ = language_index;
 }
 
 void TranslateUIDelegate::UpdateOriginalLanguage(
     const std::string& language_code) {
   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;
     }
   }
 }
 
 size_t TranslateUIDelegate::GetTargetLanguageIndex() const {
   return target_language_index_;
 }
 
 void TranslateUIDelegate::UpdateTargetLanguageIndex(size_t language_index) {
   if (target_language_index_ == language_index)
     return;
 
   DCHECK_LT(language_index, GetNumberOfLanguages());
   UMA_HISTOGRAM_BOOLEAN(kModifyTargetLang, true);
   target_language_index_ = language_index;
 }
 
 void TranslateUIDelegate::UpdateTargetLanguage(
     const std::string& language_code) {
   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;
     }
   }
 }
 
 std::string TranslateUIDelegate::GetLanguageCodeAt(size_t index) const {
   DCHECK_LT(index, GetNumberOfLanguages());
   return languages_[index].first;
 }
 
@@ skipping 17 matching lines @@
 }
 
 void TranslateUIDelegate::Translate() {
   if (!translate_driver_->IsOffTheRecord()) {
     prefs_->ResetTranslationDeniedCount(GetOriginalLanguageCode());
     prefs_->ResetTranslationIgnoredCount(GetOriginalLanguageCode());
     prefs_->IncrementTranslationAcceptedCount(GetOriginalLanguageCode());
   }
 
   if (translate_manager_) {
+    translate_manager_->RecordTranslateEvent(
+        metrics::TranslateEventProto::USER_ACCEPT);
     translate_manager_->TranslatePage(GetOriginalLanguageCode(),
                                       GetTargetLanguageCode(), false);
     UMA_HISTOGRAM_BOOLEAN(kPerformTranslate, true);
   }
 }
 
 void TranslateUIDelegate::RevertTranslation() {
   if (translate_manager_) {
     translate_manager_->RevertTranslation();
     UMA_HISTOGRAM_BOOLEAN(kRevertTranslation, true);
@@ skipping 10 matching lines @@
     } else {
       prefs_->IncrementTranslationIgnoredCount(language);
     }
   }
 
   // Remember that the user declined the translation so as to prevent showing a
   // translate UI for that page again.  (TranslateManager initiates translations
   // 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);
   }
 }
 
 bool TranslateUIDelegate::IsLanguageBlocked() {
   return prefs_->IsBlockedLanguage(GetOriginalLanguageCode());
 }
 
 void TranslateUIDelegate::SetLanguageBlocked(bool value) {
   if (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());
   }
 
   UMA_HISTOGRAM_BOOLEAN(kNeverTranslateLang, value);
 }
 
 bool TranslateUIDelegate::IsSiteBlacklisted() {
   std::string host = GetPageHost();
   return !host.empty() && prefs_->IsSiteBlacklisted(host);
 }
 
 void TranslateUIDelegate::SetSiteBlacklist(bool value) {
   std::string host = GetPageHost();
   if (host.empty())
     return;
 
   if (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);
   }
 
   UMA_HISTOGRAM_BOOLEAN(kNeverTranslateSite, value);
 }
 
 bool TranslateUIDelegate::ShouldAlwaysTranslate() {
   return prefs_->IsLanguagePairWhitelisted(GetOriginalLanguageCode(),
@@ skipping 20 matching lines @@
   // The only chance the bubble will show up is after the user manually unchecks
   // "Always do this?". In that case, since it is after user explictly unchecks,
   // we should show as it as unchecked so we only check == N instead of >= N.
   return prefs_->GetTranslationAcceptedCount(GetOriginalLanguageCode()) ==
          threshold;
 }
 
 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);
 }
 
 std::string TranslateUIDelegate::GetPageHost() {
   if (!translate_driver_->HasCurrentPage())
     return std::string();
   return translate_driver_->GetLastCommittedURL().HostNoBrackets();
 }
 
 }  // namespace translate