Refactoring for InputMethodAuraLinux.

ui/base/ime/input_method_auralinux.cc

 // Copyright 2013 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 "ui/base/ime/input_method_auralinux.h"
 
+#include "base/auto_reset.h"
 #include "base/environment.h"
 #include "ui/base/ime/linux/linux_input_method_context_factory.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/events/event.h"
 
 namespace ui {
 
 InputMethodAuraLinux::InputMethodAuraLinux(
     internal::InputMethodDelegate* delegate)
-    : allowed_to_fire_vkey_process_key_(false), vkey_processkey_flags_(0) {
+    : text_input_type_(TEXT_INPUT_TYPE_NONE),
+      is_sync_mode_(false),
+      composition_changed_(false),
+      suppress_next_result_(false) {
   SetDelegate(delegate);
+  context_ =
+      LinuxInputMethodContextFactory::instance()->CreateInputMethodContext(
+          this, false);
+  context_simple_ =
+      LinuxInputMethodContextFactory::instance()->CreateInputMethodContext(
+          this, true);
 }
 
 InputMethodAuraLinux::~InputMethodAuraLinux() {}
 
+LinuxInputMethodContext* InputMethodAuraLinux::GetContextForTesting(
+    bool is_simple) {
+  return is_simple ? context_simple_.get() : context_.get();
+}
+
 // Overriden from InputMethod.
 
 void InputMethodAuraLinux::Init(bool focused) {
-  CHECK(LinuxInputMethodContextFactory::instance())
-      << "This failure was likely caused because "
-      << "ui::InitializeInputMethod(ForTesting) was not called "
-      << "before instantiating this class.";
-  input_method_context_ =
-      LinuxInputMethodContextFactory::instance()->CreateInputMethodContext(
-          this);
-  CHECK(input_method_context_.get());
-
   InputMethodBase::Init(focused);
 
-  if (focused) {
-    input_method_context_->OnTextInputTypeChanged(
-        GetTextInputClient() ?
-        GetTextInputClient()->GetTextInputType() :
-        TEXT_INPUT_TYPE_TEXT);
-  }
+  UpdateContextFocusState();
 }
 
 bool InputMethodAuraLinux::OnUntranslatedIMEMessage(
     const base::NativeEvent& event,
     NativeEventResult* result) {
   return false;
 }
 
 bool InputMethodAuraLinux::DispatchKeyEvent(const ui::KeyEvent& event) {
   DCHECK(event.type() == ET_KEY_PRESSED || event.type() == ET_KEY_RELEASED);
   DCHECK(system_toplevel_window_focused());
 
+  TextInputClient* client = GetTextInputClient();
   // If no text input client, do nothing.
-  if (!GetTextInputClient())
+  if (!client)
     return DispatchKeyEventPostIME(event);
 
-  // Let an IME handle the key event first, and allow to fire a VKEY_PROCESSKEY
-  // event for keydown events.  Note that DOM Level 3 Events Sepc requires that
-  // only keydown events fire keyCode=229 events and not for keyup events.
-  if (event.type() == ET_KEY_PRESSED &&
-      (event.flags() & ui::EF_IME_FABRICATED_KEY) == 0)
-    AllowToFireProcessKey(event);
-  if (input_method_context_->DispatchKeyEvent(event))
-    return true;
-  StopFiringProcessKey();
+  suppress_next_result_ = false;
+  composition_changed_ = false;
+  result_text_.clear();

[James Su, 2015/04/10 09:25:16]

clear composition_ here.

[Shu Chen, 2015/04/10 16:17:38]

should not do this per offline discussion.

 
-  // Otherwise, insert the character.
-  const bool handled = DispatchKeyEventPostIME(event);
-  if (event.type() == ET_KEY_PRESSED && GetTextInputClient()) {
-    const uint16 ch = event.GetCharacter();
-    if (ch) {
-      GetTextInputClient()->InsertChar(ch, event.flags());
-      return true;
+  bool filtered = false;
+  {
+    base::AutoReset<bool> flipper(&is_sync_mode_, true);
+    if (text_input_type_ != TEXT_INPUT_TYPE_NONE &&
+        text_input_type_ != TEXT_INPUT_TYPE_PASSWORD) {
+      filtered = context_->DispatchKeyEvent(event);
+    } else {
+      filtered = context_simple_->DispatchKeyEvent(event);
     }
   }
-  return handled;
+
+  if (event.type() == ui::ET_KEY_PRESSED && filtered) {
+    if (NeedInsertChar())
+      DispatchKeyEventPostIME(event);
+    else if (HasInputMethodResult())
+      SendFakeProcessKeyEvent(event.flags());
+
+    // Don't send VKEY_PROCESSKEY event if there is no result text or
+    // composition. This is to workaround the wierd behavior of IBus with US

[James Su, 2015/04/10 09:25:16]

wierd -> weird

[Shu Chen, 2015/04/10 16:17:38]

Done.

+    // keyboard, which mutes the keydown and later fake a new keydown with IME
+    // result in sync mode. In that case, user would expect only
+    // keydown/keypress/keyup event without an initial 229 keydown event.
+  }
+
+  // Processes the result text before composition for sync mode.
+  if (!result_text_.empty()) {
+    if (filtered && NeedInsertChar()) {
+      for (const auto ch : result_text_)
+        client->InsertChar(ch, event.flags());
+    } else {
+      client->InsertText(result_text_);

[James Su, 2015/04/10 09:25:17]

add comment to explain the logic of this block. Especially why we cannot insert
char here, if the key event is not filtered.

And I'm not sure if your test covers this case.

[Shu Chen, 2015/04/10 16:17:38]

Done.

+    }
+  }
+
+  if (composition_changed_ && !IsTextInputTypeNone()) {
+    if (!composition_.text.empty())
+      client->SetCompositionText(composition_);
+    else if (result_text_.empty())
+      client->ClearCompositionText();

[James Su, 2015/04/10 09:25:17]

add comment to explain this part of logic. Especially why no need to clear
composition if result text is not empty.

[Shu Chen, 2015/04/10 16:17:38]

Done.

+  }
+
+  if (!result_text_.empty() && !composition_.text.empty())
+    composition_.Clear();

[James Su, 2015/04/10 09:25:17]

I don't understand this if block. I don't think this if check is necessary.

I think you can simply call composition_.Clear() here or at the beginning of
DispatchKeyEvent() method.

[Shu Chen, 2015/04/10 16:17:38]

Comment is added.

+

[James Su, 2015/04/10 09:25:16]

I'm wondering if it's more clear to return true here if filtered is true. So
that we don't need to check !filtered in below if else if blocks.

[Shu Chen, 2015/04/10 16:17:39]

I would prefer to avoid returning in the middle of a long method.

+  if (event.type() == ui::ET_KEY_PRESSED && !filtered) {
+    DispatchKeyEventPostIME(event);
+
+    // If a key event was not filtered by |context_| or |context_simple_|, then
+    // it means the key event didn't generate any result text. For some cases,
+    // the key event may still generate a valid character, eg. a control-key
+    // event (ctrl-a, return, tab, etc.). We need to send the character to the
+    // focused text input client by calling TextInputClient::InsertChar().
+    base::char16 ch = event.GetCharacter();
+    if (ch && client)
+      client->InsertChar(ch, event.flags());
+  } else if (event.type() == ui::ET_KEY_RELEASED && !filtered) {
+    DispatchKeyEventPostIME(event);
+  }
+
+  return true;
+}
+
+void InputMethodAuraLinux::UpdateContextFocusState() {
+  bool old_text_input_type = text_input_type_;
+  text_input_type_ = GetTextInputType();
+
+  // We only focus in |context_| when the focus is in a textfield.
+  if (old_text_input_type != TEXT_INPUT_TYPE_NONE &&
+      text_input_type_ == TEXT_INPUT_TYPE_NONE) {
+    context_->Blur();
+  } else if (old_text_input_type == TEXT_INPUT_TYPE_NONE &&
+             text_input_type_ != TEXT_INPUT_TYPE_NONE) {
+    context_->Focus();
+  }
+
+  // |context_simple_| can be used in any textfield, including password box, and
+  // even if the focused text input client's text input type is
+  // ui::TEXT_INPUT_TYPE_NONE.
+  if (GetTextInputClient())
+    context_simple_->Focus();
+  else
+    context_simple_->Blur();
 }
 
 void InputMethodAuraLinux::OnTextInputTypeChanged(
     const TextInputClient* client) {
-  if (!IsTextInputClientFocused(client))
-    return;
-  input_method_context_->Reset();
+  UpdateContextFocusState();
   // TODO(yoichio): Support inputmode HTML attribute.
-  input_method_context_->OnTextInputTypeChanged(client->GetTextInputType());
 }
 
 void InputMethodAuraLinux::OnCaretBoundsChanged(const TextInputClient* client) {
   if (!IsTextInputClientFocused(client))
     return;
-  input_method_context_->OnCaretBoundsChanged(
-      GetTextInputClient()->GetCaretBounds());
+  context_->SetCursorLocation(GetTextInputClient()->GetCaretBounds());
 }
 
 void InputMethodAuraLinux::CancelComposition(const TextInputClient* client) {
   if (!IsTextInputClientFocused(client))
     return;
-  input_method_context_->Reset();
-  input_method_context_->OnTextInputTypeChanged(client->GetTextInputType());
+  ResetContext();
+}
+
+void InputMethodAuraLinux::ResetContext() {
+  if (!GetTextInputClient())
+    return;
+
+  // To prevent any text from being committed when resetting the |context_|;
+  is_sync_mode_ = true;
+  suppress_next_result_ = true;
+
+  context_->Reset();
+  context_simple_->Reset();
+
+  // Some input methods may not honour the reset call. Focusing out/in the
+  // |context_| to make sure it gets reset correctly.
+  if (text_input_type_ != TEXT_INPUT_TYPE_NONE) {
+    context_->Blur();
+    context_->Focus();
+  }
+
+  composition_.Clear();
+  result_text_.clear();
+  is_sync_mode_ = false;
+  composition_changed_ = false;
 }
 
 void InputMethodAuraLinux::OnInputLocaleChanged() {
 }
 
 std::string InputMethodAuraLinux::GetInputLocale() {
   return "";
 }
 
 bool InputMethodAuraLinux::IsActive() {
   // InputMethodAuraLinux is always ready and up.
   return true;
 }
 
 bool InputMethodAuraLinux::IsCandidatePopupOpen() const {
   // There seems no way to detect candidate windows or any popups.
   return false;
 }
 
 // Overriden from ui::LinuxInputMethodContextDelegate
 
 void InputMethodAuraLinux::OnCommit(const base::string16& text) {
-  MaybeFireProcessKey();
-  if (!IsTextInputTypeNone())
+  if (suppress_next_result_ || !GetTextInputClient()) {

[James Su, 2015/04/10 09:25:16]

Why not check IsTextInputTypeNone() instead?

[Shu Chen, 2015/04/10 16:17:38]

When IsTextInputTypeNone() returns true, here cannot just return, because it
needs to append the result text for later InsertChar.

+    suppress_next_result_ = false;
+    return;
+  }
+
+  if (is_sync_mode_) {
+    // Append the text to the buffer, because commit signal might be fired
+    // multiple times when processing a key event.
+    result_text_.append(text);
+  } else if (!IsTextInputTypeNone()) {
+    // If we are not handling key event, do not bother sending text result if
+    // the focused text input client does not support text input.
+    SendFakeProcessKeyEvent(0);
     GetTextInputClient()->InsertText(text);
+    composition_.Clear();
+  }
 }
 
 void InputMethodAuraLinux::OnPreeditChanged(
     const CompositionText& composition_text) {
-  MaybeFireProcessKey();
-  TextInputClient* text_input_client = GetTextInputClient();
-  if (text_input_client)
-    text_input_client->SetCompositionText(composition_text);
+  if (suppress_next_result_ || IsTextInputTypeNone())
+    return;
+
+  composition_ = composition_text;

[James Su, 2015/04/10 09:25:16]

move this line into if (is_sync_mode_) block.

[Shu Chen, 2015/04/10 16:17:38]

Invalid comment per our offline discussion.

+
+  if (is_sync_mode_) {
+    if (!composition_.text.empty() || !composition_text.text.empty())
+      composition_changed_ = true;
+  } else {
+    SendFakeProcessKeyEvent(0);
+    GetTextInputClient()->SetCompositionText(composition_text);
+  }
 }
 
 void InputMethodAuraLinux::OnPreeditEnd() {
-  MaybeFireProcessKey();
-  TextInputClient* text_input_client = GetTextInputClient();
-  if (text_input_client && text_input_client->HasCompositionText())
-    text_input_client->ClearCompositionText();
-}
+  if (suppress_next_result_ || IsTextInputTypeNone())
+    return;
 
-void InputMethodAuraLinux::OnPreeditStart() {
-  MaybeFireProcessKey();
+  if (is_sync_mode_) {
+    if (!composition_.text.empty()) {
+      composition_.Clear();
+      composition_changed_ = true;
+    }
+  } else {
+    TextInputClient* client = GetTextInputClient();
+    if (client && client->HasCompositionText())
+      client->ClearCompositionText();
+    composition_.Clear();

[James Su, 2015/04/10 09:25:16]

remove this line.

[Shu Chen, 2015/04/10 16:17:38]

Invalid comment per our offline discussion.

+  }
 }
 
 // Overridden from InputMethodBase.
 
+void InputMethodAuraLinux::OnFocus() {
+  InputMethodBase::OnFocus();
+  UpdateContextFocusState();
+}
+
+void InputMethodAuraLinux::OnBlur() {
+  ConfirmCompositionText();
+  InputMethodBase::OnBlur();
+  UpdateContextFocusState();
+}
+
+void InputMethodAuraLinux::OnWillChangeFocusedClient(
+    TextInputClient* focused_before,
+    TextInputClient* focused) {
+  ConfirmCompositionText();
+}
+
 void InputMethodAuraLinux::OnDidChangeFocusedClient(
     TextInputClient* focused_before,
     TextInputClient* focused) {
-  input_method_context_->Reset();
-  input_method_context_->OnTextInputTypeChanged(
-      focused ? focused->GetTextInputType() : TEXT_INPUT_TYPE_NONE);
+  UpdateContextFocusState();
+
+  // Force to update caret bounds, in case the View thinks that the caret
+  // bounds has not changed.
+  if (text_input_type_ != TEXT_INPUT_TYPE_NONE)
+    OnCaretBoundsChanged(GetTextInputClient());
 
   InputMethodBase::OnDidChangeFocusedClient(focused_before, focused);
 }
 
-// Helper functions to support VKEY_PROCESSKEY.
+// private
 
-void InputMethodAuraLinux::AllowToFireProcessKey(const ui::KeyEvent& event) {
-  allowed_to_fire_vkey_process_key_ = true;
-  vkey_processkey_flags_ = event.flags();
+bool InputMethodAuraLinux::HasInputMethodResult() {
+  return !result_text_.empty() || composition_changed_;
 }
 
-void InputMethodAuraLinux::MaybeFireProcessKey() {
-  if (!allowed_to_fire_vkey_process_key_)
-    return;
-
-  const ui::KeyEvent fabricated_event(ET_KEY_PRESSED,
-                                      VKEY_PROCESSKEY,
-                                      vkey_processkey_flags_);
-  DispatchKeyEventPostIME(fabricated_event);
-  StopFiringProcessKey();
+bool InputMethodAuraLinux::NeedInsertChar() const {
+  return IsTextInputTypeNone() ||
+         (!composition_changed_ && composition_.text.empty() &&
+          result_text_.length() == 1);
 }
 
-void InputMethodAuraLinux::StopFiringProcessKey() {
-  allowed_to_fire_vkey_process_key_ = false;
-  vkey_processkey_flags_ = 0;
+void InputMethodAuraLinux::SendFakeProcessKeyEvent(int flags) const {
+  DispatchKeyEventPostIME(
+      KeyEvent(ui::ET_KEY_PRESSED, ui::VKEY_PROCESSKEY, flags));
+}
+
+void InputMethodAuraLinux::ConfirmCompositionText() {
+  TextInputClient* client = GetTextInputClient();
+  if (client && client->HasCompositionText())
+    client->ConfirmCompositionText();
+
+  ResetContext();
 }
 
 }  // namespace ui