IME for Mus: Make InputMethodMus use the IME Mojo API.

ui/views/mus/input_method_mus.cc

Index: ui/views/mus/input_method_mus.cc
diff --git a/ui/views/mus/input_method_mus.cc b/ui/views/mus/input_method_mus.cc
index c8d027eeab2e91aab2862567895df904e38a850f..41fd76a84fd46184389cb9114836fb665703207e 100644
--- a/ui/views/mus/input_method_mus.cc
+++ b/ui/views/mus/input_method_mus.cc
@@ -7,43 +7,51 @@
 #include <utility>
 
 #include "services/ui/public/cpp/window.h"
+#include "services/ui/public/interfaces/ime.mojom.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/events/event.h"
 #include "ui/platform_window/mojo/ime_type_converters.h"
 #include "ui/platform_window/mojo/text_input_state.mojom.h"
+#include "ui/views/mus/text_input_client_impl.h"
 
 namespace views {
 
 ////////////////////////////////////////////////////////////////////////////////
-// InputMethodMUS, public:
+// InputMethodMus, public:
 
-InputMethodMUS::InputMethodMUS(ui::internal::InputMethodDelegate* delegate,
+InputMethodMus::InputMethodMus(ui::internal::InputMethodDelegate* delegate,
                                ui::Window* window)
     : window_(window) {
   SetDelegate(delegate);
 }
 
-InputMethodMUS::~InputMethodMUS() {}
+InputMethodMus::~InputMethodMus() {}
+
+void InputMethodMus::Init(shell::Connector* connector) {
+  connector->ConnectToInterface("mojo:ui", &ime_server_);
+}
 
 ////////////////////////////////////////////////////////////////////////////////
-// InputMethodMUS, ui::InputMethod implementation:
+// InputMethodMus, ui::InputMethod implementation:
 
-void InputMethodMUS::OnFocus() {
+void InputMethodMus::OnFocus() {
   InputMethodBase::OnFocus();
   UpdateTextInputType();
 }
 
-void InputMethodMUS::OnBlur() {
+void InputMethodMus::OnBlur() {
   InputMethodBase::OnBlur();
   UpdateTextInputType();
 }
 
-bool InputMethodMUS::OnUntranslatedIMEMessage(const base::NativeEvent& event,
+bool InputMethodMus::OnUntranslatedIMEMessage(const base::NativeEvent& event,
                                               NativeEventResult* result) {
+  // This method is not called on non-Windows platforms. See the comments for
+  // ui::InputMethod::OnUntranslatedIMEMessage().
   return false;
 }
 
-void InputMethodMUS::DispatchKeyEvent(ui::KeyEvent* event) {
+void InputMethodMus::DispatchKeyEvent(ui::KeyEvent* event) {
   DCHECK(event->type() == ui::ET_KEY_PRESSED ||
          event->type() == ui::ET_KEY_RELEASED);
 
@@ -53,54 +61,71 @@ void InputMethodMUS::DispatchKeyEvent(ui::KeyEvent* event) {
     return;
   }
 
-  // Here is where we change the differ from our base class's logic. Instead of
-  // always dispatching a key down event, and then sending a synthesized
-  // character event, we instead check to see if this is a character event and
-  // send out the key if it is. (We fallback to normal dispatch if it isn't.)
-  if (event->is_char()) {
-    GetTextInputClient()->InsertChar(*event);
-    event->StopPropagation();
-    return;
-  }
-
-  ignore_result(DispatchKeyEventPostIME(event));
+  // IME driver will notify the text input client if it is not interested in
+  // event, which in turn will call DispatchKeyEventPostIME().
+  input_method_->ProcessKeyEvent(ui::Event::Clone(*event));
+  event->StopPropagation();

[sky, 2016/08/18 23:43:07]

Is the unconditional StopPropagation here going to be problematic? Will
accelerators be impacted? Can we only send the event if it's a char?

[Hadi, 2016/08/25 18:06:06]

1. If the key event still needs handling post-ime, it is done by calling
DispatchKeyEventPostIME(). In this case we call this function at
TextInputClientImpl::OnUnhandledEvent().

2. In ChromeOS, ash::InputMethodEventHandler::OnKeyEvent() marks the event as
consumed right after sending it to InputMethod::DispatchKeyEvent(). The "if
(post_ime_)" check is used for bypassing InputMethodEventHandler in the
implementation of DispatchKeyEventPostIME().

3. In Mus+ash's case, WindowTreeHostMus::DispatchEvent() already stops
processing the event after calling InputMethod::DispatchKeyEvent(), so whether
we add StopPropagation() here or not shouldn't make any visible difference.

So, I can move the event->StopPropagation() to
WindowTreeHostMus::DispatchEvent() so it looks a bit more like the code for
ChromeOS (i.e. caller of InputMethod::DispatchKeyEvent() stops the propagation).

On should we send non-char key events to IME, I couldn't find a place in
ChromeOS where it stops the key events from being sent to the extension. In the
input method extension framework docs I couldn't find anything mentioning
chrome.input.ime.onKeyEvent() will only receive character events. By looking at
Google Input Tools source code, it seemed to me that there are places where left
and right arrows are handled (although I need to do more investigation to make
sure those are called via onKeyEvent()). So I think we should send key events to
the extension.

[sky, 2016/08/25 20:17:53]

Wow, thanks for the great writeup, and it all makes sense.

 }
 
-void InputMethodMUS::OnTextInputTypeChanged(const ui::TextInputClient* client) {
+void InputMethodMus::OnTextInputTypeChanged(const ui::TextInputClient* client) {
   if (IsTextInputClientFocused(client))
     UpdateTextInputType();
   InputMethodBase::OnTextInputTypeChanged(client);
+
+  if (input_method_) {
+    input_method_->OnTextInputTypeChanged(
+        static_cast<ui::mojom::TextInputType>(client->GetTextInputType()));
+  }
 }
 
-void InputMethodMUS::OnCaretBoundsChanged(const ui::TextInputClient* client) {}
+void InputMethodMus::OnCaretBoundsChanged(const ui::TextInputClient* client) {
+  if (input_method_)
+    input_method_->OnCaretBoundsChanged(client->GetCaretBounds());
+}
 
-void InputMethodMUS::CancelComposition(const ui::TextInputClient* client) {}
+void InputMethodMus::CancelComposition(const ui::TextInputClient* client) {
+  if (input_method_)
+    input_method_->CancelComposition();
+}
 
-void InputMethodMUS::OnInputLocaleChanged() {}
+void InputMethodMus::OnInputLocaleChanged() {
+  // TODO(moshayedi): crbug.com/637418. Not supported in ChromeOS. Investigate
+  // whether we want to support this or not.
+}
 
-std::string InputMethodMUS::GetInputLocale() {
+std::string InputMethodMus::GetInputLocale() {
+  // TODO(moshayedi): crbug.com/637418. Not supported in ChromeOS. Investigate
+  // whether we want to support this or not.
   return "";

[sky, 2016/08/18 23:43:07]

"" -> std::string()

[Hadi, 2016/08/25 18:06:06]

Done.

 }
 
-bool InputMethodMUS::IsCandidatePopupOpen() const {
+bool InputMethodMus::IsCandidatePopupOpen() const {
+  // TODO(moshayedi): crbug.com/637416. Implement this properly when we have a
+  // mean for displaying candidate list popup.
   return false;
 }
 
-void InputMethodMUS::OnDidChangeFocusedClient(
+void InputMethodMus::OnDidChangeFocusedClient(
     ui::TextInputClient* focused_before,
     ui::TextInputClient* focused) {
   InputMethodBase::OnDidChangeFocusedClient(focused_before, focused);
   UpdateTextInputType();
+
+  text_input_client_.reset(new TextInputClientImpl(focused, this));
+  ime_server_->StartSession(text_input_client_->CreateInterfacePtrAndBind(),
+                            GetProxy(&input_method_));
 }
 
-void InputMethodMUS::UpdateTextInputType() {
+void InputMethodMus::UpdateTextInputType() {
   ui::TextInputType type = GetTextInputType();
   mojo::TextInputStatePtr state = mojo::TextInputState::New();
   state->type = mojo::ConvertTo<mojo::TextInputType>(type);
-  if (type != ui::TEXT_INPUT_TYPE_NONE)
-    window_->SetImeVisibility(true, std::move(state));
-  else
-    window_->SetTextInputState(std::move(state));
+  if (window_) {
+    if (type != ui::TEXT_INPUT_TYPE_NONE)
+      window_->SetImeVisibility(true, std::move(state));
+    else
+      window_->SetTextInputState(std::move(state));
+  }
 }
 
 }  // namespace views