Make A.Suggester.OnSurroundingTextChanged async.

Make A.Suggester.OnSurroundingTextChanged async in order to allow it to use the lacros compatible "GetEnabledSuggestion" function. This function would have to get the current URL which in lacros is an async call through mojo.
Currently A.Suggester OnSurroundingTextChanged is still disabled in lacros (https://crrev.com/c/3559412).
For similar example see: AssistiveSuggester.OnExternalSuggestionsUpdated

Unit tests now has extra coverage for different suggesters when Switch is disabled that didn't exist before and coverage of PersonalInfoSuggester for suggestion behaviours.

minor changes:
1) aliased A.SuggesterSwitch::EnabledSuggestion for better readability
2) OnSurroundingTextChanged now returns void. The return value wasn't used in any call sites except in some tests, which has alternative ways of checking if the function was successful. The return value was ambiguous anyways so its not useful.

Change-Id: I078fcc48fcf7d269c5521572b3867e8de6aeabb1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3551791
Reviewed-by: Curtis McMullan <curtismcmullan@chromium.org>
Reviewed-by: Keith Lee <keithlee@chromium.org>
Commit-Queue: jhtin <jhtin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988347}

chrome/browser/ash/input_method/assistive_suggester.cc

@@ -14,6 +14,7 @@
 #include "base/metrics/user_metrics.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/ash/input_method/assistive_suggester_prefs.h"
+#include "chrome/browser/ash/input_method/assistive_suggester_switch.h"
 #include "chrome/browser/ash/input_method/suggestion_handler_interface.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ -430,12 +431,24 @@ bool AssistiveSuggester::WithinGrammarFragment(int cursor_pos, int anchor_pos) {
   return grammar_fragment_opt != absl::nullopt;
 }
 
