Refactoring for the InputMethod & InputMethodDelegate interfaces.

ui/base/ime/input_method_chromeos.cc

Index: ui/base/ime/input_method_chromeos.cc
diff --git a/ui/base/ime/input_method_chromeos.cc b/ui/base/ime/input_method_chromeos.cc
index 47c9c1678e6d34ecd3380e9e43f12acc520565e9..2b9757478c77bae188977e745e9589eda4def6ef 100644
--- a/ui/base/ime/input_method_chromeos.cc
+++ b/ui/base/ime/input_method_chromeos.cc
@@ -71,7 +71,7 @@ bool InputMethodChromeOS::OnUntranslatedIMEMessage(
   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) {
@@ -87,14 +87,14 @@ void InputMethodChromeOS::ProcessKeyEventDone(const ui::KeyEvent* 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
@@ -105,10 +105,10 @@ bool InputMethodChromeOS::DispatchKeyEvent(const ui::KeyEvent& event) {
         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);
       }
     }
@@ -120,32 +120,31 @@ bool InputMethodChromeOS::DispatchKeyEvent(const ui::KeyEvent& event) {
   // 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(
@@ -336,18 +335,19 @@ void InputMethodChromeOS::UpdateContextFocusState() {
     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;
     }
@@ -369,26 +369,30 @@ void InputMethodChromeOS::ProcessKeyEventPostIME(const ui::KeyEvent& event,
   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;
   }
@@ -407,12 +411,12 @@ void InputMethodChromeOS::ProcessUnfilteredKeyPressEvent(
   // 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);
@@ -421,7 +425,7 @@ void InputMethodChromeOS::ProcessInputMethodResult(const ui::KeyEvent& event,
     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_);
@@ -460,7 +464,7 @@ void InputMethodChromeOS::SendFakeProcessKeyEvent(bool pressed) const {
   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) {