aura: Fix WidgetFocusChangeListener::OnNativeFocusChange().

aura: Fix WidgetFocusChangeListener::OnNativeFocusChange().

In Aura, we were passing the newly-focused view through
views::WidgetFocusManager::OnWidgetFocusEvent()'s
"focused_before" parameter.  This made us not close
extension popups when they lost the focus.

(Note that a popup still doesn't get closed upon clicking
outside of the popup on the desktop, presumably because the
popup retains the focus in Ash.)

BUG=132697
TEST=added, also manual testing


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=142858

[Ben Goodger (Google), 2012-06-18 16:56:44]

lgtm

[Daniel Erat, 2012-06-18 19:25:15]

https://chromiumcodereview.appspot.com/10562025/diff/6004/ui/views/focus/focu...
File ui/views/focus/focus_manager_unittest.cc (right):

https://chromiumcodereview.appspot.com/10562025/diff/6004/ui/views/focus/focu...
ui/views/focus/focus_manager_unittest.cc:122: TEST_F(FocusManagerTest,
WidgetFocusChangeListener) {
This test times out on non-Aura Windows.  I know nothing about the Windows focus
model -- is it even possible for child native views to receive the focus, or can
only toplevel ones receive it?  (The comment in NativeWidgetAura::OnFocus()
suggests that the latter may be the case.)

Recommendations for how to make this test relevant / passing on Windows, or
should I just ifdef it to be Aura-only?

[Ben Goodger (Google), 2012-06-18 20:32:09]

child HWNDs can receive focus (and indeed must be for key events to be
received by them, e.g. RenderWidgetHostViewWin) but only the toplevel can
be activated.

-Ben

On Mon, Jun 18, 2012 at 12:25 PM, <derat@chromium.org> wrote:

>
> https://chromiumcodereview.**appspot.com/10562025/diff/**
>
6004/ui/views/focus/focus_**manager_unittest.cc<https://chromiumcodereview.appspot.com/10562025/diff/6004/ui/views/focus/focus_manager_unittest.cc>
> File ui/views/focus/focus_manager_**unittest.cc (right):
>
> https://chromiumcodereview.**appspot.com/10562025/diff/**
>
6004/ui/views/focus/focus_**manager_unittest.cc#newcode122<https://chromiumcodereview.appspot.com/10562025/diff/6004/ui/views/focus/focus_manager_unittest.cc#newcode122>
> ui/views/focus/focus_manager_**unittest.cc:122: TEST_F(FocusManagerTest,
> WidgetFocusChangeListener) {
> This test times out on non-Aura Windows.  I know nothing about the
> Windows focus model -- is it even possible for child native views to
> receive the focus, or can only toplevel ones receive it?  (The comment
> in NativeWidgetAura::OnFocus() suggests that the latter may be the
> case.)
>
> Recommendations for how to make this test relevant / passing on Windows,
> or should I just ifdef it to be Aura-only?
>
>
https://chromiumcodereview.**appspot.com/10562025/<https://chromiumcodereview...
>

[Daniel Erat, 2012-06-18 21:42:57]

Thanks for the explanation.  I think that the test failure that I saw was caused
by a problem that I fixed in patch set 4 (the WidgetFocusChangeListener I'm
creating was getting destroyed before the widgets, resulting in it getting
notified post-destruction).

On 2012/06/18 20:32:09, Ben Goodger (Google) wrote:
> child HWNDs can receive focus (and indeed must be for key events to be
> received by them, e.g. RenderWidgetHostViewWin) but only the toplevel can
> be activated.
> 
> -Ben
> 
> On Mon, Jun 18, 2012 at 12:25 PM, <mailto:derat@chromium.org> wrote:
> 
> >
> > https://chromiumcodereview.**appspot.com/10562025/diff/**
> >
>
6004/ui/views/focus/focus_**manager_unittest.cc<https://chromiumcodereview.appspot.com/10562025/diff/6004/ui/views/focus/focus_manager_unittest.cc>
> > File ui/views/focus/focus_manager_**unittest.cc (right):
> >
> > https://chromiumcodereview.**appspot.com/10562025/diff/**
> >
>
6004/ui/views/focus/focus_**manager_unittest.cc#newcode122<https://chromiumcodereview.appspot.com/10562025/diff/6004/ui/views/focus/focus_manager_unittest.cc#newcode122>
> > ui/views/focus/focus_manager_**unittest.cc:122: TEST_F(FocusManagerTest,
> > WidgetFocusChangeListener) {
> > This test times out on non-Aura Windows.  I know nothing about the
> > Windows focus model -- is it even possible for child native views to
> > receive the focus, or can only toplevel ones receive it?  (The comment
> > in NativeWidgetAura::OnFocus() suggests that the latter may be the
> > case.)
> >
> > Recommendations for how to make this test relevant / passing on Windows,
> > or should I just ifdef it to be Aura-only?
> >
> >
>
https://chromiumcodereview.**appspot.com/10562025/%3Chttps://chromiumcoderevi...>
> >

[commit-bot: I haz the power, 2012-06-18 21:44:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/10562025/6004

[commit-bot: I haz the power, 2012-06-18 23:27:42]

Change committed as 142858