Reland "ime: Handle InsertTextCursorBehavior in InputMethodChromeOS."

This is a reland of 29ff7965

Reordered initialization to fix ASAN/MSAN bug.

Original change's description:
> ime: Handle InsertTextCursorBehavior in InputMethodChromeOS.
>
> TextInputClients now support moving the cursor after committing some
> text. This patch adds support for input engines to call the new API.
>
> There is a slight complication with multiple CommitText calls during
> key event handling. Previously, when handling a key event, any calls
> to CommitText will be coalesced together and executed once the key
> event is handled. However, this coalescing logic doesn't handle
> cursor movement.
>
> Add logic to handle cursor movement when coalescing commits by observing that
> any sequence of CommitTexts with cursor movement can be implemented with two
> separate CommitTexts. Note that this is a no-op change if the IME never uses
> the new cursor movement parameter.
>
> Bug: 1155331
> Change-Id: I6c89ec1cde5624eca7bf0d5c803085f7d7803c71
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576067
> Reviewed-by: Keith Lee <keithlee@chromium.org>
> Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
> Commit-Queue: Darren Shen <shend@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#840082}

Bug: 1155331
Change-Id: I373a8195946ec4fb966f932eaf084b2b74f06b6c
Cq-Include-Trybots: luci.chromium.try:linux_chromium_chromeos_msan_rel_ng,linux_chromium_chromeos_asan_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612604Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Reviewed-by: John Palmer <jopalmer@chromium.org>
Reviewed-by: Keith Lee <keithlee@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842595}

chrome/browser/chromeos/accessibility/dictation_chromeos.cc