-bool AssistiveSuggester::OnSurroundingTextChanged(const std::u16string& text,
+void AssistiveSuggester::OnSurroundingTextChanged(const std::u16string& text,
                                                   int cursor_pos,
                                                   int anchor_pos) {
+  suggester_switch_->GetEnabledSuggestions(base::BindOnce(
+      &AssistiveSuggester::ProcessOnSurroundingTextChanged,
+      weak_ptr_factory_.GetWeakPtr(), text, cursor_pos, anchor_pos));
+}
+
+void AssistiveSuggester::ProcessOnSurroundingTextChanged(
+    const std::u16string& text,
+    int cursor_pos,
+    int anchor_pos,
+    const AssistiveSuggesterSwitch::EnabledSuggestions& enabled_suggestions) {
+  // TODO(b/225988020): Make record assistive match metrics take enabled
+  // suggestions.
   RecordAssistiveMatchMetrics(text, cursor_pos, anchor_pos);
   if (!IsAssistiveFeatureEnabled() || context_id_ == -1)
-    return false;
+    return;
 
   if (IsMultiWordSuggestEnabled()) {
     // Only multi word cares about tracking the current state of the text field
@@ -444,16 +457,17 @@ bool AssistiveSuggester::OnSurroundingTextChanged(const std::u16string& text,
   }
 
   if (WithinGrammarFragment(cursor_pos, anchor_pos) ||
-      !TrySuggestWithSurroundingText(text, cursor_pos, anchor_pos)) {
+      !TrySuggestWithSurroundingText(text, cursor_pos, anchor_pos,
+                                     enabled_suggestions)) {
     DismissSuggestion();
   }
-  return IsSuggestionShown();
 }
 
 bool AssistiveSuggester::TrySuggestWithSurroundingText(
     const std::u16string& text,
     int cursor_pos,
-    int anchor_pos) {
+    int anchor_pos,
+    const AssistiveSuggesterSwitch::EnabledSuggestions& enabled_suggestions) {
   int len = static_cast<int>(text.length());
   if (cursor_pos > 0 && cursor_pos <= len && cursor_pos == anchor_pos &&
       (cursor_pos == len || base::IsAsciiWhitespace(text[cursor_pos])) &&
@@ -463,7 +477,7 @@ bool AssistiveSuggester::TrySuggestWithSurroundingText(
                                                                cursor_pos);
     }
     if (IsAssistPersonalInfoEnabled() &&
-        suggester_switch_->IsPersonalInfoSuggestionAllowed() &&
+        enabled_suggestions.personal_info_suggestions &&
         personal_info_suggester_.TrySuggestWithSurroundingText(text,
                                                                cursor_pos)) {
       current_suggester_ = &personal_info_suggester_;
@@ -473,7 +487,7 @@ bool AssistiveSuggester::TrySuggestWithSurroundingText(
       return true;
     } else if (IsEmojiSuggestAdditionEnabled() &&
                !IsEnhancedEmojiSuggestEnabled() &&
-               suggester_switch_->IsEmojiSuggestionAllowed() &&
+               enabled_suggestions.emoji_suggestions &&
                emoji_suggester_.TrySuggestWithSurroundingText(text,
                                                               cursor_pos)) {
       current_suggester_ = &emoji_suggester_;

chrome/browser/ash/input_method/assistive_suggester.h

@@ -61,7 +61,7 @@ class AssistiveSuggester : public SuggestionsSource {
   // Called when a surrounding text is changed.
   // Returns true if it changes the surrounding text, e.g. a suggestion is
   // generated or dismissed.
-  bool OnSurroundingTextChanged(const std::u16string& text,
+  void OnSurroundingTextChanged(const std::u16string& text,
                                 int cursor_pos,
                                 int anchor_pos);
 
@@ -84,11 +84,20 @@ class AssistiveSuggester : public SuggestionsSource {
   }
 
  private:
+  // Callback that is run after enabled_suggestions is received.
+  void ProcessOnSurroundingTextChanged(
+      const std::u16string& text,
+      int cursor_pos,
+      int anchor_pos,
+      const AssistiveSuggesterSwitch::EnabledSuggestions& enabled_suggestions);
+
   // Returns if any suggestion text should be displayed according to the
   // surrounding text information.
-  bool TrySuggestWithSurroundingText(const std::u16string& text,
-                                     int cursor_pos,
-                                     int anchor_pos);
+  bool TrySuggestWithSurroundingText(
+      const std::u16string& text,
+      int cursor_pos,
+      int anchor_pos,
+      const AssistiveSuggesterSwitch::EnabledSuggestions& enabled_suggestions);
 
   void DismissSuggestion();
 

chrome/browser/ash/input_method/assistive_suggester_unittest.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/ash/keyboard/chrome_keyboard_controller_client.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/autofill/core/browser/test_personal_data_manager.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -29,6 +30,7 @@ namespace {
 using ime::TextSuggestion;
 using ime::TextSuggestionMode;
 using ime::TextSuggestionType;
+using EnabledSuggestions = AssistiveSuggesterSwitch::EnabledSuggestions;
 
 const char kUsEnglishEngineId[] = "xkb:us::eng";
 const char kSpainSpanishEngineId[] = "xkb:es::spa";
@@ -273,10 +275,9 @@ TEST_F(AssistiveSuggesterTest,
   assistive_suggester_ = std::make_unique<AssistiveSuggester>(
       suggestion_handler_.get(), profile_.get(),
       std::make_unique<FakeSuggesterSwitch>(
-          FakeSuggesterSwitch::EnabledSuggestions{
-              .emoji_suggestions = true,
-              .multi_word_suggestions = true,
-              .personal_info_suggestions = true}));
+          EnabledSuggestions{.emoji_suggestions = true,
+                             .multi_word_suggestions = true,
+                             .personal_info_suggestions = true}));
 
   assistive_suggester_->OnActivate(kUsEnglishEngineId);
 
@@ -295,8 +296,7 @@ TEST_F(AssistiveSuggesterTest, RecordPredictiveWritingPrefOnActivate) {
       /*disabled_features=*/{});
   assistive_suggester_ = std::make_unique<AssistiveSuggester>(
       suggestion_handler_.get(), profile_.get(),
-      std::make_unique<FakeSuggesterSwitch>(
-          FakeSuggesterSwitch::EnabledSuggestions{}));
+      std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{}));
 
   SetInputMethodOptions(*profile_, /*predictive_writing_enabled=*/true);
   assistive_suggester_->OnActivate(kUsEnglishEngineId);
@@ -312,8 +312,7 @@ TEST_F(AssistiveSuggesterTest, RecordsMultiWordTextInputAsNotAllowed) {
       /*disabled_features=*/{});
   assistive_suggester_ = std::make_unique<AssistiveSuggester>(
       suggestion_handler_.get(), profile_.get(),
-      std::make_unique<FakeSuggesterSwitch>(
-          FakeSuggesterSwitch::EnabledSuggestions{}));
+      std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{}));
 
   SetInputMethodOptions(*profile_, /*predictive_writing_enabled=*/true);
   assistive_suggester_->OnActivate(kUsEnglishEngineId);
@@ -334,8 +333,7 @@ TEST_F(AssistiveSuggesterTest, RecordsMultiWordTextInputAsDisabledByUser) {
   assistive_suggester_ = std::make_unique<AssistiveSuggester>(
       suggestion_handler_.get(), profile_.get(),
       std::make_unique<FakeSuggesterSwitch>(
-          FakeSuggesterSwitch::EnabledSuggestions{.multi_word_suggestions =
-                                                      true}));
+          EnabledSuggestions{.multi_word_suggestions = true}));
 
   SetInputMethodOptions(*profile_, /*predictive_writing_enabled=*/false);
   assistive_suggester_->OnActivate(kUsEnglishEngineId);
@@ -357,8 +355,7 @@ TEST_F(AssistiveSuggesterTest, RecordsMultiWordTextInputAsDisabledByLacros) {
   assistive_suggester_ = std::make_unique<AssistiveSuggester>(
       suggestion_handler_.get(), profile_.get(),
       std::make_unique<FakeSuggesterSwitch>(
-          FakeSuggesterSwitch::EnabledSuggestions{.multi_word_suggestions =
-                                                      true}));
+          EnabledSuggestions{.multi_word_suggestions = true}));
 
   SetInputMethodOptions(*profile_, /*predictive_writing_enabled=*/true);
   assistive_suggester_->OnActivate(kUsEnglishEngineId);
@@ -380,8 +377,7 @@ TEST_F(AssistiveSuggesterTest,
   assistive_suggester_ = std::make_unique<AssistiveSuggester>(
       suggestion_handler_.get(), profile_.get(),
       std::make_unique<FakeSuggesterSwitch>(
-          FakeSuggesterSwitch::EnabledSuggestions{.multi_word_suggestions =
-                                                      true}));
+          EnabledSuggestions{.multi_word_suggestions = true}));
 
   SetInputMethodOptions(*profile_, /*predictive_writing_enabled=*/true);
   assistive_suggester_->OnActivate(kSpainSpanishEngineId);
