aura: Updates the text input client when native activation changes.

ui/views/widget/widget.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/widget.h"
 
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 1022 matching lines @@
 void Widget::OnNativeWidgetActivationChanged(bool active) {
   // On windows we may end up here before we've completed initialization (from
   // an WM_NCACTIVATE). If that happens the WidgetDelegate likely doesn't know
   // the Widget and will crash attempting to access it.
   if (!active && native_widget_initialized_)
     SaveWindowPlacement();
 
   FOR_EACH_OBSERVER(WidgetObserver, observers_,
                     OnWidgetActivationChanged(this, active));
 
+  // With typical windowing systems, the order of activate/deactivate,
+  // focus/blur is the following.
+  //   1. Activate
+  //   2. Focus
+  //   3. Blur
+  //   4. Deactivate
+  //
+  // For Aura, the order of calls to OnNativeFocus, OnNativeBlur and
+  // OnNativeWidgetActivationChanged is a little confusing.  At the time of

[msw, 2014/05/20 17:08:07]

NativeWidgetAura::OnWindowActivated calls this function and it also stores and
restores the focused view, but *DesktopNativeWidgetAura::OnWindowActivated*
oddly doesn't call this, instead
*DesktopNativeWidgetAura::HandleActivationChanged* calls this, and it has a
modified view focusing logic with no view storing logic... This is pretty
complicated, but does the bug you're fixing only repro on desktop platforms
(Win/Linux), not CrOS/Metro/Ash? Perhaps the defect is actually in the order of
activation/focus operations, as you elude to in this comment?

[Yuki, 2014/05/21 14:36:51]

The issue is happening on Win, Linux and CrOS as I tested.  Probably on all Aura
platforms.

Win, Linux: After window creation, Omnibox doesn't get the text input focus.
CrOS: After the boot-up, sign-in screen (password field) doesn't get the text
input focus.

[msw, 2014/05/22 04:53:01]

Okay, then you can ignore my question, but the long comment here seems
potentially indicative of a bigger problem than what you're solving by adding
new [Focus|Blur]TextInputClient calls. Perhaps we'd get a better headway on the
underlying defect by filing this commentary as a bug, rather than adding a big
block of text here?

[Yuki, 2014/05/26 14:26:43]

Done.

+  // window creation, the order is as follows.
+  //   1. DesktopNativeWidgetAura::InitNativeWidget()
+  //   2. FocusClient::FocusWindow()
+  //      This call is made during the window initialization so that the focused
+  //      window never be null.
+  //   3. Widget::OnNativeFocus()
+  // Note that OnNativeFocus() is called at step 3 before
+  // OnNativeWidgetActivationChanged(true) is called.  OnNativeFocus() is called
+  // before it gets activated.
+  //
+  // Once the window creation finished and the native widget gets activated,
+  // the following methods are called.
+  //   4. DesktopNativeWidgetAura::HandleActivationChanged()
+  //   5. Widget::OnNativeWidgetActivationChanged(true)
+  // Note that OnNativeFocus() is not called this time because it's already been
+  // focused.
+  //
+  // For this reason, thre is a case that the active focused view has just
+  // changed but OnNativeFocus() will not be called later.  So we update the
+  // focused text input here in addition to OnNativeFocus and OnNativeBlur.

[msw, 2014/05/20 17:08:07]

nit: This comment is no longer accurate. I removed the OnNativeFocus and
OnNativeBlur TextInputClient updates in r271312, because
[Desktop]NativeWidgetAura::OnWindowFocused now stores and restores View focus
itself, which should trigger the desired TextInputClient changes.

[Yuki, 2014/05/26 14:26:43]

Done.

+  views::FocusManager* focus_manager = GetFocusManager();

[sky, 2014/05/20 13:47:15]

Your comments make me thing it would be better to do this from Init. Would that
make more sense?

[Yuki, 2014/05/20 13:54:55]

FocusTextInputClient() requires the widget to be active in order to make an
actual effect.  If the widget is not active, FocusTextInputClient() does nothing
because only the active widget's text input client should get focused.

The problem is that the widget is not active yet when Widget::Init is called. 
So we have to wait that the widget gets activated.

+  if (focus_manager) {
+    if (active)
+      focus_manager->FocusTextInputClient(focus_manager->GetFocusedView());
+    else
+      focus_manager->BlurTextInputClient(focus_manager->GetFocusedView());

[msw, 2014/05/22 04:53:01]

Is this actually needed to fix the issue? Perhaps you just added it for
completeness/parity?

[Yuki, 2014/05/22 14:02:11]

This is just for parity and completeness.

+  }
+
   if (IsVisible() && non_client_view())
     non_client_view()->frame_view()->SchedulePaint();
 }
 
 void Widget::OnNativeFocus(gfx::NativeView old_focused_view) {
   WidgetFocusManager::GetInstance()->OnWidgetFocusEvent(old_focused_view,
                                                         GetNativeView());
 }
 
 void Widget::OnNativeBlur(gfx::NativeView new_focused_view) {
@@ skipping 461 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // internal::NativeWidgetPrivate, NativeWidget implementation:
 
 internal::NativeWidgetPrivate* NativeWidgetPrivate::AsNativeWidgetPrivate() {
   return this;
 }
 
 }  // namespace internal
 }  // namespace views