Refactoring for the InputMethod & InputMethodDelegate interfaces.

ui/base/ime/input_method_chromeos.cc

 // Copyright 2014 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_chromeos.h"
 
 #include <algorithm>
 #include <cstring>
 #include <set>
 #include <vector>
@@ skipping 53 matching lines @@
   InputMethodBase::OnBlur();
   OnTextInputTypeChanged(GetTextInputClient());
 }
 
 bool InputMethodChromeOS::OnUntranslatedIMEMessage(
     const base::NativeEvent& event,
     NativeEventResult* result) {
   return false;
 }
 
-void InputMethodChromeOS::ProcessKeyEventDone(const ui::KeyEvent* event,
+void InputMethodChromeOS::ProcessKeyEventDone(ui::KeyEvent* event,
                                               bool is_handled) {
   DCHECK(event);
   if (event->type() == ET_KEY_PRESSED) {
     if (is_handled) {
       // IME event has a priority to be handled, so that character composer
       // should be reset.
       character_composer_.Reset();
     } else {
       // If IME does not handle key event, passes keyevent to character composer
       // to be able to compose complex characters.
       is_handled = ExecuteCharacterComposer(*event);
     }
   }
 
   if (event->type() == ET_KEY_PRESSED || event->type() == ET_KEY_RELEASED)
-    ProcessKeyEventPostIME(*event, is_handled);
+    ProcessKeyEventPostIME(event, is_handled);
 
   handling_key_event_ = false;
 }
 
-bool InputMethodChromeOS::DispatchKeyEvent(const ui::KeyEvent& event) {
-  DCHECK(event.IsKeyEvent());
-  DCHECK(!(event.flags() & ui::EF_IS_SYNTHESIZED));
+void InputMethodChromeOS::DispatchKeyEvent(ui::KeyEvent* event) {
+  DCHECK(event->IsKeyEvent());
+  DCHECK(!(event->flags() & ui::EF_IS_SYNTHESIZED));
   DCHECK(system_toplevel_window_focused());
 
   // For linux_chromeos, the ime keyboard cannot track the caps lock state by
   // itself, so need to call SetCapsLockEnabled() method to reflect the caps
   // lock state by the key event.
   if (!base::SysInfo::IsRunningOnChromeOS()) {
     chromeos::input_method::InputMethodManager* manager =
         chromeos::input_method::InputMethodManager::Get();
     if (manager) {
       chromeos::input_method::ImeKeyboard* keyboard = manager->GetImeKeyboard();
-      if (keyboard && event.type() == ui::ET_KEY_PRESSED) {
-        bool caps = (event.key_code() == ui::VKEY_CAPITAL)
+      if (keyboard && event->type() == ui::ET_KEY_PRESSED) {
+        bool caps = (event->key_code() == ui::VKEY_CAPITAL)
             ? !keyboard->CapsLockIsEnabled()
-            : (event.flags() & EF_CAPS_LOCK_DOWN);
+            : (event->flags() & EF_CAPS_LOCK_DOWN);
         keyboard->SetCapsLockEnabled(caps);
       }
     }
   }
 
   // If |context_| is not usable, then we can only dispatch the key event as is.
   // We only dispatch the key event to input method when the |context_| is an
   // normal input field (not a password field).
   // Note: We need to send the key event to ibus even if the |context_| is not
   // enabled, so that ibus can have a chance to enable the |context_|.
   if (!IsNonPasswordInputFieldFocused() || !GetEngine()) {
-    if (event.type() == ET_KEY_PRESSED) {
-      if (ExecuteCharacterComposer(event)) {
+    if (event->type() == ET_KEY_PRESSED) {
+      if (ExecuteCharacterComposer(*event)) {
         // Treating as PostIME event if character composer handles key event and
         // generates some IME event,
         ProcessKeyEventPostIME(event, true);
-        return true;
+        return;
       }
       ProcessUnfilteredKeyPressEvent(event);
     } else {
-      DispatchKeyEventPostIME(event);
+      ignore_result(DispatchKeyEventPostIME(event));
     }
-    return true;
+    return;
   }
 
   handling_key_event_ = true;
   if (GetEngine()->IsInterestedInKeyEvent()) {
     GetEngine()->ProcessKeyEvent(
-        event,
+        *event,
         base::Bind(&InputMethodChromeOS::ProcessKeyEventDone,
                    weak_ptr_factory_.GetWeakPtr(),
                    // Pass the ownership of the new copied event.
-                   base::Owned(new ui::KeyEvent(event))));
+                   base::Owned(new ui::KeyEvent(*event))));
   } else {
-    ProcessKeyEventDone(&event, false);
+    ProcessKeyEventDone(event, false);
   }
-  return true;
 }
 
 void InputMethodChromeOS::OnTextInputTypeChanged(
     const TextInputClient* client) {
   if (!IsTextInputClientFocused(client))
     return;
 
   UpdateContextFocusState();
 
   chromeos::IMEEngineHandlerInterface* engine = GetEngine();
@@ skipping 170 matching lines @@
     candidate_window->FocusStateChanged(IsNonPasswordInputFieldFocused());
 
   chromeos::IMEEngineHandlerInterface::InputContext context(
       GetTextInputType(), GetTextInputMode(), GetTextInputFlags());
   chromeos::IMEBridge::Get()->SetCurrentInputContext(context);
 
   if (!IsTextInputTypeNone())
     OnCaretBoundsChanged(GetTextInputClient());
 }
 
-void InputMethodChromeOS::ProcessKeyEventPostIME(const ui::KeyEvent& event,
+void InputMethodChromeOS::ProcessKeyEventPostIME(ui::KeyEvent* event,
                                                  bool handled) {
   TextInputClient* client = GetTextInputClient();
   if (!client) {
     // As ibus works asynchronously, there is a chance that the focused client
     // loses focus before this method gets called.
-    DispatchKeyEventPostIME(event);
+    ignore_result(DispatchKeyEventPostIME(event));
     return;
   }
 
-  if (event.type() == ET_KEY_PRESSED && handled) {
-    if (ProcessFilteredKeyPressEvent(event)) {
+  if (event->type() == ET_KEY_PRESSED && handled) {
+    ProcessFilteredKeyPressEvent(event);

[James Su, 2015/07/31 11:43:58]

I think the result of DispatchKeyEventPostIME() call shouldn't be ignored here.

[Shu Chen, 2015/08/03 01:44:47]

ChromeOS uses InputMethodEventHandler so destruction of WTH won't cause
destruction of InputMethodChromeOS. So it is safe to not check
EventDispatchDetails::dispatcher_destroyed.

+    if (event->stopped_propagation()) {
       ResetContext();
       return;
     }
   }
 
   // In case the focus was changed by the key event. The |context_| should have
   // been reset when the focused window changed.
   if (client != GetTextInputClient())
     return;
 
   if (HasInputMethodResult())
     ProcessInputMethodResult(event, handled);
 
   // In case the focus was changed when sending input method results to the
   // focused window.
   if (client != GetTextInputClient())
     return;
 
   if (handled)
     return;  // IME handled the key event. do not forward.
 
-  if (event.type() == ET_KEY_PRESSED)
+  if (event->type() == ET_KEY_PRESSED)
     ProcessUnfilteredKeyPressEvent(event);
-  else if (event.type() == ET_KEY_RELEASED)
-    DispatchKeyEventPostIME(event);
+  else if (event->type() == ET_KEY_RELEASED)
+    ignore_result(DispatchKeyEventPostIME(event));
 }
 
-bool InputMethodChromeOS::ProcessFilteredKeyPressEvent(
-    const ui::KeyEvent& event) {
-  if (NeedInsertChar())
-    return DispatchKeyEventPostIME(event);
-  const ui::KeyEvent fabricated_event(ET_KEY_PRESSED,
-                                      VKEY_PROCESSKEY,
-                                      event.flags());
-  return DispatchKeyEventPostIME(fabricated_event);
+void InputMethodChromeOS::ProcessFilteredKeyPressEvent(ui::KeyEvent* event) {
+  if (NeedInsertChar()) {
+    ignore_result(DispatchKeyEventPostIME(event));
+    return;
+  }
+  ui::KeyEvent fabricated_event(ET_KEY_PRESSED,
+                                VKEY_PROCESSKEY,
+                                event->flags());
+  ignore_result(DispatchKeyEventPostIME(&fabricated_event));
+  if (fabricated_event.stopped_propagation())
+    event->StopPropagation();
 }
 
 void InputMethodChromeOS::ProcessUnfilteredKeyPressEvent(
-    const ui::KeyEvent& event) {
+    ui::KeyEvent* event) {
   const TextInputClient* prev_client = GetTextInputClient();
-  if (DispatchKeyEventPostIME(event)) {
+  ignore_result(DispatchKeyEventPostIME(event));

[James Su, 2015/07/31 11:43:58]

I think this result shouldn't be ignored.

[Shu Chen, 2015/08/03 01:44:47]

ditto.

+  if (event->stopped_propagation()) {
     ResetContext();
     return;
   }
 
   // We shouldn't dispatch the character anymore if the key event dispatch
   // caused focus change. For example, in the following scenario,
   // 1. visit a web page which has a <textarea>.
   // 2. click Omnibox.
   // 3. enable Korean IME, press A, then press Tab to move the focus to the web
   //    page.
   // We should return here not to send the Tab key event to RWHV.
   TextInputClient* client = GetTextInputClient();
   if (!client || client != prev_client)
     return;
 
   // If a key event was not filtered by |context_| and |character_composer_|,
   // then it means the key event didn't generate any result text. So we need
   // to send corresponding character to the focused text input client.
-  uint16 ch = event.GetCharacter();
+  uint16 ch = event->GetCharacter();
   if (ch)
-    client->InsertChar(ch, event.flags());
+    client->InsertChar(ch, event->flags());
 }
 
-void InputMethodChromeOS::ProcessInputMethodResult(const ui::KeyEvent& event,
+void InputMethodChromeOS::ProcessInputMethodResult(ui::KeyEvent* event,
                                                    bool handled) {
   TextInputClient* client = GetTextInputClient();
   DCHECK(client);
 
   if (result_text_.length()) {
     if (handled && NeedInsertChar()) {
       for (base::string16::const_iterator i = result_text_.begin();
            i != result_text_.end(); ++i) {
-        client->InsertChar(*i, event.flags());
+        client->InsertChar(*i, event->flags());
       }
     } else {
       client->InsertText(result_text_);
       composing_text_ = false;
     }
   }
 
   if (composition_changed_ && !IsTextInputTypeNone()) {
     if (composition_.text.length()) {
       composing_text_ = true;
@@ skipping 18 matching lines @@
 bool InputMethodChromeOS::HasInputMethodResult() const {
   return result_text_.length() || composition_changed_;
 }
 
 void InputMethodChromeOS::SendFakeProcessKeyEvent(bool pressed) const {
   if (!GetTextInputClient())
     return;
   KeyEvent evt(pressed ? ET_KEY_PRESSED : ET_KEY_RELEASED,
                pressed ? VKEY_PROCESSKEY : VKEY_UNKNOWN,
                EF_IME_FABRICATED_KEY);
-  DispatchKeyEventPostIME(evt);
+  ignore_result(DispatchKeyEventPostIME(&evt));

[James Su, 2015/07/31 11:43:58]

I think this result shouldn't be ignored, and evt.stopped_propagation() should
also be checked everywhere.

[Shu Chen, 2015/08/03 01:44:47]

ditto & Done.

 }
 
 void InputMethodChromeOS::CommitText(const std::string& text) {
   if (text.empty())
     return;
 
   // We need to receive input method result even if the text input type is
   // TEXT_INPUT_TYPE_NONE, to make sure we can always send correct
   // character for each key event to the focused text input client.
   if (!GetTextInputClient())
@@ skipping 196 matching lines @@
 bool InputMethodChromeOS::IsNonPasswordInputFieldFocused() {
   TextInputType type = GetTextInputType();
   return (type != TEXT_INPUT_TYPE_NONE) && (type != TEXT_INPUT_TYPE_PASSWORD);
 }
 
 bool InputMethodChromeOS::IsInputFieldFocused() {
   return GetTextInputType() != TEXT_INPUT_TYPE_NONE;
 }
 
 }  // namespace ui