Redesigns the text input focus handling.

ui/views/widget/desktop_aura/desktop_native_widget_aura.cc

 // Copyright (c) 2012 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/views/widget/desktop_aura/desktop_native_widget_aura.h"
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "ui/aura/client/activation_client.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/client/cursor_client.h"
 #include "ui/aura/client/drag_drop_client.h"
 #include "ui/aura/client/focus_client.h"
 #include "ui/aura/client/window_tree_client.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_observer.h"
 #include "ui/aura/window_property.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/hit_test.h"
+#include "ui/base/ui_base_switches_util.h"
 #include "ui/compositor/layer.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/display.h"
 #include "ui/gfx/point_conversions.h"
 #include "ui/gfx/screen.h"
 #include "ui/gfx/size_conversions.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/corewm/compound_event_filter.h"
 #include "ui/views/corewm/corewm_switches.h"
 #include "ui/views/corewm/cursor_manager.h"
 #include "ui/views/corewm/focus_controller.h"
 #include "ui/views/corewm/input_method_event_filter.h"
 #include "ui/views/corewm/native_cursor_manager.h"
 #include "ui/views/corewm/shadow_controller.h"
 #include "ui/views/corewm/shadow_types.h"
 #include "ui/views/corewm/tooltip.h"
 #include "ui/views/corewm/tooltip_controller.h"
 #include "ui/views/corewm/visibility_controller.h"
 #include "ui/views/corewm/window_modality_controller.h"
 #include "ui/views/drag_utils.h"
-#include "ui/views/ime/input_method.h"
 #include "ui/views/ime/input_method_bridge.h"
+#include "ui/views/ime/null_input_method.h"
 #include "ui/views/widget/desktop_aura/desktop_capture_client.h"
 #include "ui/views/widget/desktop_aura/desktop_cursor_loader_updater.h"
 #include "ui/views/widget/desktop_aura/desktop_dispatcher_client.h"
 #include "ui/views/widget/desktop_aura/desktop_event_client.h"
 #include "ui/views/widget/desktop_aura/desktop_focus_rules.h"
 #include "ui/views/widget/desktop_aura/desktop_native_cursor_manager.h"
 #include "ui/views/widget/desktop_aura/desktop_screen_position_client.h"
 #include "ui/views/widget/desktop_aura/desktop_window_tree_host.h"
 #include "ui/views/widget/drop_helper.h"
 #include "ui/views/widget/native_widget_aura.h"
@@ skipping 566 matching lines @@
 
   content_window_->ReleaseCapture();
 }
 
 bool DesktopNativeWidgetAura::HasCapture() const {
   return content_window_ && content_window_->HasCapture() &&
       desktop_window_tree_host_->HasCapture();
 }
 
 InputMethod* DesktopNativeWidgetAura::CreateInputMethod() {
+  if (switches::IsNewTextInputFocusEnabled())
+    return new NullInputMethod();

[msw, 2014/03/11 00:58:50]

Is there an advantage to using NullInputMethod here, instead of NULL?

[Yuki, 2014/03/11 15:27:37]

NULL doesn't work.  Some of (not few) clients assume that GetInputMethod()
always returns non-NULL.

[msw, 2014/03/11 23:24:37]

Okay, then this is fine.

+
   ui::InputMethod* host = input_method_event_filter_->input_method();
   return new InputMethodBridge(this, host, false);
 }
 
 internal::InputMethodDelegate*
     DesktopNativeWidgetAura::GetInputMethodDelegate() {
   return this;
 }
 
