Quick Answers: Handle preferred languages for translation

Bug: b/173185932
Test: Run IntentGeneratorTest.* in chromeos_components_unittests
Change-Id: I4f78d5713e7161b059d71cb9985461fa2f4e0155
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537596Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Jeroen Dhollander <jeroendh@chromium.org>
Commit-Queue: Yue Li <updowndota@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827924}

chrome/browser/renderer_context_menu/quick_answers_menu_observer.cc

@@ -129,18 +129,20 @@ void QuickAnswersMenuObserver::OnEligibilityChanged(bool eligible) {
   is_eligible_ = eligible;
 }
 
-std::string QuickAnswersMenuObserver::GetDeviceLanguage() {
-  return l10n_util::GetLanguage(g_browser_process->GetApplicationLocale());
-}
-
 void QuickAnswersMenuObserver::OnTextSurroundingSelectionAvailable(
     const std::string& selected_text,
     const base::string16& surrounding_text,
     uint32_t start_offset,
     uint32_t end_offset) {
+  PrefService* prefs =
+      Profile::FromBrowserContext(proxy_->GetBrowserContext())->GetPrefs();
+
   Context context;
   context.surrounding_text = base::UTF16ToUTF8(surrounding_text);
-  context.device_properties.language = GetDeviceLanguage();
+  context.device_properties.language =
+      l10n_util::GetLanguage(g_browser_process->GetApplicationLocale());
+  context.device_properties.preferred_languages =
+      prefs->GetString(language::prefs::kPreferredLanguages);
   quick_answers_controller_->MaybeShowQuickAnswers(bounds_in_screen_,
                                                    selected_text, context);
 }

chromeos/components/quick_answers/quick_answers_model.h

@@ -124,6 +124,9 @@ struct QuickAnswer {
 struct DeviceProperties {
   // Device language code.
   std::string language;
+
+  // List (separated by comma) of user preferred languages.
+  std::string preferred_languages;
 };
 
 struct IntentInfo {

chromeos/components/quick_answers/understanding/intent_generator.cc

@@ -8,6 +8,7 @@
 
 #include "base/i18n/case_conversion.h"
 #include "base/no_destructor.h"
+#include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chromeos/components/quick_answers/quick_answers_model.h"
 #include "chromeos/components/quick_answers/utils/quick_answers_utils.h"
@@ -108,6 +109,14 @@ bool ShouldSkipDefinition(const std::string& text) {
   return false;
 }
 
+bool IsPreferredLanguage(const std::string& detected_locale,
+                         const std::string& preferred_languages_string) {
+  auto preferred_languages =
+      base::SplitString(preferred_languages_string, ",", base::TRIM_WHITESPACE,
+                        base::SPLIT_WANT_NONEMPTY);
+  return base::Contains(preferred_languages, detected_locale);
+}
+
 }  // namespace
 
 IntentGenerator::IntentGenerator(IntentGeneratorCallback complete_callback)
@@ -184,9 +193,14 @@ void IntentGenerator::LanguageDetectorCallback(
     const QuickAnswersRequest& request,
     base::Optional<std::string> detected_locale) {
   language_detector_.reset();
+
+  // Generate translation intent if the detected language is different to the
+  // system language and is not one of the preferred languages.
   if (detected_locale.has_value() &&
-      !request.context.device_properties.language.empty() &&
-      detected_locale.value() != request.context.device_properties.language) {
+      detected_locale.value() != request.context.device_properties.language &&
+      !IsPreferredLanguage(
+          detected_locale.value(),
+          request.context.device_properties.preferred_languages)) {
     MaybeGenerateTranslationIntent(request, detected_locale.value());
     return;
   }
@@ -219,8 +233,9 @@ void IntentGenerator::MaybeGenerateTranslationIntent(
     return;
   }
 
-  // Don't do language detection if no device language is provided or the length
-  // of selected text is above the threshold. Returns unknown intent type.
+  // Don't generate translation intent if no device language is provided or the
+  // length of selected text is above the threshold. Returns unknown intent
+  // type.
   if (request.context.device_properties.language.empty() ||
       request.selected_text.length() > kTranslationTextLengthThreshold) {
     std::move(complete_callback_)

chromeos/components/quick_answers/understanding/intent_generator_unittest.cc

@@ -90,10 +90,12 @@ TEST_F(IntentGeneratorTest, TranslationIntent) {
   QuickAnswersRequest request;
   request.selected_text = "quick answers";
   request.context.device_properties.language = "es";
+  request.context.device_properties.preferred_languages = "es";
   intent_generator_->GenerateIntent(request);
 
   task_environment_.RunUntilIdle();
 
+  // Should generate translation intent.
   EXPECT_EQ(IntentType::kTranslation, intent_info_.intent_type);
   EXPECT_EQ("quick answers", intent_info_.intent_text);
   EXPECT_EQ("en", intent_info_.source_language);
@@ -108,10 +110,32 @@ TEST_F(IntentGeneratorTest, TranslationIntentSameLanguage) {
   QuickAnswersRequest request;
   request.selected_text = "quick answers";
   request.context.device_properties.language = "en";
+  request.context.device_properties.preferred_languages = "en";
   intent_generator_->GenerateIntent(request);
 
   task_environment_.RunUntilIdle();
 
+  // Should not generate translation intent since the detected language is the
+  // same as system language.
+  EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
+  EXPECT_EQ("quick answers", intent_info_.intent_text);
+}
+
+TEST_F(IntentGeneratorTest, TranslationIntentPreferredLanguage) {
+  std::vector<TextLanguagePtr> languages;
+  languages.push_back(DefaultLanguage());
+  UseFakeServiceConnection({}, languages);
+
+  QuickAnswersRequest request;
+  request.selected_text = "quick answers";
+  request.context.device_properties.language = "es";
+  request.context.device_properties.preferred_languages = "es,en,zh";
+  intent_generator_->GenerateIntent(request);
+
+  task_environment_.RunUntilIdle();
+
+  // Should not generate translation intent since the detected language is in
+  // the preferred languages list.
   EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
   EXPECT_EQ("quick answers", intent_info_.intent_text);
 }
@@ -127,10 +151,13 @@ TEST_F(IntentGeneratorTest, TranslationIntentTextLengthAboveThreshold) {
       "more. Google has many special features to help you find exactly what "
       "you're looking ...";
   request.context.device_properties.language = "es";
+  request.context.device_properties.preferred_languages = "es";
   intent_generator_->GenerateIntent(request);
 
   task_environment_.RunUntilIdle();
 
+  // Should not generate translation intent since the length of the selected
+  // text is above the threshold.
   EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
   EXPECT_EQ(
       "Search the world's information, including webpages, images, videos and "
@@ -143,6 +170,7 @@ TEST_F(IntentGeneratorTest, TranslationIntentWithAnnotation) {
   QuickAnswersRequest request;
   request.selected_text = "unfathomable";
   request.context.device_properties.language = "es";
+  request.context.device_properties.preferred_languages = "es";
 
   // Create the test annotations.
   std::vector<TextEntityPtr> entities;
@@ -165,6 +193,8 @@ TEST_F(IntentGeneratorTest, TranslationIntentWithAnnotation) {
 
   task_environment_.RunUntilIdle();
 
+  // Should generate translation intent which is prioritized against
+  // annotations.
   EXPECT_EQ(IntentType::kTranslation, intent_info_.intent_type);
   EXPECT_EQ("unfathomable", intent_info_.intent_text);
   EXPECT_EQ("en", intent_info_.source_language);
@@ -183,10 +213,12 @@ TEST_F(IntentGeneratorTest, TranslationIntentNotEnabled) {
   QuickAnswersRequest request;
   request.selected_text = "quick answers";
   request.context.device_properties.language = "es";
+  request.context.device_properties.preferred_languages = "es";
   intent_generator_->GenerateIntent(request);
 
   task_environment_.RunUntilIdle();
 
+  // Should not generate translation intent since the feature is not enabled.
   EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
   EXPECT_EQ("quick answers", intent_info_.intent_text);
 }
@@ -202,6 +234,8 @@ TEST_F(IntentGeneratorTest, TranslationIntentDeviceLanguageNotSet) {
 
   task_environment_.RunUntilIdle();
 
+  // Should not generate translation intent since the device language is not
+  // set.
   EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
   EXPECT_EQ("quick answers", intent_info_.intent_text);
 }
@@ -231,6 +265,7 @@ TEST_F(IntentGeneratorTest, TextAnnotationDefinitionIntent) {
 
   task_environment_.RunUntilIdle();
 
+  // Should generate dictionary intent.
   EXPECT_EQ(IntentType::kDictionary, intent_info_.intent_type);
   EXPECT_EQ("unfathomable", intent_info_.intent_text);
 }
@@ -261,6 +296,8 @@ TEST_F(IntentGeneratorTest,
 
   task_environment_.RunUntilIdle();
 
+  // Should generate dictionary intent since the extra characters is below the
+  // threshold.
   EXPECT_EQ(IntentType::kDictionary, intent_info_.intent_type);
   EXPECT_EQ("unfathomable", intent_info_.intent_text);
 }
@@ -291,6 +328,8 @@ TEST_F(IntentGeneratorTest,
 
   task_environment_.RunUntilIdle();
 
+  // Should not generate dictionary intent since the extra characters is above
+  // the threshold.
   EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
   EXPECT_EQ("unfathomable", intent_info_.intent_text);
 }
@@ -320,6 +359,7 @@ TEST_F(IntentGeneratorTest, TextAnnotationUnitIntentExtraChars) {
 
   task_environment_.RunUntilIdle();
 
+  // Should generate unit conversion intent.
   EXPECT_EQ(IntentType::kUnit, intent_info_.intent_type);
   EXPECT_EQ("23 cm", intent_info_.intent_text);
 }
@@ -349,6 +389,7 @@ TEST_F(IntentGeneratorTest, TextAnnotationUnitIntentUtf16Char) {
 
   task_environment_.RunUntilIdle();
 
+  // Should generate unit conversion intent.
   EXPECT_EQ(IntentType::kUnit, intent_info_.intent_type);
   EXPECT_EQ("350°F", intent_info_.intent_text);
 }
@@ -378,6 +419,8 @@ TEST_F(IntentGeneratorTest, TextAnnotationUnitIntentExtraCharsAboveThreshold) {
 
   task_environment_.RunUntilIdle();
 
+  // Should not generate unit conversion intent since the extra characters is
+  // above the threshold.
   EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
   EXPECT_EQ("23 cm", intent_info_.intent_text);
 }
@@ -393,6 +436,7 @@ TEST_F(IntentGeneratorTest, TextAnnotationIntentNoAnnotation) {
   intent_generator_->GenerateIntent(*quick_answers_request);
   task_environment_.RunUntilIdle();
 
+  // Should generate unknown intent since no annotation found.
   EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
   EXPECT_EQ("the unfathomable reaches of space", intent_info_.intent_text);
 }
@@ -414,6 +458,7 @@ TEST_F(IntentGeneratorTest, TextAnnotationIntentNoEntity) {
   intent_generator_->GenerateIntent(*quick_answers_request);
   task_environment_.RunUntilIdle();
 
+  // Should generate unknown intent since no entity found.
   EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
   EXPECT_EQ("the unfathomable reaches of space", intent_info_.intent_text);
 }
@@ -441,6 +486,7 @@ TEST_F(IntentGeneratorTest, TextAnnotationIntentUnSupportedEntity) {
   intent_generator_->GenerateIntent(*quick_answers_request);
   task_environment_.RunUntilIdle();
 
+  // Should generate unknown intent unsupported entity is provided.
   EXPECT_EQ(IntentType::kUnknown, intent_info_.intent_type);
   EXPECT_EQ("the unfathomable reaches of space", intent_info_.intent_text);
 }