Should clear composition text when OnCommitText() is called.

According to the reference doc of InputConnection.commitText(),
we should clear the current composition text when commitText() is called
instead of commiting the current composition text.

Bug: 845079
Test: Typing text wiht Google Japanse Input app and it works well.
Change-Id: Ie2d9cd73d64f5728dc82f3f441d73af7a2740836
Reviewed-on: https://chromium-review.googlesource.com/1252226
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595629}

chrome/browser/chromeos/arc/input_method_manager/arc_input_method_manager_service_unittest.cc

@@ -748,8 +748,7 @@ TEST_F(ArcInputMethodManagerServiceTest, IMEOperations) {
   InputConnectionImpl* connection = service()->GetInputConnectionForTesting();
   ASSERT_NE(nullptr, connection);
   connection->CommitText(base::ASCIIToUTF16("text"), 0);
-  // It's called from both of FinishComposingText() and CommitText().
-  EXPECT_EQ(2, test_context_handler.commit_text_call_count());
+  EXPECT_EQ(1, test_context_handler.commit_text_call_count());
   // Trigger an observer method to trigger text input state updating.
   engine_handler->SetSurroundingText("", 0, 0, 0);
   EXPECT_EQ(1, bridge()->update_text_input_state_calls_count_);
@@ -763,9 +762,20 @@ TEST_F(ArcInputMethodManagerServiceTest, IMEOperations) {
   connection->FinishComposingText();
   EXPECT_EQ(1, test_context_handler.commit_text_call_count());
 
+  base::string16 text = base::ASCIIToUTF16("text");
   test_context_handler.Reset();
-  connection->SetComposingText(base::ASCIIToUTF16("text"), 0);
+  connection->SetComposingText(text, 0);
   EXPECT_EQ(1, test_context_handler.update_preedit_text_call_count());
+  EXPECT_EQ(
+      text,
+      test_context_handler.last_update_composition_arg().composition_text.text);
+  // Committing the composing text calls ClearComposition() and CommitText().
+  connection->CommitText(base::ASCIIToUTF16("text"), 0);
+  EXPECT_EQ(2, test_context_handler.update_preedit_text_call_count());
+  EXPECT_EQ(
+      base::ASCIIToUTF16(""),
+      test_context_handler.last_update_composition_arg().composition_text.text);
+  EXPECT_EQ(1, test_context_handler.commit_text_call_count());
 
   engine_handler->FocusOut();
 

chrome/browser/chromeos/arc/input_method_manager/input_connection_impl.cc

@@ -76,10 +76,10 @@ void InputConnectionImpl::CommitText(const base::string16& text,
                                      int new_cursor_pos) {
   StartStateUpdateTimer();
 
-  // Confirm the current composing text at first.
-  FinishComposingTextInternal();
-
   std::string error;
+  // Clear the current composing text at first.
+  if (!ime_engine_->ClearComposition(input_context_id_, &error))
+    LOG(ERROR) << "ClearComposition failed: error=\"" << error << "\"";
   if (!ime_engine_->CommitText(input_context_id_,
                                base::UTF16ToUTF8(text).c_str(), &error))
     LOG(ERROR) << "CommitText failed: error=\"" << error << "\"";
@@ -103,7 +103,15 @@ void InputConnectionImpl::DeleteSurroundingText(int before, int after) {
 
 void InputConnectionImpl::FinishComposingText() {
   StartStateUpdateTimer();
-  FinishComposingTextInternal();
+
+  std::string error;
+  if (!ime_engine_->CommitText(input_context_id_,
+                               base::UTF16ToUTF8(composing_text_).c_str(),
+                               &error)) {
+    LOG(ERROR) << "FinishComposingText: CommitText() failed, error=\"" << error
+               << "\"";
+  }
+  composing_text_.clear();
 }
 
 void InputConnectionImpl::SetComposingText(const base::string16& text,
@@ -142,15 +150,4 @@ void InputConnectionImpl::StartStateUpdateTimer() {
                      true /* is_input_state_update_requested */));
 }
 
-void InputConnectionImpl::FinishComposingTextInternal() {
-  std::string error;
-  if (!ime_engine_->CommitText(input_context_id_,
-                               base::UTF16ToUTF8(composing_text_).c_str(),
-                               &error)) {
-    LOG(ERROR) << "FinishComposingText: CommitText() failed, error=\"" << error
-               << "\"";
-  }
-  composing_text_.clear();
-}
-
 }  // namespace arc

chrome/browser/chromeos/arc/input_method_manager/input_connection_impl.h

@@ -49,7 +49,6 @@ class InputConnectionImpl : public mojom::InputConnection {
   // update surely. Some implementations of TextInputClient are synchronous. If
   // starting timer is after API call, the timer won't be cancelled.
   void StartStateUpdateTimer();
-  void FinishComposingTextInternal();
 
   chromeos::InputMethodEngine* const ime_engine_;  // Not owned
   ArcInputMethodManagerBridge* const imm_bridge_;  // Not owned