InputMethod should stop TextInputClient::InsertChar/InsertText calls when the event is stopped prop…

ash/ime/input_method_event_handler.cc

Index: ash/ime/input_method_event_handler.cc
diff --git a/ash/ime/input_method_event_handler.cc b/ash/ime/input_method_event_handler.cc
index 6cac9bba9798341eab45abd9b333062342a51634..8d3c0a8a6b8f22d4b4b1a15b8ba643c6b14295a5 100644
--- a/ash/ime/input_method_event_handler.cc
+++ b/ash/ime/input_method_event_handler.cc
@@ -30,7 +30,10 @@ void InputMethodEventHandler::OnKeyEvent(ui::KeyEvent* event) {
   // Never send synthesized key event.
   if (event->flags() & ui::EF_IS_SYNTHESIZED)
     return;
-  if (!post_ime_ && input_method_->DispatchKeyEvent(*event))
-    event->StopPropagation();
-}
+  if (post_ime_)
+    return;
+  input_method_->DispatchKeyEvent(*event);

[oshima, 2015/06/30 15:15:50]

Why we're ignoring the result now? Looks like the current impl returns true
only, but the code should be written based on API spec, not
particular impl.

[Shu Chen, 2015/07/01 02:02:42]

I will change the API spec in a separated CL.
The InputMethod::DispatchKeyEvent() return type should be void.
And its parameter should be KeyEvent* instead of const KeyEvent&.
Always return true in InputMethodChromeOS::DispatchKeyEvent() is actually a
workaround.

DispatchKeyEventPostIME() used to returns event.handled() but now
event.stopped_propagation(), which means sometimes it will return false. As I
noticed, some Views will do event.SetHandled() after handling the events, while
EventHandlers do event.StopPropagation().
If DispatchKeyEvent() uses DispatchKeyEventPostIME()'s return value to return
(not happen on ChromeOS but for other platforms), event->StopPropagation() won't
be called and there would be duplicated handling.

From another perspective, the key event flow should be always using
DispatchKeyEventPostIME() to continue dispatching instead of leveraging event
bits. So that IMEH should always consume the key events.

+  event->StopPropagation();
 }
+
+}  // namespace ash