cros: Fixes the text input focus when screen is locked.

cros: Fixes the text input focus when screen is locked.

When the screen is locked after some time not used, the initial text input focus is not in the password field when --enable-text-input-focus-manager is enabled.  This CL fixes this issue and puts the text input focus in the password field of the lock screen.

The issue occurs only when the screen gets locked because of time-out.  When it's explicitly locked by a user, the issue doesn't occur.  Please allow me about not adding a test case because it's tough to add a test case.

BUG=290701
TEST=Done manually.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281727

[Yuki, 2014-07-07 14:33:04]

Could you review this CL?

dzhioev@ as an owner of c/b/chromeos/login/.

msw@ as a reviwer of a series of text input focus refactoring.

[Nikita (slow), 2014-07-07 14:46:02]

>> When the screen is locked after some time not used,
>> the initial text input focus is not in the password field
>> when --enable-text-input-focus-manager is enabled.

Can you please explain why exactly is this happening?

[Yuki, 2014-07-07 15:09:44]

On 2014/07/07 14:46:02, Nikita Kostylev wrote:
> >> When the screen is locked after some time not used,
> >> the initial text input focus is not in the password field
> >> when --enable-text-input-focus-manager is enabled.
> 
> Can you please explain why exactly is this happening?

My understanding is that WebUIScreenLocker as views::View is not focused in the
old implementation, but we can type because the text input focus is managed
independent from views::FocusManager.

The experimental flag --enable-text-input-focus-manager lets views::FocusManager
control the text input focus, too.  So if a views::View is not focused, we
cannot type anything.

We've not noticed that WebUIScreenLocker is not focused because no one needed it
to be focused.

This is my understanding.

[Nikita (slow), 2014-07-07 15:32:09]

https://codereview.chromium.org/371923002/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/lock/webui_screen_locker.cc (right):

https://codereview.chromium.org/371923002/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/lock/webui_screen_locker.cc:97: if
(::switches::IsTextInputFocusManagerEnabled())
Can this condition be dropped?

[Nikita (slow), 2014-07-07 15:33:48]

On 2014/07/07 15:09:44, Yuki wrote:
> On 2014/07/07 14:46:02, Nikita Kostylev wrote:
> > >> When the screen is locked after some time not used,
> > >> the initial text input focus is not in the password field
> > >> when --enable-text-input-focus-manager is enabled.
> > 
> > Can you please explain why exactly is this happening?
> 
> My understanding is that WebUIScreenLocker as views::View is not focused in
the
> old implementation, but we can type because the text input focus is managed
> independent from views::FocusManager.
> 
> The experimental flag --enable-text-input-focus-manager lets
views::FocusManager
> control the text input focus, too.  So if a views::View is not focused, we
> cannot type anything.
> 
> We've not noticed that WebUIScreenLocker is not focused because no one needed
it
> to be focused.
> 
> This is my understanding.

So just requesting focus in all cases wouldn't hurt but I think this should be
investigated even further later i.e. why do we need to request manual focus at
all?

+oshima@

[oshima, 2014-07-07 17:41:13]

On 2014/07/07 15:33:48, Nikita Kostylev wrote:
> On 2014/07/07 15:09:44, Yuki wrote:
> > On 2014/07/07 14:46:02, Nikita Kostylev wrote:
> > > >> When the screen is locked after some time not used,
> > > >> the initial text input focus is not in the password field
> > > >> when --enable-text-input-focus-manager is enabled.
> > > 
> > > Can you please explain why exactly is this happening?
> > 
> > My understanding is that WebUIScreenLocker as views::View is not focused in
> the
> > old implementation, but we can type because the text input focus is managed
> > independent from views::FocusManager.
> > 
> > The experimental flag --enable-text-input-focus-manager lets
> views::FocusManager
> > control the text input focus, too.  So if a views::View is not focused, we
> > cannot type anything.
> > 
> > We've not noticed that WebUIScreenLocker is not focused because no one
needed
> it
> > to be focused.
> > 
> > This is my understanding.
> 
> So just requesting focus in all cases wouldn't hurt but I think this should be
> investigated even further later i.e. why do we need to request manual focus at
> all?
> 
> +oshima@

WidgetDelegate::GetInitiallyForcusedView is a generic way to tell which view
should get the initial focus,
(it can handle edge cases case such as a widget is shown without activation),
although this CL should work for this particular case.
(i'll leave this to you and yuki)

Given that a widget does not automatically set the focus on a view unless
specified upon Show(), Yuki's explanation sounds reasonable which was a bug in
IME.

[Yuki, 2014-07-08 09:34:18]

Thanks for the comment.  I've changed the approach to use
WidgetDelegate::GetInitiallyFocusedView.

Could you take another look?

[Nikita (slow), 2014-07-08 10:07:49]

lgtm

[dzhioev (left Google), 2014-07-08 10:17:35]

On 2014/07/08 09:34:18, Yuki wrote:
> Thanks for the comment.  I've changed the approach to use
> WidgetDelegate::GetInitiallyFocusedView.
> 
> Could you take another look?

lgtm

[Yuki, 2014-07-08 11:28:19]

The CQ bit was checked by yukishiino@chromium.org

[commit-bot: I haz the power, 2014-07-08 11:28:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yukishiino@chromium.org/371923002/20001

[commit-bot: I haz the power, 2014-07-08 12:13:10]

Change committed as 281727