+ui::InputMethod* DesktopNativeWidgetAura::GetHostInputMethod() {
+  return input_method_event_filter_->input_method();
+}
+
 void DesktopNativeWidgetAura::CenterWindow(const gfx::Size& size) {
   if (content_window_)
     desktop_window_tree_host_->CenterWindow(size);
 }
 
 void DesktopNativeWidgetAura::GetWindowPlacement(
       gfx::Rect* bounds,
       ui::WindowShowState* maximized) const {
   if (content_window_)
     desktop_window_tree_host_->GetWindowPlacement(bounds, maximized);
@@ skipping 421 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // DesktopNativeWidgetAura, aura::client::FocusChangeObserver implementation:
 
 void DesktopNativeWidgetAura::OnWindowFocused(aura::Window* gained_focus,
                                               aura::Window* lost_focus) {
   if (content_window_ == gained_focus) {
     desktop_window_tree_host_->OnNativeWidgetFocus();
     native_widget_delegate_->OnNativeFocus(lost_focus);
 
-    // If focus is moving from a descendant Window to |content_window_| then
-    // native activation hasn't changed. We still need to inform the InputMethod
-    // we've been focused though.
-    InputMethod* input_method = GetWidget()->GetInputMethod();
-    if (input_method)
-      input_method->OnFocus();
+    if (!switches::IsNewTextInputFocusEnabled()) {
+      // If focus is moving from a descendant Window to |content_window_| then
+      // native activation hasn't changed. We still need to inform the
+      // InputMethod we've been focused though.
+      InputMethod* input_method = GetWidget()->GetInputMethod();
+      if (input_method)
+        input_method->OnFocus();
+    }
+
+    views::FocusManager* focus_manager = GetWidget()->GetFocusManager();

[msw, 2014/03/11 00:58:50]

If the View focused before the Widget was deactivated was "stored" (blurred on
deactivate) and is going to be focused on Widget activation, has that already
happened? Or is the focused view going to be NULL here?

[Yuki, 2014/03/11 15:27:37]

OnWindowActivated() is called before OnWindowFocused(), and the stored view has
already been restored at this point.  Therefore
|focus_manager->GetFocusedView()| returns the focused view that we want.

[msw, 2014/03/11 23:24:37]

Great, thanks for checking. This may not actually be necessary if the
FocusManager is guaranteed to restore Focus to its last-focused View on
Activate, but I think having this explicit call is better than hoping the
implicit focus restoration does the same thing (at least for now). You might
want to add a comment here, explaining what this does, like "Ensure the focused
view's TextInputClient is used for text input."

[Yuki, 2014/03/12 09:13:15]

Done.

+    focus_manager->OnTextInputClientChanged(focus_manager->GetFocusedView());
   } else if (content_window_ == lost_focus) {

[msw, 2014/03/11 00:58:50]

Should we explicitly clear the TextInputClient in this case?

[Yuki, 2014/03/11 15:27:37]

I think we don't need to do so.

Losing the focus means that someone else is getting the focus right away.  So
the new focused window's view will soon set the focused TextInputClient.  So
there is no need to clear the TextInputClient explicitly here.

Actually there are 3 scenarios.
1. Another Chrome window gets the focus. => No problem as I wrote above. 
Another view gets the focus.
2. Another application gets the focus. => No problem because Chrome doesn't get
any input.
3. Chrome is terminating. => No problem because everything is soon gone.

We can clear the TextInputClient here, but I don't like to put such code outside
of FocusManager as much as possible.

[msw, 2014/03/11 23:24:37]

Hmm, I guess this should be okay; the focused View should be Blur()'ed on
deactivate anyway, and that'll do the same thing. I just have a slight
inclination to make this explicit like above, or make both implicit based on the
FocusManager doing a blur/focus on deactivate/active.

[Yuki, 2014/03/12 09:13:15]

This has been a tough question for me.  Now I've changed my mind and put calls
to {Focus,Blur}TextInputClient() here and above.

     desktop_window_tree_host_->OnNativeWidgetBlur();
     native_widget_delegate_->OnNativeBlur(
         aura::client::GetFocusClient(content_window_)->GetFocusedWindow());
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // DesktopNativeWidgetAura, views::internal::InputMethodDelegate:
 
 void DesktopNativeWidgetAura::DispatchKeyEventPostIME(const ui::KeyEvent& key) {
@@ skipping 91 matching lines @@
   if (cursor_reference_count_ == 0) {
     // We are the last DesktopNativeWidgetAura instance, and we are responsible
     // for cleaning up |cursor_manager_|.
     delete cursor_manager_;
     native_cursor_manager_ = NULL;
     cursor_manager_ = NULL;
   }
 }
 
 }  // namespace views