@@ -402,8 +398,7 @@ TEST_F(AssistiveSuggesterTest, RecordsMultiWordTextInputAsEnabled) {
   assistive_suggester_ = std::make_unique<AssistiveSuggester>(
       suggestion_handler_.get(), profile_.get(),
       std::make_unique<FakeSuggesterSwitch>(
-          FakeSuggesterSwitch::EnabledSuggestions{.multi_word_suggestions =
-                                                      true}));
+          EnabledSuggestions{.multi_word_suggestions = true}));
 
   SetInputMethodOptions(*profile_, /*predictive_writing_enabled=*/true);
   assistive_suggester_->OnActivate(kUsEnglishEngineId);
@@ -416,6 +411,82 @@ TEST_F(AssistiveSuggesterTest, RecordsMultiWordTextInputAsEnabled) {
       AssistiveTextInputState::kFeatureEnabled, 1);
 }
 
+class AssistiveSuggesterPersonalInfoTest : public testing::Test {
+ protected:
+  AssistiveSuggesterPersonalInfoTest() {
+    profile_ = std::make_unique<TestingProfile>();
+  }
+
+  void SetUp() override {
+    suggestion_handler_ = std::make_unique<FakeSuggestionHandler>();
+    assistive_suggester_ = std::make_unique<AssistiveSuggester>(
+        suggestion_handler_.get(), profile_.get(),
+        std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{
+            .personal_info_suggestions = true,
+        }));
+    personal_data_ = std::make_unique<autofill::TestPersonalDataManager>();
+    personal_data_->SetPrefService(profile_->GetPrefs());
+
+    feature_list_.InitWithFeatures(
+        /*enabled_features=*/{features::kAssistPersonalInfo,
+                              features::kAssistPersonalInfoEmail},
+        /*disabled_features=*/{});
+
+    profile_->GetPrefs()->SetBoolean(prefs::kAssistPersonalInfoEnabled, true);
+    SetInputMethodOptions(*profile_, /*predictive_writing_enabled=*/true);
+    profile_->set_profile_name(base::UTF16ToUTF8(email_));
+    chrome_keyboard_controller_client_ =
+        ChromeKeyboardControllerClient::CreateForTest();
+    chrome_keyboard_controller_client_->set_keyboard_visible_for_test(false);
+  }
+
+  content::BrowserTaskEnvironment task_environment_;
+  base::test::ScopedFeatureList feature_list_;
+  std::unique_ptr<TestingProfile> profile_;
+  std::unique_ptr<AssistiveSuggester> assistive_suggester_;
+  std::unique_ptr<FakeSuggestionHandler> suggestion_handler_;
+  std::unique_ptr<autofill::TestPersonalDataManager> personal_data_;
+  base::HistogramTester histogram_tester_;
+  std::unique_ptr<ChromeKeyboardControllerClient>
+      chrome_keyboard_controller_client_;
+
+  const std::u16string email_ = u"johnwayne@me.xyz";
+};
+
+TEST_F(AssistiveSuggesterPersonalInfoTest, ShouldNotSuggestWhenPrefDisabled) {
+  profile_->GetPrefs()->SetBoolean(prefs::kAssistPersonalInfoEnabled, false);
+  assistive_suggester_->OnActivate(kUsEnglishEngineId);
+  assistive_suggester_->OnFocus(5);
+
+  assistive_suggester_->OnSurroundingTextChanged(u"my email is ", 12, 12);
+
+  EXPECT_FALSE(suggestion_handler_->GetShowingSuggestion());
+}
+
+TEST_F(AssistiveSuggesterPersonalInfoTest, ShouldNotSuggestWhenSwitchDisabled) {
+  assistive_suggester_ = std::make_unique<AssistiveSuggester>(
+      suggestion_handler_.get(), profile_.get(),
+      std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{
+          .personal_info_suggestions = false,
+      }));
+  assistive_suggester_->OnActivate(kUsEnglishEngineId);
+  assistive_suggester_->OnFocus(5);
+
+  assistive_suggester_->OnSurroundingTextChanged(u"my email is ", 12, 12);
+
+  EXPECT_FALSE(suggestion_handler_->GetShowingSuggestion());
+}
+
+TEST_F(AssistiveSuggesterPersonalInfoTest, ShouldReturnPrefixBasedSuggestions) {
+  assistive_suggester_->OnActivate(kUsEnglishEngineId);
+  assistive_suggester_->OnFocus(5);
+
+  assistive_suggester_->OnSurroundingTextChanged(u"my email is ", 12, 12);
+
+  EXPECT_TRUE(suggestion_handler_->GetShowingSuggestion());
+  EXPECT_EQ(suggestion_handler_->GetSuggestionText(), u"johnwayne@me.xyz");
+}
+
 class AssistiveSuggesterMultiWordTest : public testing::Test {
  protected:
   AssistiveSuggesterMultiWordTest() {
@@ -426,10 +497,9 @@ class AssistiveSuggesterMultiWordTest : public testing::Test {
     suggestion_handler_ = std::make_unique<FakeSuggestionHandler>();
     assistive_suggester_ = std::make_unique<AssistiveSuggester>(
         suggestion_handler_.get(), profile_.get(),
-        std::make_unique<FakeSuggesterSwitch>(
-            FakeSuggesterSwitch::EnabledSuggestions{
-                .multi_word_suggestions = true,
-            }));
+        std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{
+            .multi_word_suggestions = true,
+        }));
 
     feature_list_.InitWithFeatures(
         /*enabled_features=*/{features::kAssistMultiWord},
@@ -486,6 +556,25 @@ TEST_F(AssistiveSuggesterMultiWordTest, OnDisabledFlagShouldNotShowSuggestion) {
   EXPECT_FALSE(suggestion_handler_->GetShowingSuggestion());
 }
 
+TEST_F(AssistiveSuggesterMultiWordTest, ShouldNotSuggestWhenSwitchDisabled) {
+  assistive_suggester_ = std::make_unique<AssistiveSuggester>(
+      suggestion_handler_.get(), profile_.get(),
+      std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{
+          .multi_word_suggestions = false,
+      }));
+  std::vector<TextSuggestion> suggestions = {
+      TextSuggestion{.mode = TextSuggestionMode::kPrediction,
+                     .type = TextSuggestionType::kMultiWord,
+                     .text = "hello there"}};
+  assistive_suggester_->OnActivate(kUsEnglishEngineId);
+  assistive_suggester_->OnFocus(5);
+  assistive_suggester_->OnSurroundingTextChanged(u"", 0, 0);
+
+  assistive_suggester_->OnExternalSuggestionsUpdated(suggestions);
+
+  EXPECT_FALSE(suggestion_handler_->GetShowingSuggestion());
+}
+
 TEST_F(AssistiveSuggesterMultiWordTest,
        MatchMetricRecordedWhenOneOrMoreSuggestions) {
   std::vector<TextSuggestion> suggestions = {
@@ -526,8 +615,7 @@ TEST_F(AssistiveSuggesterMultiWordTest,
        DisableMetricNotRecordedWhenNoSuggestionAndMultiWordBlocked) {
   assistive_suggester_ = std::make_unique<AssistiveSuggester>(
       suggestion_handler_.get(), profile_.get(),
-      std::make_unique<FakeSuggesterSwitch>(
-          FakeSuggesterSwitch::EnabledSuggestions{}));
+      std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{}));
 
   assistive_suggester_->OnActivate(kUsEnglishEngineId);
   assistive_suggester_->OnFocus(5);
@@ -542,8 +630,7 @@ TEST_F(AssistiveSuggesterMultiWordTest,
        DisableMetricRecordedWhenGivenSuggestionAndMultiWordBlocked) {
   assistive_suggester_ = std::make_unique<AssistiveSuggester>(
       suggestion_handler_.get(), profile_.get(),
-      std::make_unique<FakeSuggesterSwitch>(
-          FakeSuggesterSwitch::EnabledSuggestions{}));
+      std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{}));
   std::vector<TextSuggestion> suggestions = {
       TextSuggestion{.mode = TextSuggestionMode::kPrediction,
                      .type = TextSuggestionType::kMultiWord,
@@ -708,10 +795,9 @@ class AssistiveSuggesterEmojiTest : public testing::Test {
     suggestion_handler_ = std::make_unique<FakeSuggestionHandler>();
     assistive_suggester_ = std::make_unique<AssistiveSuggester>(
         suggestion_handler_.get(), profile_.get(),
-        std::make_unique<FakeSuggesterSwitch>(
-            FakeSuggesterSwitch::EnabledSuggestions{
-                .emoji_suggestions = true,
-            }));
+        std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{
+            .emoji_suggestions = true,
+        }));
     assistive_suggester_->get_emoji_suggester_for_testing()
         ->LoadEmojiMapForTesting(kEmojiData);
 
@@ -744,16 +830,29 @@ TEST_F(AssistiveSuggesterEmojiTest, ShouldNotSuggestWhenEmojiDisabled) {
 
   assistive_suggester_->OnActivate(kUsEnglishEngineId);
   assistive_suggester_->OnFocus(5);
+  assistive_suggester_->OnSurroundingTextChanged(u"arrow ", 6, 6);
+
+  EXPECT_FALSE(suggestion_handler_->GetShowingSuggestion());
+}
+
+TEST_F(AssistiveSuggesterEmojiTest, ShouldNotSuggestWhenSwitchDisabled) {
+  assistive_suggester_ = std::make_unique<AssistiveSuggester>(
+      suggestion_handler_.get(), profile_.get(),
+      std::make_unique<FakeSuggesterSwitch>(EnabledSuggestions{
+          .emoji_suggestions = false,
+      }));
+  assistive_suggester_->OnActivate(kUsEnglishEngineId);
+  assistive_suggester_->OnFocus(5);
+  assistive_suggester_->OnSurroundingTextChanged(u"arrow ", 6, 6);
 
-  EXPECT_FALSE(assistive_suggester_->OnSurroundingTextChanged(u"arrow ", 6, 6));
   EXPECT_FALSE(suggestion_handler_->GetShowingSuggestion());
 }
 
 TEST_F(AssistiveSuggesterEmojiTest, ShouldReturnPrefixBasedEmojiSuggestions) {
   assistive_suggester_->OnActivate(kUsEnglishEngineId);
   assistive_suggester_->OnFocus(5);
+  assistive_suggester_->OnSurroundingTextChanged(u"arrow ", 6, 6);
 
-  EXPECT_TRUE(assistive_suggester_->OnSurroundingTextChanged(u"arrow ", 6, 6));
   EXPECT_TRUE(suggestion_handler_->GetShowingSuggestion());
   EXPECT_EQ(suggestion_handler_->GetSuggestionText(), u"←");
 }