Refactoring for InputMethodAuraLinux.

ui/base/ime/input_method_auralinux.cc

Index: ui/base/ime/input_method_auralinux.cc
diff --git a/ui/base/ime/input_method_auralinux.cc b/ui/base/ime/input_method_auralinux.cc
index f5d4bee59a4cf01377bcb2b16920c94b233ab7aa..7592ae61750ede965e62bd93c12a2fa818e74ede 100644
--- a/ui/base/ime/input_method_auralinux.cc
+++ b/ui/base/ime/input_method_auralinux.cc
@@ -11,34 +11,43 @@
 
 namespace ui {
 
+namespace {
+
+class ScopedBooleanFlipper {

[yukawa, 2015/04/08 22:26:48]

Can we use base::AutoReset in //src/base/auto_reset.h ?

[Shu Chen, 2015/04/09 01:16:32]

Done.

+ public:
+  ScopedBooleanFlipper(bool* v) : v_(v) { DCHECK(!*v_); *v_ = true; }
+  ~ScopedBooleanFlipper() { *v_ = false; }
+ private:
+  bool* v_;
+};
+
+}  // namespace
+
 InputMethodAuraLinux::InputMethodAuraLinux(
     internal::InputMethodDelegate* delegate)
-    : allowed_to_fire_vkey_process_key_(false), vkey_processkey_flags_(0) {
+    : context_focused_(false), handling_key_event_(false),
+      composing_text_(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(
@@ -50,54 +59,128 @@ bool InputMethodAuraLinux::OnUntranslatedIMEMessage(
 bool InputMethodAuraLinux::DispatchKeyEvent(const ui::KeyEvent& event) {
   DCHECK(event.type() == ET_KEY_PRESSED || event.type() == ET_KEY_RELEASED);
   DCHECK(system_toplevel_window_focused());
+  suppress_next_result_ = false;
 
   // If no text input client, do nothing.
   if (!GetTextInputClient())
     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();
-
-  // 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;
+  composition_changed_ = false;
+  result_text_.clear();
+
+  bool filtered = false;
+  {
+    ScopedBooleanFlipper flipper(&handling_key_event_);
+    if (context_focused_)
+      filtered = context_->DispatchKeyEvent(event);
+    else
+      filtered = context_simple_->DispatchKeyEvent(event);
+  }
+
+  if (event.type() == ui::ET_KEY_PRESSED && filtered) {
+    if (NeedInsertChar()) {
+      DispatchKeyEventPostIME(event);
+    } else {
+      ui::KeyEvent key(ui::ET_KEY_PRESSED, ui::VKEY_PROCESSKEY, event.flags());
+      DispatchKeyEventPostIME(key);
     }
   }
-  return handled;
+
+  // If has input result process the result.
+  if (result_text_.length() || composition_changed_)

[yukawa, 2015/04/08 22:26:49]

I slightly prefer string::empty(). How about this?
  if (!result_text_.empty() || composition_changed_)

[Shu Chen, 2015/04/09 01:16:32]

Done.

+    ProcessInputMethodResult(event, filtered);
+
+  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();
+    TextInputClient* client = GetTextInputClient();
+    if (ch && client)
+      client->InsertChar(ch, event.flags());
+  } else if (event.type() == ui::ET_KEY_RELEASED && !filtered) {
+    DispatchKeyEventPostIME(event);
+  }

[James Su, 2015/04/09 08:40:02]

add a comment somewhere to clarify that filtered key release events will be
swallowed and not generate 229 key up event.

+
+  return true;
+}
+
+void InputMethodAuraLinux::UpdateContextFocusState() {
+  bool old_context_focused = context_focused_;
+  // Use switch here in case we are going to add more text input types.
+  switch (GetTextInputType()) {
+    case ui::TEXT_INPUT_TYPE_NONE:
+    case ui::TEXT_INPUT_TYPE_PASSWORD:

[Seigo Nonaka, 2015/04/08 17:15:40]

Probably off the topic but I just noticed the issue about GtkContextSimple for
password field.
https://code.google.com/p/chromium/issues/detail?id=125161

If you have any idea, please also consider to fix it.

[yukawa, 2015/04/08 22:26:49]

Sounds good.  But I think it's OK to try it later so that we can keep the diff
in this CL as small as possible in case we need to do bisect.

[Shu Chen, 2015/04/09 01:16:33]

I would also prefer to consider it in a separated cl.

+      context_focused_ = false;
+      break;
+    default:
+      context_focused_ = true;
+      break;
+  }
+
+  // We only focus in |context_| when the focus is in a normal textfield.
+  if (old_context_focused && !context_focused_)
+    context_->Blur();
+  else if (!old_context_focused && context_focused_)
+    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();
+  DCHECK(!composing_text_);
+  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;
+
+  DCHECK(!handling_key_event_);

[Seigo Nonaka, 2015/04/08 17:15:40]

ditto?

[Shu Chen, 2015/04/09 01:16:32]

Done.

+
+  // To prevent any text from being committed when resetting the |context_|;
+  handling_key_event_ = 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 (context_focused_) {
+    context_->Blur();
+    context_->Focus();
+  }
+
+  composition_.Clear();
+  result_text_.clear();
+  handling_key_event_ = false;
+  composing_text_ = false;
+  composition_changed_ = false;
 }
 
 void InputMethodAuraLinux::OnInputLocaleChanged() {
@@ -120,63 +203,146 @@ bool InputMethodAuraLinux::IsCandidatePopupOpen() const {
 // Overriden from ui::LinuxInputMethodContextDelegate
 
 void InputMethodAuraLinux::OnCommit(const base::string16& text) {
-  MaybeFireProcessKey();
-  if (!IsTextInputTypeNone())
+  if (suppress_next_result_) {
+    suppress_next_result_ = false;
+    return;
+  }
+  if (!GetTextInputClient())
+    return;
+
+  if (!composing_text_ && !text.length())

[yukawa, 2015/04/08 22:26:48]

I slightly prefer string::empty(). How about this?
  if (!composing_text_ && text.empty())

[Shu Chen, 2015/04/09 01:16:32]

Done.

+    return;
+
+  // Append the text to the buffer, because commit signal might be fired
+  // multiple times when processing a key event.
+  result_text_.append(text);
+
+  // If we are not handling key event, do not bother sending text result if the
+  // focused text input client does not support text input.
+  if (!handling_key_event_ && !IsTextInputTypeNone()) {
+    SendFakeProcessKeyEvent(true);
     GetTextInputClient()->InsertText(text);
+  }
+}
+
+void InputMethodAuraLinux::OnPreeditStart() {
+  if (suppress_next_result_ || IsTextInputTypeNone())
+    return;
+
+  composing_text_ = true;
 }
 
 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;
+
+  if (!composition_.text.length() && !composition_text.text.length())

[yukawa, 2015/04/08 22:26:48]

I slightly prefer string::empty(). How about this?
  if (composition_.text.empty() && composition_text.text.empty())

[Shu Chen, 2015/04/09 01:16:32]

Done.

+    return;
+
+  composition_ = composition_text;
+  composition_changed_ = true;
+  if (composition_.text.length())

[yukawa, 2015/04/08 22:26:48]

I slightly prefer string::empty(). How about this?
  if (!composition_.text.empty())

[Shu Chen, 2015/04/09 01:16:32]

Done.

+    composing_text_ = true;
+
+  if (!handling_key_event_ && !IsTextInputTypeNone()) {
+    SendFakeProcessKeyEvent(true);
+    GetTextInputClient()->SetCompositionText(composition_);
+  }
 }
 
 void InputMethodAuraLinux::OnPreeditEnd() {
-  MaybeFireProcessKey();
-  TextInputClient* text_input_client = GetTextInputClient();
-  if (text_input_client && text_input_client->HasCompositionText())
-    text_input_client->ClearCompositionText();
-}
+  if (composition_.text.empty() || IsTextInputTypeNone())
+    return;
 
-void InputMethodAuraLinux::OnPreeditStart() {
-  MaybeFireProcessKey();
+  composition_changed_ = true;
+  composition_.Clear();
+
+  if (!handling_key_event_) {
+    TextInputClient* client = GetTextInputClient();
+    if (client && client->HasCompositionText())
+      client->ClearCompositionText();
+  }
 }
 
 // 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 (context_focused_)
+    OnCaretBoundsChanged(GetTextInputClient());
 
   InputMethodBase::OnDidChangeFocusedClient(focused_before, focused);
 }
 
-// Helper functions to support VKEY_PROCESSKEY.
+// private
+
+void InputMethodAuraLinux::ProcessInputMethodResult(const KeyEvent& key,
+                                                    bool filtered) {
+  TextInputClient* client = GetTextInputClient();
+  DCHECK(client);
+
+  if (result_text_.length()) {

[yukawa, 2015/04/08 22:26:48]

I slightly prefer string::empty(). How about this?
  if (!result_text_.empty()) {

[Shu Chen, 2015/04/09 01:16:32]

Done.

+    if (filtered && NeedInsertChar()) {
+      for (base::string16::const_iterator i = result_text_.begin();

[yukawa, 2015/04/08 22:26:48]

How about using range-based for?
  for (const auto c : result_text_)
    client->InsertChar(c, key.flags());

[Shu Chen, 2015/04/09 01:16:32]

Done.

+           i != result_text_.end(); ++i) {
+        client->InsertChar(*i, key.flags());
+      }
+    } else {
+      client->InsertText(result_text_);
+      composing_text_ = false;
+    }
+  }
 
-void InputMethodAuraLinux::AllowToFireProcessKey(const ui::KeyEvent& event) {
-  allowed_to_fire_vkey_process_key_ = true;
-  vkey_processkey_flags_ = event.flags();
+  if (composition_changed_ && !IsTextInputTypeNone()) {
+    if (composition_.text.length()) {

[yukawa, 2015/04/08 22:26:49]

I slightly prefer string::empty(). How about this?
    if (!composition_.text.empty()) {

[Shu Chen, 2015/04/09 01:16:32]

Done.

+      composing_text_ = true;
+      client->SetCompositionText(composition_);
+    } else if (result_text_.empty()) {
+      client->ClearCompositionText();
+    }
+  }
 }
 
-void InputMethodAuraLinux::MaybeFireProcessKey() {
-  if (!allowed_to_fire_vkey_process_key_)
-    return;
+bool InputMethodAuraLinux::NeedInsertChar() const {
+  return IsTextInputTypeNone() ||
+      (!composing_text_ && result_text_.length() == 1);
+}
 
-  const ui::KeyEvent fabricated_event(ET_KEY_PRESSED,
-                                      VKEY_PROCESSKEY,
-                                      vkey_processkey_flags_);
-  DispatchKeyEventPostIME(fabricated_event);
-  StopFiringProcessKey();
+void InputMethodAuraLinux::SendFakeProcessKeyEvent(bool pressed) const {
+  KeyEvent key(pressed ? ui::ET_KEY_PRESSED : ui::ET_KEY_RELEASED,
+               ui::VKEY_PROCESSKEY, 0);
+  DispatchKeyEventPostIME(key);
 }
 
-void InputMethodAuraLinux::StopFiringProcessKey() {
-  allowed_to_fire_vkey_process_key_ = false;
-  vkey_processkey_flags_ = 0;
+void InputMethodAuraLinux::ConfirmCompositionText() {
+  TextInputClient* client = GetTextInputClient();
+  if (client && client->HasCompositionText())
+    client->ConfirmCompositionText();
+
+  ResetContext();
 }
 
 }  // namespace ui