ime: Make AutocorrectManager more testable and add unit tests.

AutocorrectManager was hard to test because it required a full
InputMethodEngine as a dependency. However, it really only needs two
dependencies: an input context handler to manipulate text, and a
suggestion handler to manipulate the autocorrect range.

Pass in the suggestion handler via the ctor, and grab the input context
handler via IMEBridge*.

Add a few unit tests with mock handlers. In a future patch, we will
add a proper fake input context handler so that it's easy to test
UndoAutocorrect.

* Note that there's an open bug to remove IMEBridge as it is global
state. See crbug.com/1149751.

Bug: b/172969200
Change-Id: Ic9deefb873ee94895846bd91efe608ef10418eb8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2543537
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: My Nguyen <myy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828933}

chrome/browser/chromeos/BUILD.gn

@@ -3524,6 +3524,7 @@ source_set("unit_tests") {
     "hats/hats_notification_controller_unittest.cc",
     "input_method/assistive_suggester_unittest.cc",
     "input_method/assistive_window_controller_unittest.cc",
+    "input_method/autocorrect_manager_unittest.cc",
     "input_method/emoji_suggester_unittest.cc",
     "input_method/input_method_configuration_unittest.cc",
     "input_method/input_method_engine_unittest.cc",

chrome/browser/chromeos/input_method/assistive_window_properties.cc

@@ -9,4 +9,17 @@ namespace chromeos {
 AssistiveWindowProperties::AssistiveWindowProperties() = default;
 AssistiveWindowProperties::~AssistiveWindowProperties() = default;
 
+AssistiveWindowProperties::AssistiveWindowProperties(
+    const AssistiveWindowProperties& other) = default;
+AssistiveWindowProperties& AssistiveWindowProperties::operator=(
+    const AssistiveWindowProperties& other) = default;
+
+bool AssistiveWindowProperties::operator==(
+    const AssistiveWindowProperties& other) const {
+  return type == other.type && visible == other.visible &&
+         announce_string == other.announce_string &&
+         candidates == other.candidates && show_indices == other.show_indices &&
+         show_setting_link == other.show_setting_link;
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/input_method/assistive_window_properties.h

@@ -14,6 +14,11 @@ struct AssistiveWindowProperties {
   AssistiveWindowProperties();
   ~AssistiveWindowProperties();
 
+  AssistiveWindowProperties(const AssistiveWindowProperties& other);
+  AssistiveWindowProperties& operator=(const AssistiveWindowProperties& other);
+
+  bool operator==(const AssistiveWindowProperties& other) const;
+
   ui::ime::AssistiveWindowType type = ui::ime::AssistiveWindowType::kNone;
   bool visible = false;
   std::string announce_string;

chrome/browser/chromeos/input_method/autocorrect_manager.cc

@@ -16,8 +16,9 @@ namespace chromeos {
 
 constexpr int kKeysUntilAutocorrectWindowHides = 4;
 
-AutocorrectManager::AutocorrectManager(InputMethodEngine* engine)
-    : engine_(engine) {}
+AutocorrectManager::AutocorrectManager(
+    SuggestionHandlerInterface* suggestion_handler)
+    : suggestion_handler_(suggestion_handler) {}
 
 void AutocorrectManager::MarkAutocorrectRange(const std::string& corrected_word,
                                               const std::string& typed_word,
@@ -29,8 +30,11 @@ void AutocorrectManager::MarkAutocorrectRange(const std::string& corrected_word,
   key_presses_until_underline_hide_ = kKeysUntilAutocorrectWindowHides;
   ClearUnderline();
 
-  if (context_id_ != -1) {
-    engine_->SetAutocorrectRange(base::UTF8ToUTF16(corrected_word), start_index,
+  ui::IMEInputContextHandlerInterface* input_context =
+      ui::IMEBridge::Get()->GetInputContextHandler();
+  if (input_context) {
+    input_context->SetAutocorrectRange(base::UTF8ToUTF16(corrected_word),
+                                       start_index,
                                        start_index + corrected_word.length());
   }
 }
@@ -48,7 +52,8 @@ bool AutocorrectManager::OnKeyEvent(
     button.announce_string =
         l10n_util::GetStringFUTF8(IDS_SUGGESTION_AUTOCORRECT_UNDO_BUTTON,
                                   base::UTF8ToUTF16(last_typed_word_));
-    engine_->SetButtonHighlighted(context_id_, button, true, &error);
+    suggestion_handler_->SetButtonHighlighted(context_id_, button, true,
+                                              &error);
     button_highlighted = true;
     return true;
   }
@@ -66,14 +71,20 @@ bool AutocorrectManager::OnKeyEvent(
 }
 
 void AutocorrectManager::ClearUnderline() {
-  engine_->ClearAutocorrectRange();
+  ui::IMEInputContextHandlerInterface* input_context =
+      ui::IMEBridge::Get()->GetInputContextHandler();
+  if (input_context) {
+    input_context->ClearAutocorrectRange();
+  }
 }
 
 void AutocorrectManager::OnSurroundingTextChanged(const base::string16& text,
                                                   const int cursor_pos,
                                                   const int anchpr_pos) {
   std::string error;
-  const gfx::Range range = engine_->GetAutocorrectRange();
+  ui::IMEInputContextHandlerInterface* input_context =
+      ui::IMEBridge::Get()->GetInputContextHandler();
+  const gfx::Range range = input_context->GetAutocorrectRange();
   if (!range.is_empty() && cursor_pos >= range.start() &&
       cursor_pos <= range.end()) {
     if (!window_visible) {
@@ -86,7 +97,8 @@ void AutocorrectManager::OnSurroundingTextChanged(const base::string16& text,
           base::UTF8ToUTF16(last_corrected_word_));
       window_visible = true;
       button_highlighted = false;
-      engine_->SetAssistiveWindowProperties(context_id_, properties, &error);
+      suggestion_handler_->SetAssistiveWindowProperties(context_id_, properties,
+                                                        &error);
     }
     key_presses_until_underline_hide_ = kKeysUntilAutocorrectWindowHides;
   } else if (window_visible) {
@@ -95,7 +107,8 @@ void AutocorrectManager::OnSurroundingTextChanged(const base::string16& text,
     properties.visible = false;
     window_visible = false;
     button_highlighted = false;
-    engine_->SetAssistiveWindowProperties(context_id_, properties, &error);
+    suggestion_handler_->SetAssistiveWindowProperties(context_id_, properties,
+                                                      &error);
   }
 }
 
@@ -112,36 +125,38 @@ void AutocorrectManager::UndoAutocorrect() {
   window_visible = false;
   button_highlighted = false;
   window_visible = false;
-  engine_->SetAssistiveWindowProperties(context_id_, properties, &error);
+  suggestion_handler_->SetAssistiveWindowProperties(context_id_, properties,
+                                                    &error);
 
-  const gfx::Range range = engine_->GetAutocorrectRange();
+  ui::IMEInputContextHandlerInterface* input_context =
+      ui::IMEBridge::Get()->GetInputContextHandler();
+  const gfx::Range range = input_context->GetAutocorrectRange();
   const ui::SurroundingTextInfo surrounding_text =
-      ui::IMEBridge::Get()->GetInputContextHandler()->GetSurroundingTextInfo();
+      input_context->GetSurroundingTextInfo();
+
   // TODO(crbug/1111135): Can we get away with deleting less text?
   // This will not quite work properly if there is text actually highlighted,
   // and cursor is at end of the highlight block, but no easy way around it.
   // First delete everything before cursor.
-  engine_->DeleteSurroundingText(
-      context_id_, -static_cast<int>(surrounding_text.selection_range.start()),
-      surrounding_text.surrounding_text.length(), &error);
+  input_context->DeleteSurroundingText(
+      -static_cast<int>(surrounding_text.selection_range.start()),
+      surrounding_text.surrounding_text.length());
+
   // Submit the text after the cursor in composition mode to leave the cursor at
   // the start
-  engine_->SetComposition(
-      context_id_,
-      base::UTF16ToUTF8(surrounding_text.surrounding_text.substr(range.end()))
-          .c_str(),
-      /*selection_start=*/0, /*selection_end=*/0, /*cursor=*/0, /*segments=*/{},
-      &error);
-  engine_->FinishComposingText(context_id_, &error);
+  ui::CompositionText composition_text;
+  composition_text.text = surrounding_text.surrounding_text.substr(range.end());
+  input_context->UpdateCompositionText(composition_text,
+                                       /*cursor_pos=*/0, /*visible=*/true);
+  input_context->ConfirmCompositionText(/*reset_engine=*/false,
+                                        /*keep_selection=*/true);
+
   // Insert the text before the cursor - now there should be the correct text
   // and the cursor position will not have changed.
-  engine_->CommitText(
-      context_id_,
+  input_context->CommitText(
       (base::UTF16ToUTF8(
            surrounding_text.surrounding_text.substr(0, range.start())) +
-       last_typed_word_)
-          .c_str(),
-      &error);
+       last_typed_word_));
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/input_method/autocorrect_manager.h

@@ -8,20 +8,19 @@
 #include <string>
 
 #include "chrome/browser/chromeos/input_method/assistive_window_controller.h"
-#include "chrome/browser/chromeos/input_method/input_method_engine.h"
 #include "chrome/browser/chromeos/input_method/input_method_engine_base.h"
+#include "chrome/browser/chromeos/input_method/suggestion_handler_interface.h"
 
 namespace chromeos {
 
 // Implements functionality for chrome.input.ime.autocorrect() extension API.
 // This function shows UI to indicate that autocorrect has happened and allows
 // it to be undone easily.
-// TODO(b/171920749): Add unit tests.
 class AutocorrectManager {
  public:
-  // Engine is used to interact with the text field, and is assumed to be
-  // valid for the entire lifetime of the autocorrect manager.
-  explicit AutocorrectManager(InputMethodEngine* engine);
+  // `suggestion_handler_` must be alive for the duration of the lifetime of
+  // this instance.
+  explicit AutocorrectManager(SuggestionHandlerInterface* suggestion_handler);
 
   AutocorrectManager(const AutocorrectManager&) = delete;
   AutocorrectManager& operator=(const AutocorrectManager&) = delete;
@@ -32,24 +31,28 @@ class AutocorrectManager {
   void MarkAutocorrectRange(const std::string& corrected_word,
                             const std::string& typed_word,
                             int start_index);
+
   // To hide the underline after enough keypresses, this class intercepts
   // keystrokes. Returns whether the keypress has now been handled.
   bool OnKeyEvent(const InputMethodEngineBase::KeyboardEvent& event);
+
   // Indicates a new text field is focused, used to save context ID.
   void OnFocus(int context_id);
+
   // To show the undo window when cursor is in an autocorrected word, this class
   // is notified of surrounding text changes.
   void OnSurroundingTextChanged(const base::string16& text,
                                 int cursor_pos,
                                 int anchor_pos);
+
   void UndoAutocorrect();
 
  private:
   void ClearUnderline();
 
+  SuggestionHandlerInterface* suggestion_handler_;
+  int context_id_ = 0;
   int key_presses_until_underline_hide_ = 0;
-  int context_id_ = -1;
-  InputMethodEngine* const engine_;
   std::string last_typed_word_;
   std::string last_corrected_word_;
   bool window_visible = false;

chrome/browser/chromeos/input_method/autocorrect_manager_unittest.cc

+// Copyright 2020 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 "chrome/browser/chromeos/input_method/autocorrect_manager.h"
+
+#include "chrome/browser/chromeos/input_method/ui/suggestion_details.h"
+#include "chrome/grit/generated_resources.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/ime/chromeos/ime_bridge.h"
+#include "ui/base/ime/chromeos/mock_ime_input_context_handler.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace chromeos {
+namespace {
+
+using ::testing::_;
+
+InputMethodEngineBase::KeyboardEvent CreateKeyEvent(std::string key,
+                                                    std::string code) {
+  InputMethodEngineBase::KeyboardEvent event;
+  event.type = "keydown";
+  event.key = std::move(key);
+  event.code = std::move(code);
+  return event;
+}
+
+class MockSuggestionHandler : public SuggestionHandlerInterface {
+ public:
+  MOCK_METHOD(bool,
+              DismissSuggestion,
+              (int context_id, std::string* error),
+              (override));
+  MOCK_METHOD(bool,
+              SetSuggestion,
+              (int context_id,
+               const ui::ime::SuggestionDetails& details,
+               std::string* error),
+              (override));
+  MOCK_METHOD(bool,
+              AcceptSuggestion,
+              (int context_id, std::string* error),
+              (override));
+  MOCK_METHOD(void,
+              OnSuggestionsChanged,
+              (const std::vector<std::string>& suggestions),
+              (override));
+  MOCK_METHOD(bool,
+              SetButtonHighlighted,
+              (int context_id,
+               const ui::ime::AssistiveWindowButton& button,
+               bool highlighted,
+               std::string* error),
+              (override));
+  MOCK_METHOD(void,
+              ClickButton,
+              (const ui::ime::AssistiveWindowButton& button),
+              (override));
+  MOCK_METHOD(bool,
+              AcceptSuggestionCandidate,
+              (int context_id,
+               const base::string16& candidate,
+               std::string* error),
+              (override));
+  MOCK_METHOD(bool,
+              SetAssistiveWindowProperties,
+              (int context_id,
+               const AssistiveWindowProperties& assistive_window,
+               std::string* error),
+              (override));
+};
+
+TEST(AutocorrectManagerTest, MarkAutocorrectRangeSetsAutocorrectRange) {
+  ui::IMEBridge::Initialize();
+  ui::MockIMEInputContextHandler mock_ime_input_context_handler;
+  ui::IMEBridge::Get()->SetInputContextHandler(&mock_ime_input_context_handler);
+  MockSuggestionHandler mock_suggestion_handler;
+  AutocorrectManager manager(&mock_suggestion_handler);
+
+  manager.MarkAutocorrectRange("the", "teh", /*start_index=*/0);
+
+  EXPECT_EQ(mock_ime_input_context_handler.GetAutocorrectRange(),
+            gfx::Range(0, 3));
+}
+
+TEST(AutocorrectManagerTest, OnKeyEventHidesUnderlineAfterEnoughKeyPresses) {
+  ui::IMEBridge::Initialize();
+  ui::MockIMEInputContextHandler mock_ime_input_context_handler;
+  ui::IMEBridge::Get()->SetInputContextHandler(&mock_ime_input_context_handler);
+  MockSuggestionHandler mock_suggestion_handler;
+  AutocorrectManager manager(&mock_suggestion_handler);
+  manager.MarkAutocorrectRange("the", "teh", /*start_index=*/0);
+
+  const auto key_event = CreateKeyEvent("a", "KeyA");
+  EXPECT_FALSE(manager.OnKeyEvent(key_event));
+  EXPECT_FALSE(manager.OnKeyEvent(key_event));
+  EXPECT_FALSE(manager.OnKeyEvent(key_event));
+  EXPECT_FALSE(manager.OnKeyEvent(key_event));
+  EXPECT_FALSE(manager.OnKeyEvent(key_event));
+
+  EXPECT_TRUE(mock_ime_input_context_handler.GetAutocorrectRange().is_empty());
+}
+
+TEST(AutocorrectManagerTest, MovingCursorInsideRangeShowsAssistiveWindow) {
+  ui::IMEBridge::Initialize();
+  ui::MockIMEInputContextHandler mock_ime_input_context_handler;
+  ui::IMEBridge::Get()->SetInputContextHandler(&mock_ime_input_context_handler);
+  ::testing::StrictMock<MockSuggestionHandler> mock_suggestion_handler;
+  AutocorrectManager manager(&mock_suggestion_handler);
+  manager.OnSurroundingTextChanged(base::ASCIIToUTF16("the "), /*cursor_pos=*/4,
+                                   /*anchor_pos=*/4);
+  manager.MarkAutocorrectRange("the", "teh", /*start_index=*/0);
+
+  AssistiveWindowProperties properties;
+  properties.type = ui::ime::AssistiveWindowType::kUndoWindow;
+  properties.visible = true;
+  properties.announce_string = l10n_util::GetStringFUTF8(
+      IDS_SUGGESTION_AUTOCORRECT_UNDO_WINDOW_SHOWN, base::ASCIIToUTF16("teh"),
+      base::ASCIIToUTF16("the"));
+  EXPECT_CALL(mock_suggestion_handler,
+              SetAssistiveWindowProperties(_, properties, _));
+
+  manager.OnSurroundingTextChanged(base::ASCIIToUTF16("the "), /*cursor_pos=*/1,
+                                   /*anchor_pos=*/1);
+}
+
+TEST(AutocorrectManagerTest, MovingCursorOutsideRangeHidesAssistiveWindow) {
+  ui::IMEBridge::Initialize();
+  ui::MockIMEInputContextHandler mock_ime_input_context_handler;
+  ui::IMEBridge::Get()->SetInputContextHandler(&mock_ime_input_context_handler);
+  ::testing::StrictMock<MockSuggestionHandler> mock_suggestion_handler;
+  AutocorrectManager manager(&mock_suggestion_handler);
+  manager.OnSurroundingTextChanged(base::ASCIIToUTF16("the "), /*cursor_pos=*/4,
+                                   /*anchor_pos=*/4);
+  manager.MarkAutocorrectRange("the", "teh", /*start_index=*/0);
+
+  {
+    ::testing::InSequence seq;
+
+    AssistiveWindowProperties shown_properties;
+    shown_properties.type = ui::ime::AssistiveWindowType::kUndoWindow;
+    shown_properties.visible = true;
+    shown_properties.announce_string = l10n_util::GetStringFUTF8(
+        IDS_SUGGESTION_AUTOCORRECT_UNDO_WINDOW_SHOWN, base::ASCIIToUTF16("teh"),
+        base::ASCIIToUTF16("the"));
+    EXPECT_CALL(mock_suggestion_handler,
+                SetAssistiveWindowProperties(_, shown_properties, _));
+
+    AssistiveWindowProperties hidden_properties;
+    hidden_properties.type = ui::ime::AssistiveWindowType::kUndoWindow;
+    hidden_properties.visible = false;
+    EXPECT_CALL(mock_suggestion_handler,
+                SetAssistiveWindowProperties(_, hidden_properties, _));
+  }
+
+  manager.OnSurroundingTextChanged(base::ASCIIToUTF16("the "), /*cursor_pos=*/1,
+                                   /*anchor_pos=*/1);
+  manager.OnSurroundingTextChanged(base::ASCIIToUTF16("the "), /*cursor_pos=*/4,
+                                   /*anchor_pos=*/4);
+}
+
+// TODO(b/171920749): Add unit tests for UndoAutocorrect().
+
+}  // namespace
+}  // namespace chromeos

ui/base/ime/chromeos/mock_ime_input_context_handler.cc

@@ -57,7 +57,7 @@ bool MockIMEInputContextHandler::SetComposingRange(
 }
 
 gfx::Range MockIMEInputContextHandler::GetAutocorrectRange() {
-  return gfx::Range();
+  return autocorrect_range_;
 }
 
 gfx::Rect MockIMEInputContextHandler::GetAutocorrectCharacterBounds() {
@@ -68,11 +68,13 @@ bool MockIMEInputContextHandler::SetAutocorrectRange(
     const base::string16& autocorrect_text,
     uint32_t start,
     uint32_t end) {
-  // TODO(crbug.com/1091088): Implement function.
-  return false;
+  autocorrect_range_ = gfx::Range(start, end);
+  return true;
 }
 
-void MockIMEInputContextHandler::ClearAutocorrectRange() {}
+void MockIMEInputContextHandler::ClearAutocorrectRange() {
+  autocorrect_range_ = gfx::Range();
+}
 
 bool MockIMEInputContextHandler::SetSelectionRange(uint32_t start,
                                                    uint32_t end) {

ui/base/ime/chromeos/mock_ime_input_context_handler.h

@@ -100,6 +100,7 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) MockIMEInputContextHandler
   std::vector<ui::KeyEvent> sent_key_events_;
   UpdateCompositionTextArg last_update_composition_arg_;
   DeleteSurroundingTextArg last_delete_surrounding_text_arg_;
+  gfx::Range autocorrect_range_;
 };
 }  // namespace ui