@@ -138,7 +138,9 @@ void DictationChromeos::DictationOff() {
 
     ui::IMEInputContextHandlerInterface* input_context = GetInputContext();
     if (input_context)
-      input_context->CommitText(base::UTF16ToUTF8(composition_->text));
+      input_context->CommitText(
+          base::UTF16ToUTF8(composition_->text),
+          ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
 
     composition_->text = base::string16();
   } else {

chrome/browser/chromeos/input_method/autocorrect_manager.cc

@@ -214,7 +214,8 @@ void AutocorrectManager::UndoAutocorrect() {
   input_context->CommitText(
       (base::UTF16ToUTF8(
            surrounding_text.surrounding_text.substr(0, range.start())) +
-       original_text_));
+       original_text_),
+      ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
   LogAssistiveAutocorrectAction(AutocorrectActions::kReverted);
   RecordAssistiveCoverage(AssistiveType::kAutocorrectReverted);
   RecordAssistiveSuccess(AssistiveType::kAutocorrectReverted);

chrome/browser/chromeos/input_method/input_method_engine.cc

@@ -502,7 +502,9 @@ void InputMethodEngine::CommitTextToInputContext(int context_id,
     return;
 
   const bool had_composition_text = input_context->HasCompositionText();
-  input_context->CommitText(text);
+  input_context->CommitText(
+      text,
+      ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
 
   if (had_composition_text) {
     // Records histograms for committed characters with composition text.

chrome/browser/chromeos/input_method/native_input_method_engine.cc

@@ -412,7 +412,9 @@ void NativeInputMethodEngine::ImeObserver::OnInputMethodOptionsChanged(
 }
 
 void NativeInputMethodEngine::ImeObserver::CommitText(const std::string& text) {
-  GetInputContext()->CommitText(NormalizeString(text));
+  GetInputContext()->CommitText(
+      NormalizeString(text),
+      ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
 }
 
 void NativeInputMethodEngine::ImeObserver::SetComposition(
@@ -491,7 +493,10 @@ void NativeInputMethodEngine::ImeObserver::OnRuleBasedKeyEventResponse(
   for (const auto& op : response->operations) {
     switch (op->method) {
       case ime::mojom::OperationMethodForRulebased::COMMIT_TEXT:
-        GetInputContext()->CommitText(NormalizeString(op->arguments));
+        GetInputContext()->CommitText(
+            NormalizeString(op->arguments),
+            ui::TextInputClient::InsertTextCursorBehavior::
+                kMoveCursorAfterText);
         break;
       case ime::mojom::OperationMethodForRulebased::SET_COMPOSITION:
         ui::CompositionText composition;

ui/base/ime/chromeos/ime_input_context_handler_interface.h

@@ -11,6 +11,7 @@
 #include "base/component_export.h"
 #include "ui/base/ime/composition_text.h"
 #include "ui/base/ime/input_method.h"
+#include "ui/base/ime/text_input_client.h"
 #include "ui/events/event.h"
 
 namespace ui {
@@ -23,7 +24,9 @@ struct SurroundingTextInfo {
 class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) IMEInputContextHandlerInterface {
  public:
   // Called when the engine commit a text.
-  virtual void CommitText(const std::string& text) = 0;
+  virtual void CommitText(
+      const std::string& text,
+      TextInputClient::InsertTextCursorBehavior cursor_behavior) = 0;
 
   // Called when the engine changes the composition range.
   // Returns true if the operation was successful.

ui/base/ime/chromeos/input_method_chromeos.cc

@@ -442,6 +442,7 @@ void InputMethodChromeOS::ResetContext(bool reset_engine) {
 
   pending_composition_ = base::nullopt;
   result_text_.clear();
+  result_text_cursor_ = 0;
   composing_text_ = false;
   composition_changed_ = false;
 
@@ -578,9 +579,22 @@ void InputMethodChromeOS::MaybeProcessPendingInputMethodResult(
         client->InsertChar(ch_event);
       }
     } else {
-      client->InsertText(
-          result_text_,
-          TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
+      // Split |result_text_| into two separate commits, one for the substring
+      // before |result_text_cursor_| and one for the substring after.
+      const base::string16 before_cursor =
+          result_text_.substr(0, result_text_cursor_);
+      if (!before_cursor.empty()) {
+        client->InsertText(
+            before_cursor,
+            TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
+      }
+      const base::string16 after_cursor =
+          result_text_.substr(result_text_cursor_);
+      if (!after_cursor.empty()) {
+        client->InsertText(
+            after_cursor,
+            TextInputClient::InsertTextCursorBehavior::kMoveCursorBeforeText);
+      }
       composing_text_ = false;
     }
   }
@@ -612,16 +626,24 @@ void InputMethodChromeOS::MaybeProcessPendingInputMethodResult(
   // We should not clear composition text here, as it may belong to the next
   // composition session.
   result_text_.clear();
+  result_text_cursor_ = 0;
   composition_changed_ = false;
 }
 
 bool InputMethodChromeOS::NeedInsertChar() const {
   return GetTextInputClient() &&
-      (IsTextInputTypeNone() ||
-       (!composing_text_ && result_text_.length() == 1));
+         (IsTextInputTypeNone() ||
+          (!composing_text_ && result_text_.length() == 1 &&
+           result_text_cursor_ == 1));
 }
 
-void InputMethodChromeOS::CommitText(const std::string& text) {
+bool InputMethodChromeOS::HasInputMethodResult() const {
+  return result_text_.length() || composition_changed_;
+}
+
+void InputMethodChromeOS::CommitText(
+    const std::string& text,
+    TextInputClient::InsertTextCursorBehavior cursor_behavior) {
   if (text.empty())
     return;
 
@@ -642,7 +664,11 @@ void InputMethodChromeOS::CommitText(const std::string& text) {
 
   // Append the text to the buffer, because commit signal might be fired
   // multiple times when processing a key event.
-  result_text_.append(utf16_text);
+  result_text_.insert(result_text_cursor_, utf16_text);
+  if (cursor_behavior ==
+      TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText) {
+    result_text_cursor_ += utf16_text.length();
+  }
 
   // If we are not handling key event, do not bother sending text result if the
   // focused text input client does not support text input.
@@ -653,6 +679,7 @@ void InputMethodChromeOS::CommitText(const std::string& text) {
           TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
     SendFakeProcessKeyEvent(false);
     result_text_.clear();
+    result_text_cursor_ = 0;
   }
 }
 
@@ -763,7 +790,8 @@ bool InputMethodChromeOS::ExecuteCharacterComposer(const ui::KeyEvent& event) {
   std::string commit_text =
       base::UTF16ToUTF8(character_composer_.composed_character());
   if (!commit_text.empty()) {
-    CommitText(commit_text);
+    CommitText(commit_text,
+               TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
   }
   return true;
 }

ui/base/ime/chromeos/input_method_chromeos.h

@@ -53,7 +53,9 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) InputMethodChromeOS
                                 TextInputClient* focused) override;
 
   // ui::IMEInputContextHandlerInterface overrides:
-  void CommitText(const std::string& text) override;
+  void CommitText(
+      const std::string& text,
+      TextInputClient::InsertTextCursorBehavior cursor_behavior) override;
   bool SetCompositionRange(
       uint32_t before,
       uint32_t after,
@@ -158,6 +160,10 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) InputMethodChromeOS
   // processing result of the pending key event.
   base::string16 result_text_;
 
+  // Where the cursor should be place after inserting |result_text_|.
+  // 0 <= |result_text_cursor_| <= |result_text_.length()|.
+  size_t result_text_cursor_ = 0;
+
   base::string16 previous_surrounding_text_;
   gfx::Range previous_selection_range_;
 

ui/base/ime/chromeos/input_method_chromeos_unittest.cc

@@ -89,8 +89,10 @@ class TestableInputMethodChromeOS : public InputMethodChromeOS {
     }
     return details;
   }
-  void CommitText(const std::string& text) override {
-    InputMethodChromeOS::CommitText(text);
+  void CommitText(
+      const std::string& text,
+      TextInputClient::InsertTextCursorBehavior cursor_behavior) override {
+    InputMethodChromeOS::CommitText(text, cursor_behavior);
     text_committed_ = text;
   }
 
@@ -447,7 +449,8 @@ TEST_F(InputMethodChromeOSTest,
        OnWillChangeFocusedClientClearAutocorrectRange) {
   input_type_ = TEXT_INPUT_TYPE_TEXT;
   ime_->SetFocusedTextInputClient(this);
-  ime_->CommitText("hello");
+  ime_->CommitText(
+      "hello", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
   ime_->SetAutocorrectRange(gfx::Range(0, 5));
   EXPECT_EQ(gfx::Range(0, 5), this->GetAutocorrectRange());
 
@@ -1018,7 +1021,9 @@ TEST_F(InputMethodChromeOSKeyEventTest, KeyEventDelayResponseTest) {
   EXPECT_EQ(kFlags, key_event->flags());
   EXPECT_EQ(0, ime_->process_key_event_post_ime_call_count());
 
-  static_cast<IMEInputContextHandlerInterface*>(ime_.get())->CommitText("A");
+  static_cast<IMEInputContextHandlerInterface*>(ime_.get())
+      ->CommitText(
+          "A", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
 
   EXPECT_EQ(0, inserted_char_);
 
@@ -1178,7 +1183,8 @@ TEST_F(InputMethodChromeOSKeyEventTest, JP106KeyTest) {
 TEST_F(InputMethodChromeOSKeyEventTest, SetAutocorrectRangeRunsAfterKeyEvent) {
   input_type_ = TEXT_INPUT_TYPE_TEXT;
   ime_->OnTextInputTypeChanged(this);
-  ime_->CommitText("a");
+  ime_->CommitText(
+      "a", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
 
   ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
   ime_->DispatchKeyEvent(&event);
@@ -1196,7 +1202,8 @@ TEST_F(InputMethodChromeOSKeyEventTest,
   ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
   ime_->DispatchKeyEvent(&event);
 
-  ime_->CommitText("a");
+  ime_->CommitText(
+      "a", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
   ime_->SetAutocorrectRange(gfx::Range(0, 1));
   std::move(mock_ime_engine_handler_->last_passed_callback())
       .Run(/*handled=*/true);
@@ -1205,4 +1212,50 @@ TEST_F(InputMethodChromeOSKeyEventTest,
   EXPECT_EQ(gfx::Range(0, 1), GetAutocorrectRange());
 }
 
+TEST_F(InputMethodChromeOSKeyEventTest,
+       MultipleCommitTextsWhileHandlingKeyEventCoalescesIntoOne) {
+  FakeTextInputClient fake_text_input_client(TEXT_INPUT_TYPE_TEXT);
+  InputMethodChromeOS ime(this);
+  ime.SetFocusedTextInputClient(&fake_text_input_client);
+
+  ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
+  ime.DispatchKeyEvent(&event);
+  ime.CommitText(
+      "a", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
+  ime.CommitText(
+      "b", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
+  ime.CommitText(
+      "cde", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
+  std::move(mock_ime_engine_handler_->last_passed_callback())
+      .Run(/*handled=*/true);
+
+  EXPECT_EQ(fake_text_input_client.text(), base::ASCIIToUTF16("abcde"));
+  EXPECT_EQ(fake_text_input_client.selection(), gfx::Range(5, 5));
+}
+
+TEST_F(InputMethodChromeOSKeyEventTest,
+       MultipleCommitTextsWhileHandlingKeyEventCoalescesByCaretBehavior) {
+  FakeTextInputClient fake_text_input_client(TEXT_INPUT_TYPE_TEXT);
+  InputMethodChromeOS ime(this);
+  ime.SetFocusedTextInputClient(&fake_text_input_client);
+
+  ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
+  ime.DispatchKeyEvent(&event);
+  ime.CommitText(
+      "a", TextInputClient::InsertTextCursorBehavior::kMoveCursorBeforeText);
+  ime.CommitText(
+      "b", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
+  ime.CommitText(
+      "c", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
+  ime.CommitText(
+      "d", TextInputClient::InsertTextCursorBehavior::kMoveCursorBeforeText);
+  ime.CommitText(
+      "e", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
+  std::move(mock_ime_engine_handler_->last_passed_callback())
+      .Run(/*handled=*/true);
+
+  EXPECT_EQ(fake_text_input_client.text(), base::ASCIIToUTF16("bceda"));
+  EXPECT_EQ(fake_text_input_client.selection(), gfx::Range(3, 3));
+}
+
 }  // namespace ui

ui/base/ime/chromeos/mock_ime_input_context_handler.cc

@@ -21,7 +21,9 @@ MockIMEInputContextHandler::MockIMEInputContextHandler()
 
 MockIMEInputContextHandler::~MockIMEInputContextHandler() = default;
 
-void MockIMEInputContextHandler::CommitText(const std::string& text) {
+void MockIMEInputContextHandler::CommitText(
+    const std::string& text,
+    TextInputClient::InsertTextCursorBehavior cursor_behavior) {
   ++commit_text_call_count_;
   last_commit_text_ = text;
 }
@@ -115,7 +117,8 @@ void MockIMEInputContextHandler::ConfirmCompositionText(bool reset_engine,
     return;
 
   CommitText(
-      base::UTF16ToUTF8(last_update_composition_arg_.composition_text.text));
+      base::UTF16ToUTF8(last_update_composition_arg_.composition_text.text),
+      TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
   last_update_composition_arg_.composition_text.text = base::string16();
 }
 

ui/base/ime/chromeos/mock_ime_input_context_handler.h

@@ -33,7 +33,9 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) MockIMEInputContextHandler
   MockIMEInputContextHandler();
   virtual ~MockIMEInputContextHandler();
 
-  void CommitText(const std::string& text) override;
+  void CommitText(
+      const std::string& text,
+      TextInputClient::InsertTextCursorBehavior cursor_behavior) override;
   void UpdateCompositionText(const CompositionText& text,
                              uint32_t cursor_pos,
                              bool visible) override;

ui/base/ime/fake_text_input_client.cc

@@ -30,7 +30,15 @@ void FakeTextInputClient::ClearCompositionText() {}
 
 void FakeTextInputClient::InsertText(
     const base::string16& text,
-    TextInputClient::InsertTextCursorBehavior cursor_behavior) {}
+    TextInputClient::InsertTextCursorBehavior cursor_behavior) {
+  text_.insert(selection_.start(), text);
+
+  if (cursor_behavior ==
+      TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText) {
+    selection_ = gfx::Range(selection_.start() + text.length(),
+                            selection_.end() + text.length());
+  }
+}
 
 void FakeTextInputClient::InsertChar(const KeyEvent& event) {}
 

ui/base/ime/fake_text_input_client.h

@@ -25,6 +25,8 @@ class FakeTextInputClient : public TextInputClient {
   ~FakeTextInputClient() override;
 
   void set_text_input_type(TextInputType text_input_type);
+  const base::string16& text() const { return text_; }
+  const gfx::Range& selection() const { return selection_; }
 
   // TextInputClient:
   void SetCompositionText(const CompositionText& composition) override;
@@ -82,6 +84,8 @@ class FakeTextInputClient : public TextInputClient {
 
  private:
   TextInputType text_input_type_;
+  base::string16 text_;
+  gfx::Range selection_;
 };
 
 }  // namespace ui