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/+/2576067Reviewed-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}

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

@@ -189,7 +189,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::ProcessInputMethodResult(ui::KeyEvent* event,
         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,20 +626,24 @@ void InputMethodChromeOS::ProcessInputMethodResult(ui::KeyEvent* event,
   // 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));
 }
 
 bool InputMethodChromeOS::HasInputMethodResult() const {
   return result_text_.length() || composition_changed_;
 }
 
-void InputMethodChromeOS::CommitText(const std::string& text) {
+void InputMethodChromeOS::CommitText(
+    const std::string& text,
+    TextInputClient::InsertTextCursorBehavior cursor_behavior) {
   if (text.empty())
     return;
 
@@ -646,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.
@@ -657,6 +679,7 @@ void InputMethodChromeOS::CommitText(const std::string& text) {
           TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
     SendFakeProcessKeyEvent(false);
     result_text_.clear();
+    result_text_cursor_ = 0;
   }
 }
 
@@ -767,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_);
 
@@ -1182,7 +1187,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);
@@ -1191,4 +1197,50 @@ TEST_F(InputMethodChromeOSKeyEventTest,
   EXPECT_EQ(gfx::Range(0, 1), GetAutocorrectRange());
 }
 
+TEST_F(InputMethodChromeOSKeyEventTest,
+       MultipleCommitTextsWhileHandlingKeyEventCoalescesIntoOne) {
+  InputMethodChromeOS ime(this);
+  FakeTextInputClient fake_text_input_client(TEXT_INPUT_TYPE_TEXT);
+  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) {
+  InputMethodChromeOS ime(this);
+  FakeTextInputClient fake_text_input_client(TEXT_INPUT_TYPE_TEXT);
+  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