Added tracking of translation errors that occur over the course of a

page load.

This CL adds tracking of translate errors over the course of a page
load. This summary includes the first translation error and the total
number of errors. This information will be later added to the new
Translate UKM (see design doc for full details).

Full design doc: https://docs.google.com/document/d/1dyWh1Xw5VgUA00VA-5PTgKQ6ItziPBnSyeDR8saJ9vM/edit?usp=sharing

Bug: 1114868
Change-Id: I92b4912cd175c6e5943c24e095d42eb8091c2e14
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2556339
Commit-Queue: Max Curran <curranmax@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Scott Little <sclittle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832505}

chrome/browser/page_load_metrics/observers/translate_page_load_metrics_observer_unittest.cc

@@ -58,8 +58,9 @@ class MockTranslateMetricsLoggerContainer
     mock_translate_metrics_logger_->LogTranslationStarted();
   }
 
-  void LogTranslationFinished(bool was_sucessful) override {
-    mock_translate_metrics_logger_->LogTranslationFinished(was_sucessful);
+  void LogTranslationFinished(
+      translate::TranslateErrors::Type error_type) override {
+    mock_translate_metrics_logger_->LogTranslationFinished(error_type);
   }
 
   void LogReversion() override {

components/translate/content/browser/content_translate_driver.cc

@@ -325,8 +325,9 @@ void ContentTranslateDriver::OnPageTranslated(
     TranslateErrors::Type error_type) {
   if (cancelled) {
     // Informs the |TranslateMetricsLogger| that the translation was cancelled.
+    DCHECK(error_type != TranslateErrors::NONE);
     translate_manager_->GetActiveTranslateMetricsLogger()
-        ->LogTranslationFinished(false);
+        ->LogTranslationFinished(error_type);
     return;
   }
 

components/translate/core/browser/mock_translate_metrics_logger.h

@@ -35,7 +35,7 @@ class MockTranslateMetricsLogger : public TranslateMetricsLogger {
   MOCK_METHOD0(LogAutofillAssistantDeferredTriggerDecision, void());
   MOCK_METHOD0(LogInitialState, void());
   MOCK_METHOD0(LogTranslationStarted, void());
-  MOCK_METHOD1(LogTranslationFinished, void(bool));
+  MOCK_METHOD1(LogTranslationFinished, void(TranslateErrors::Type));
   MOCK_METHOD0(LogReversion, void());
   MOCK_METHOD1(LogUIChange, void(bool));
   MOCK_METHOD1(LogOmniboxIconChange, void(bool));

components/translate/core/browser/translate_manager.cc

@@ -522,8 +522,7 @@ void TranslateManager::PageTranslated(const std::string& source_lang,
                                      false);
   NotifyTranslateError(error_type);
 
-  GetActiveTranslateMetricsLogger()->LogTranslationFinished(
-      error_type == TranslateErrors::NONE);
+  GetActiveTranslateMetricsLogger()->LogTranslationFinished(error_type);
 }
 
 void TranslateManager::OnTranslateScriptFetchComplete(
@@ -545,7 +544,8 @@ void TranslateManager::OnTranslateScriptFetchComplete(
         translate::TRANSLATE_STEP_TRANSLATE_ERROR, source_lang, target_lang,
         TranslateErrors::NETWORK, false);
     NotifyTranslateError(TranslateErrors::NETWORK);
-    GetActiveTranslateMetricsLogger()->LogTranslationFinished(false);
+    GetActiveTranslateMetricsLogger()->LogTranslationFinished(
+        TranslateErrors::NETWORK);
   }
 }
 

components/translate/core/browser/translate_metrics_logger.h

@@ -8,6 +8,8 @@
 #include <stdint.h>
 #include <string>
 
+#include "components/translate/core/common/translate_errors.h"
+
 namespace translate {
 
 // These values are persisted to logs. Entries should not be renumbered and
@@ -85,7 +87,7 @@ class TranslateMetricsLogger {
   // Tracks the state of Translate over the course of the page load.
   virtual void LogInitialState() = 0;
   virtual void LogTranslationStarted() = 0;
-  virtual void LogTranslationFinished(bool was_successful) = 0;
+  virtual void LogTranslationFinished(TranslateErrors::Type error_type) = 0;
   virtual void LogReversion() = 0;
   virtual void LogUIChange(bool is_ui_shown) = 0;
   virtual void LogOmniboxIconChange(bool is_omnibox_icon_show) = 0;

components/translate/core/browser/translate_metrics_logger_impl.cc

@@ -161,8 +161,10 @@ void TranslateMetricsLoggerImpl::LogTranslationStarted() {
   is_translation_in_progress_ = true;
 }
 
-void TranslateMetricsLoggerImpl::LogTranslationFinished(bool was_sucessful) {
-  if (was_sucessful) {
+void TranslateMetricsLoggerImpl::LogTranslationFinished(
+    TranslateErrors::Type error_type) {
+  // The translation succeeded if and only if there were no translation errors.
+  if (error_type == TranslateErrors::NONE) {
     UpdateTimeTranslated(previous_state_is_translated_, is_foreground_);
     num_translations_++;
   } else {
@@ -172,6 +174,12 @@ void TranslateMetricsLoggerImpl::LogTranslationFinished(bool was_sucessful) {
     // Update the initial state if it was dependent on this translation..
     if (is_initial_state_dependent_on_in_progress_translation_)
       initial_state_is_translated_ = previous_state_is_translated_;
+
+    // Check if this was the first error, and then increment the number of
+    // errors for this page load.
+    if (first_translate_error_type_ == TranslateErrors::NONE)
+      first_translate_error_type_ = error_type;
+    num_translate_errors_++;
   }
 
   is_translation_in_progress_ = false;

components/translate/core/browser/translate_metrics_logger_impl.h

@@ -45,7 +45,7 @@ class NullTranslateMetricsLogger : public TranslateMetricsLogger {
   void LogAutofillAssistantDeferredTriggerDecision() override {}
   void LogInitialState() override {}
   void LogTranslationStarted() override {}
-  void LogTranslationFinished(bool was_sucessful) override {}
+  void LogTranslationFinished(TranslateErrors::Type error_type) override {}
   void LogReversion() override {}
   void LogUIChange(bool is_ui_shown) override {}
   void LogOmniboxIconChange(bool is_omnibox_icon_shown) override {}
@@ -85,7 +85,7 @@ class TranslateMetricsLoggerImpl : public TranslateMetricsLogger {
   void LogAutofillAssistantDeferredTriggerDecision() override;
   void LogInitialState() override;
   void LogTranslationStarted() override;
-  void LogTranslationFinished(bool was_sucessful) override;
+  void LogTranslationFinished(TranslateErrors::Type error_type) override;
   void LogReversion() override;
   void LogUIChange(bool is_ui_shown) override;
   void LogOmniboxIconChange(bool is_omnibox_icon_shown) override;
@@ -167,6 +167,10 @@ class TranslateMetricsLoggerImpl : public TranslateMetricsLogger {
   std::string current_target_language_;
   int num_target_language_changes_ = 0;
 
+  // Tracks any translation errors that occur over the course of the page load.
+  TranslateErrors::Type first_translate_error_type_ = TranslateErrors::NONE;
+  int num_translate_errors_ = 0;
+
   base::WeakPtrFactory<TranslateMetricsLoggerImpl> weak_method_factory_{this};
 };
 

components/translate/core/browser/translate_metrics_logger_impl_unittest.cc

@@ -46,6 +46,14 @@ class TranslateMetricsLoggerImplTest : public ::testing::Test {
                                            expected_num_reversions, 1);
   }
 
+  void CheckTranslateErrors(TranslateErrors::Type first_translate_error_type,
+                            int num_translate_errors) {
+    EXPECT_EQ(translate_metrics_logger_->first_translate_error_type_,
+              first_translate_error_type);
+    EXPECT_EQ(translate_metrics_logger_->num_translate_errors_,
+              num_translate_errors);
+  }
+
   void CheckTotalTimeTranslated(base::TimeDelta total_time_translated,
                                 base::TimeDelta total_time_not_translated) {
     EXPECT_EQ(translate_metrics_logger_->total_time_translated_,
@@ -76,7 +84,7 @@ TEST_F(TranslateMetricsLoggerImplTest, MultipleRecordMetrics) {
   translate_metrics_logger()->LogInitialState();
   translate_metrics_logger()->LogUIChange(true);
   translate_metrics_logger()->LogTranslationStarted();
-  translate_metrics_logger()->LogTranslationFinished(true);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NONE);
   translate_metrics_logger()->LogReversion();
 
   // Simulate |RecordMetrics| being called multiple times.
@@ -152,24 +160,26 @@ TEST_F(TranslateMetricsLoggerImplTest, LogTranslationAndReversion) {
   translate_metrics_logger()->LogInitialState();
 
   translate_metrics_logger()->LogTranslationStarted();
-  translate_metrics_logger()->LogTranslationFinished(true);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NONE);
 
   translate_metrics_logger()->RecordMetrics(true);
 
   CheckTranslateStateHistograms(TranslateState::kNotTranslatedNoUI,
                                 TranslateState::kTranslatedNoUI, 1, 0);
+  CheckTranslateErrors(TranslateErrors::NONE, 0);
 
   // Simulate a failed translation.
   ResetTest();
   translate_metrics_logger()->LogInitialState();
 
   translate_metrics_logger()->LogTranslationStarted();
-  translate_metrics_logger()->LogTranslationFinished(false);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NETWORK);
 
   translate_metrics_logger()->RecordMetrics(true);
 
   CheckTranslateStateHistograms(TranslateState::kNotTranslatedNoUI,
                                 TranslateState::kNotTranslatedNoUI, 0, 0);
+  CheckTranslateErrors(TranslateErrors::NETWORK, 1);
 
   // Simulate a translation that does not finish.
   ResetTest();
@@ -181,6 +191,7 @@ TEST_F(TranslateMetricsLoggerImplTest, LogTranslationAndReversion) {
 
   CheckTranslateStateHistograms(TranslateState::kNotTranslatedNoUI,
                                 TranslateState::kNotTranslatedNoUI, 0, 0);
+  CheckTranslateErrors(TranslateErrors::NONE, 0);
 
   // Simulate translating an already translated page, but the second translation
   // fails.
@@ -188,14 +199,15 @@ TEST_F(TranslateMetricsLoggerImplTest, LogTranslationAndReversion) {
   translate_metrics_logger()->LogInitialState();
 
   translate_metrics_logger()->LogTranslationStarted();
-  translate_metrics_logger()->LogTranslationFinished(true);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NONE);
   translate_metrics_logger()->LogTranslationStarted();
-  translate_metrics_logger()->LogTranslationFinished(false);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NETWORK);
 
   translate_metrics_logger()->RecordMetrics(true);
 
   CheckTranslateStateHistograms(TranslateState::kNotTranslatedNoUI,
                                 TranslateState::kTranslatedNoUI, 1, 0);
+  CheckTranslateErrors(TranslateErrors::NETWORK, 1);
 
   // Simulate the page being auto translated. Note that in this case the
   // translation will be queued before we mark the initial state, but in general
@@ -204,23 +216,25 @@ TEST_F(TranslateMetricsLoggerImplTest, LogTranslationAndReversion) {
   ResetTest();
   translate_metrics_logger()->LogTranslationStarted();
   translate_metrics_logger()->LogInitialState();
-  translate_metrics_logger()->LogTranslationFinished(true);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NONE);
 
   translate_metrics_logger()->RecordMetrics(true);
 
   CheckTranslateStateHistograms(TranslateState::kTranslatedNoUI,
                                 TranslateState::kTranslatedNoUI, 1, 0);
+  CheckTranslateErrors(TranslateErrors::NONE, 0);
 
   // Simulate an auto translation where the translation fails.
   ResetTest();
   translate_metrics_logger()->LogTranslationStarted();
   translate_metrics_logger()->LogInitialState();
-  translate_metrics_logger()->LogTranslationFinished(false);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NETWORK);
 
   translate_metrics_logger()->RecordMetrics(true);
 
   CheckTranslateStateHistograms(TranslateState::kNotTranslatedNoUI,
                                 TranslateState::kNotTranslatedNoUI, 0, 0);
+  CheckTranslateErrors(TranslateErrors::NETWORK, 1);
 
   // Simulate an auto translation where the translation does not finish.
   ResetTest();
@@ -231,6 +245,7 @@ TEST_F(TranslateMetricsLoggerImplTest, LogTranslationAndReversion) {
 
   CheckTranslateStateHistograms(TranslateState::kNotTranslatedNoUI,
                                 TranslateState::kNotTranslatedNoUI, 0, 0);
+  CheckTranslateErrors(TranslateErrors::NONE, 0);
 
   // Simualte a page that is repeatedly translated and then reverted.
   ResetTest();
@@ -241,7 +256,7 @@ TEST_F(TranslateMetricsLoggerImplTest, LogTranslationAndReversion) {
 
   for (int i = 0; i < num_translations_and_reversions; i++) {
     translate_metrics_logger()->LogTranslationStarted();
-    translate_metrics_logger()->LogTranslationFinished(true);
+    translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NONE);
     translate_metrics_logger()->LogReversion();
   }
 
@@ -250,6 +265,39 @@ TEST_F(TranslateMetricsLoggerImplTest, LogTranslationAndReversion) {
   CheckTranslateStateHistograms(
       TranslateState::kNotTranslatedNoUI, TranslateState::kNotTranslatedNoUI,
       num_translations_and_reversions, num_translations_and_reversions);
+  CheckTranslateErrors(TranslateErrors::NONE, 0);
+}
+
+TEST_F(TranslateMetricsLoggerImplTest, LogTranslateErrors) {
+  // Sets the sequences of errors to supply.
+  const TranslateErrors::Type kTranslateErrorTypes[] = {
+      TranslateErrors::NONE,
+      TranslateErrors::NETWORK,
+      TranslateErrors::INITIALIZATION_ERROR,
+      TranslateErrors::NONE,
+      TranslateErrors::UNSUPPORTED_LANGUAGE,
+      TranslateErrors::TRANSLATION_ERROR,
+      TranslateErrors::NONE,
+      TranslateErrors::TRANSLATION_TIMEOUT,
+      TranslateErrors::SCRIPT_LOAD_ERROR,
+      TranslateErrors::NONE};
+
+  // Simulates the translations with the predefined errors.
+  for (auto translate_error_type : kTranslateErrorTypes) {
+    translate_metrics_logger()->LogTranslationStarted();
+    translate_metrics_logger()->LogTranslationFinished(translate_error_type);
+  }
+
+  translate_metrics_logger()->RecordMetrics(true);
+
+  // We expect to capture the first non-NONE value, and the total number of
+  // non-NONE errors.
+  CheckTranslateErrors(TranslateErrors::NETWORK, 6);
+
+  // The number of successful translations is equal to the number of NONE
+  // errors.
+  histogram_tester()->ExpectUniqueSample(kTranslatePageLoadNumTranslations, 4,
+                                         1);
 }
 
 TEST_F(TranslateMetricsLoggerImplTest, LogTranslateState) {
@@ -261,7 +309,7 @@ TEST_F(TranslateMetricsLoggerImplTest, LogTranslateState) {
   translate_metrics_logger()->LogInitialState();
 
   translate_metrics_logger()->LogTranslationStarted();
-  translate_metrics_logger()->LogTranslationFinished(true);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NONE);
   translate_metrics_logger()->LogUIChange(true);
   translate_metrics_logger()->LogOmniboxIconChange(true);
 
@@ -278,7 +326,7 @@ TEST_F(TranslateMetricsLoggerImplTest, LogTranslateState) {
   translate_metrics_logger()->LogUIChange(true);
   translate_metrics_logger()->LogOmniboxIconChange(true);
   translate_metrics_logger()->LogInitialState();
-  translate_metrics_logger()->LogTranslationFinished(true);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NONE);
 
   translate_metrics_logger()->LogReversion();
   translate_metrics_logger()->LogUIChange(false);
@@ -313,7 +361,7 @@ TEST_F(TranslateMetricsLoggerImplTest, TrackTimeTranslatedAndNotTranslated) {
 
   // Translate the page (while still in the background).
   translate_metrics_logger()->LogTranslationStarted();
-  translate_metrics_logger()->LogTranslationFinished(true);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NONE);
 
   test_clock.Advance(delay3);
 
@@ -352,7 +400,7 @@ TEST_F(TranslateMetricsLoggerImplTest,
   test_clock.Advance(delay2);
 
   // Translation finally finishes.
-  translate_metrics_logger()->LogTranslationFinished(true);
+  translate_metrics_logger()->LogTranslationFinished(TranslateErrors::NONE);
 
   test_clock.Advance(delay3);