[Translate] Integrate TranslateEventProto UMA logging into TranslateManager.

chrome/browser/translate/chrome_translate_client.cc

Index: chrome/browser/translate/chrome_translate_client.cc
diff --git a/chrome/browser/translate/chrome_translate_client.cc b/chrome/browser/translate/chrome_translate_client.cc
index 06ce2bbbcd236307ca099d12faf0e7064c05534a..827ab7410e9b8ff6ac8001988655e1dcd8bf33f3 100644
--- a/chrome/browser/translate/chrome_translate_client.cc
+++ b/chrome/browser/translate/chrome_translate_client.cc
@@ -26,6 +26,7 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/theme_resources.h"
+#include "components/metrics/proto/translate_event.pb.h"
 #include "components/prefs/pref_service.h"
 #include "components/translate/core/browser/language_model.h"
 #include "components/translate/core/browser/language_state.h"
@@ -174,6 +175,8 @@ void ChromeTranslateClient::ShowTranslateUI(
     translate::TranslateErrors::Type error_type,
     bool triggered_from_menu) {
   DCHECK(web_contents());
+  DCHECK(translate_manager_);
+
   if (error_type != translate::TranslateErrors::NONE)
     step = translate::TRANSLATE_STEP_TRANSLATE_ERROR;
 
@@ -198,14 +201,45 @@ void ChromeTranslateClient::ShowTranslateUI(
     // another page.
     if (!base::FeatureList::IsEnabled(translate::kTranslateUI2016Q2) &&
         !GetLanguageState().HasLanguageChanged()) {
+      translate_manager_->RecordTranslateEvent(
+          metrics::TranslateEventProto::MATCHES_PREVIOUS_LANGUAGE);
       return;
     }
 
     if (!triggered_from_menu &&
-        GetTranslatePrefs()->IsTooOftenDenied(source_language))
+        GetTranslatePrefs()->IsTooOftenDenied(source_language)) {
+      translate_manager_->RecordTranslateEvent(
+          metrics::TranslateEventProto::LANGUAGE_DISABLED_BY_AUTO_BLACKLIST);
       return;
+    }
+  }
+  ShowTranslateBubbleResult result = ShowBubble(step, error_type);
+  if (step == translate::TRANSLATE_STEP_BEFORE_TRANSLATE) {
+    switch (result) {
+      case ShowTranslateBubbleResult::SUCCESS:
+        break;
+      case ShowTranslateBubbleResult::BROWSER_WINDOW_NOT_VALID:

[groby-ooo-7-16, 2016/11/15 01:15:43]

I am deeply saddened we have two enums we need to map to each other :(

Given that the alternative is littering every place consuming/generating the
result with the proto definition, I suppose it is what it is. I still haz a sad.

Part of me wants to at least make this code more readable via a macro....

#define RECORD_IF(X) case ShowTranslateBubbleResult::X:
translate_manager_->RecordTranslateEvent(metrics::TranslateEventProto::X); break

Or maybe RecordTranslateEvent does the mapping? It's a huge chunk of code here
that doesn't really have anything to do with the translateclient.

RECORD_IF(BROWSER_WINDOW_NOT_VALID);
RECORD_IF(BROWSER_WINDOW_MINIMIZED);
...

[Roger McFarlane (Chromium), 2016/11/16 05:51:31]

I pulled the mapping out to a helper function in the anonymous namespace.

+        translate_manager_->RecordTranslateEvent(
+            metrics::TranslateEventProto::BROWSER_WINDOW_IS_INVALID);
+        break;
+      case ShowTranslateBubbleResult::BROWSER_WINDOW_MINIMIZED:
+        translate_manager_->RecordTranslateEvent(
+            metrics::TranslateEventProto::BROWSER_WINDOW_IS_MINIMIZED);
+        break;
+      case ShowTranslateBubbleResult::BROWSER_WINDOW_NOT_ACTIVE:
+        translate_manager_->RecordTranslateEvent(
+            metrics::TranslateEventProto::BROWSER_WINDOW_NOT_ACTIVE);
+        break;
+      case ShowTranslateBubbleResult::WEB_CONTENTS_NOT_ACTIVE:
+        translate_manager_->RecordTranslateEvent(
+            metrics::TranslateEventProto::WEB_CONTENTS_NOT_ACTIVE);
+        break;
+      case ShowTranslateBubbleResult::EDITABLE_FIELD_IS_ACTIVE:
+        translate_manager_->RecordTranslateEvent(
+            metrics::TranslateEventProto::EDITABLE_FIELD_IS_ACTIVE);
+        break;
+    }
   }
-  ShowBubble(step, error_type);
 }
 
 translate::TranslateDriver* ChromeTranslateClient::GetTranslateDriver() {
@@ -305,9 +339,10 @@ void ChromeTranslateClient::OnPageTranslated(
       content::Details<translate::PageTranslatedDetails>(&details));
 }
 
-void ChromeTranslateClient::ShowBubble(
+ShowTranslateBubbleResult ChromeTranslateClient::ShowBubble(
     translate::TranslateStep step,
     translate::TranslateErrors::Type error_type) {
+  DCHECK(translate_manager_);
 // The bubble is implemented only on the desktop platforms.
 #if !defined(OS_ANDROID)
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
@@ -315,31 +350,31 @@ void ChromeTranslateClient::ShowBubble(
   // |browser| might be NULL when testing. In this case, Show(...) should be
   // called because the implementation for testing is used.
   if (!browser) {
-    TranslateBubbleFactory::Show(NULL, web_contents(), step, error_type);
-    return;
+    return TranslateBubbleFactory::Show(NULL, web_contents(), step, error_type);
   }
 
   if (web_contents() != browser->tab_strip_model()->GetActiveWebContents())
-    return;
+    return ShowTranslateBubbleResult::WEB_CONTENTS_NOT_ACTIVE;
 
-  // This ShowBubble function is also used for upating the existing bubble.
+  // This ShowBubble function is also used for updating the existing bubble.
   // However, with the bubble shown, any browser windows are NOT activated
   // because the bubble takes the focus from the other widgets including the
   // browser windows. So it is checked that |browser| is the last activated
   // browser, not is now activated.
   if (browser != chrome::FindLastActive())
-    return;
+    return ShowTranslateBubbleResult::BROWSER_WINDOW_NOT_ACTIVE;
 
   // During auto-translating, the bubble should not be shown.
   if (step == translate::TRANSLATE_STEP_TRANSLATING ||
       step == translate::TRANSLATE_STEP_AFTER_TRANSLATE) {
     if (GetLanguageState().InTranslateNavigation())
-      return;
+      return ShowTranslateBubbleResult::SUCCESS;
   }
 
-  TranslateBubbleFactory::Show(browser->window(), web_contents(), step,
-                               error_type);
+  return TranslateBubbleFactory::Show(browser->window(), web_contents(), step,
+                                      error_type);
 #else
   NOTREACHED();
+  return ShowTranslateBubbleResult::SUCCESS;
 #endif
 }