Refactoring the ownership of ui::InputMethod.

ui/aura/window_tree_host.cc

Index: ui/aura/window_tree_host.cc
diff --git a/ui/aura/window_tree_host.cc b/ui/aura/window_tree_host.cc
index 772b020586660d0d9f671afdb914f0326536b926..343854a56e2233ccf887323eb2969ee43b7b4c91 100644
--- a/ui/aura/window_tree_host.cc
+++ b/ui/aura/window_tree_host.cc
@@ -13,6 +13,8 @@
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_targeter.h"
 #include "ui/aura/window_tree_host_observer.h"
+#include "ui/base/ime/input_method.h"
+#include "ui/base/ime/input_method_factory.h"
 #include "ui/base/view_prop.h"
 #include "ui/compositor/dip_util.h"
 #include "ui/compositor/layer.h"
@@ -41,6 +43,10 @@ float GetDeviceScaleFactorFromDisplay(Window* window) {
 
 WindowTreeHost::~WindowTreeHost() {
   DCHECK(!compositor_) << "compositor must be destroyed before root window";
+  if (owned_input_method_) {
+    delete input_method_;
+    input_method_ = nullptr;
+  }
 }
 
 #if defined(OS_ANDROID)
@@ -50,7 +56,7 @@ WindowTreeHost* WindowTreeHost::Create(const gfx::Rect& bounds) {
   // adding the CHECK.
   // TODO(sky): decide if we want a factory.
   CHECK(false);
-  return NULL;
+  return nullptr;
 }
 #endif
 
@@ -172,6 +178,33 @@ void WindowTreeHost::MoveCursorToHostLocation(const gfx::Point& host_location) {
   MoveCursorToInternal(root_location, host_location);
 }
 
+ui::InputMethod* WindowTreeHost::GetInputMethod() {
+  if (!input_method_) {
+    input_method_ =
+        ui::CreateInputMethod(this, GetAcceleratedWidget()).release();
+    // Makes sure the input method is focused by default when created, because
+    // some environment doesn't have activated/focused state in WindowTreeHost.
+    // TODO(shuchen): move this to DisplayController so it's only for Ash.
+    input_method_->OnFocus();
+    owned_input_method_ = true;
+  }
+  return input_method_;
+}
+
+void WindowTreeHost::SetInputMethod(ui::InputMethod* input_method) {
+  DCHECK(!input_method_);
+  input_method_ = input_method;
+  owned_input_method_ = false;
+}
+
+bool WindowTreeHost::DispatchKeyEventPostIME(const ui::KeyEvent& event) {

[sadrul, 2015/06/08 22:45:12]

Not for this CL, but would it be possible for DispatchKeyEventPostIME() (and
InputMethod::DispatchKeyEvent()) to return an EventDispatchDetails? It is
possible for the dispatched event to actually destroy the target/dispatcher. So
it would be good to retain that information and pass it on to the caller.

[Shu Chen, 2015/06/09 02:28:49]

The caller of InputMethod::DispatchKeyEvent() wants to know whether the
InputMethod has handled the key event, so that it can stop the propagation as
appropriate (see WindowTreeHost::DeliverEventToProcessor below).
I would prefer to put EventDispatchDetails as the second param for the
DispatchKeyEvent().
So that the bool return value can be kept for above reason.
It is doable in a separated CL.

However, I'd like to raise a concern to the design of event dispatching.
Currently, the re-writer logics are implemented in EventSource, which is WTH.
So to dispatch events from a WTH, the re-writer logics run.
For re-dispatching logics in WindowTargeter::FindTargetForEvent(), which happens
after re-writers, will cause the re-writers run again in another WTH.
Is it feasible to put the re-writer logics close to the platform-dependent
layer? so that they can:
1) directly read/write the native events instead of ui::KeyEvent's.
2) become global, instead of bound to certain WTH.
3) InputMethod can intercept the key events from there too.
Therefore, there would be no need to deal with EventDispatchDetails.

[sadrul, 2015/06/09 15:29:32]

You could send a pointer to the KeyEvent in DispatchKeyEvent(), so that the
consumer can set the handled bit on the event if it handled the event. This
would be consistent with the normal event dispatch logic. (passing the EDD as
the second param would also work, but maybe not as nice because of
inconsistency).
EventDispatchDetails is used to track whether the event-target or the host is
destroyed during the dispatch, and the InputMethod is also involved during the
dispatch phase. So I do not see how changing anything in the rewriter/targeter
logic obviates its usefulness.
We should use less of 'platform dependent' events, and not more. (e.g. we used
to have chromeos event-rewriter that used to deal with raw X11 events. But for
ozone, it has been converted to work with ui::KeyEvent (kpschoedel@ can give you
more details)).
This can maybe happen, yes (maybe these should hang off of Env).
It would be terrific if IM could be implemented as an event-rewriter. I seem to
remember I made this suggestion earlier in the doc.

[Shu Chen, 2015/06/10 03:01:35]

Actually I meant "InputMethod can intercept the key events from the 'platform
dependent' layer instead of being a event rewriter.

Thanks for the elaboration. I've added a TODO in DeliverEventToProcessor().

+  ui::KeyEvent copied_event(event);
+  ui::EventDispatchDetails details =
+      event_processor()->OnEventFromSource(&copied_event);
+  DCHECK(!details.dispatcher_destroyed);
+  return copied_event.handled();
+}
+
 void WindowTreeHost::Show() {
   if (compositor())
     compositor()->SetVisible(true);
@@ -188,8 +221,10 @@ void WindowTreeHost::Hide() {
 // WindowTreeHost, protected:
 
 WindowTreeHost::WindowTreeHost()
-    : window_(new Window(NULL)),
-      last_cursor_(ui::kCursorNull) {
+    : window_(new Window(nullptr)),
+      last_cursor_(ui::kCursorNull),
+      input_method_(nullptr),
+      owned_input_method_(false) {
 }
 
 void WindowTreeHost::DestroyCompositor() {
@@ -198,7 +233,7 @@ void WindowTreeHost::DestroyCompositor() {
 
 void WindowTreeHost::DestroyDispatcher() {
   delete window_;
-  window_ = NULL;
+  window_ = nullptr;
   dispatcher_.reset();
 
   // TODO(beng): this comment is no longer quite valid since this function
@@ -271,6 +306,22 @@ void WindowTreeHost::OnHostLostWindowCapture() {
     capture_window->ReleaseCapture();
 }
 
+ui::EventProcessor* WindowTreeHost::GetEventProcessor() {
+  return event_processor();
+}
+
+ui::EventDispatchDetails WindowTreeHost::DeliverEventToProcessor(
+    ui::Event* event) {
+  if (event->IsKeyEvent()) {
+    if (GetInputMethod()->DispatchKeyEvent(
+            *static_cast<ui::KeyEvent*>(event))) {
+      event->StopPropagation();
+      return ui::EventDispatchDetails();
+    }
+  }
+  return ui::EventSource::DeliverEventToProcessor(event);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // WindowTreeHost, private: