MacViews: Fix ViewTest.HandleAccelerator by faking window activation

MacViews: Fix ViewTest.HandleAccelerator by faking window activation

In fact, fake activation for all tests that use ViewsTestHelper, except
those that run as interactive_ui_tests.

Currently for HandleAccelerator, `EXPECT_TRUE(widget->IsActive())` fails
on Mac since desktop widgets are used rather than Aura widgets, so
activation is asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by just giving it to them
by default.

BUG=592890
Committed: https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0
Cr-Commit-Position: refs/heads/master@{#380917}

[tapted, 2016-03-10 03:10:02]

Description was changed from

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

Currently, `EXPECT_TRUE(widget->IsActive())` fails on Mac since desktop
widgets are used rather than Aura widgets, so activation is asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all platforms).

Also make it easier for a unit test inheriting from ViewsTestBase to
use fake window activation on Mac without using #ifdefs by adding
ViewsTestBase::EnableMacFakeWindowActivation().

BUG=592890
==========

to

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

Currently, `EXPECT_TRUE(widget->IsActive())` fails on Mac since desktop
widgets are used rather than Aura widgets, so activation is
asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms).

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by adding
ViewsTestBase::EnableMacFakeWindowActivation().

Then call EnableMacFakeWindowActivation() in ViewTest.HandleAccelerator
to fix it and make the widget activatable.

BUG=592890
==========

[tapted, 2016-03-10 05:01:33]

Patchset #3 (id:40001) has been deleted

[tapted, 2016-03-10 05:02:29]

Description was changed from

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

Currently, `EXPECT_TRUE(widget->IsActive())` fails on Mac since desktop
widgets are used rather than Aura widgets, so activation is
asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms).

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by adding
ViewsTestBase::EnableMacFakeWindowActivation().

Then call EnableMacFakeWindowActivation() in ViewTest.HandleAccelerator
to fix it and make the widget activatable.

BUG=592890
==========

to

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

Currently, `EXPECT_TRUE(widget->IsActive())` fails on Mac since desktop
widgets are used rather than Aura widgets, so activation is
asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by adding
ViewsTestBase::EnableMacFakeWindowActivation().

Then call EnableMacFakeWindowActivation() in ViewTest.HandleAccelerator
to fix it and make the widget activatable.

BUG=592890
==========

[tapted, 2016-03-10 10:47:37]

Description was changed from

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

Currently, `EXPECT_TRUE(widget->IsActive())` fails on Mac since desktop
widgets are used rather than Aura widgets, so activation is
asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by adding
ViewsTestBase::EnableMacFakeWindowActivation().

Then call EnableMacFakeWindowActivation() in ViewTest.HandleAccelerator
to fix it and make the widget activatable.

BUG=592890
==========

to

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

And make it cleaner for tests to do the fake-activation thing.

Currently, `EXPECT_TRUE(widget->IsActive())` fails on Mac since desktop
widgets are used rather than Aura widgets, so activation is
asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by adding
ViewsTestBase::EnableMacFakeWindowActivation().

Then call EnableMacFakeWindowActivation() in ViewTest.HandleAccelerator
to fix it and make the widget activatable.

BUG=592890
==========

[tapted, 2016-03-10 10:52:41]

tapted@chromium.org changed reviewers:
+ sky@chromium.org

[tapted, 2016-03-10 10:52:42]

Hi sky, please take a look when you get a chance. Thanks!

https://codereview.chromium.org/1782773002/diff/60001/ui/views/view_unittest.cc
File ui/views/view_unittest.cc (right):

https://codereview.chromium.org/1782773002/diff/60001/ui/views/view_unittest....
ui/views/view_unittest.cc:2138:
widget->widget_delegate()->set_can_activate(true);
There's nothing in NativeWidgetAura::Activate that checks Widget::CanActivate
(just NativeWidgetAura::ShowWithWindowState()). And I think the
DefaultActivationClient that tests use don't apply the FocusRules the way the
"real" wm::FocusController ActivationClient does, which would otherwise also
prevent activation. So, I think that's why this test works with regular Aura
widgets without this line.

[sky, 2016-03-10 18:59:16]

https://codereview.chromium.org/1782773002/diff/60001/ui/views/test/views_tes...
File ui/views/test/views_test_helper_mac.mm (right):

https://codereview.chromium.org/1782773002/diff/60001/ui/views/test/views_tes...
ui/views/test/views_test_helper_mac.mm:51: faked_focus_.reset(new
ui::test::ScopedFakeNSWindowFocus);
Is there a reason not to do this always? By that I mean move this code to the
constructor?

[tapted, 2016-03-11 03:20:27]

Patchset #5 (id:100001) has been deleted

[tapted, 2016-03-11 09:49:41]

Description was changed from

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

And make it cleaner for tests to do the fake-activation thing.

Currently, `EXPECT_TRUE(widget->IsActive())` fails on Mac since desktop
widgets are used rather than Aura widgets, so activation is
asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by adding
ViewsTestBase::EnableMacFakeWindowActivation().

Then call EnableMacFakeWindowActivation() in ViewTest.HandleAccelerator
to fix it and make the widget activatable.

BUG=592890
==========

to

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

In fact, fake activation for all tests that use ViewsTestHelper, except
those that run as interactive_ui_tests.

Currently for HandleAccelerator, `EXPECT_TRUE(widget->IsActive())` fails
on Mac since desktop widgets are used rather than Aura widgets, so
activation is asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by just giving it to them
by default.

BUG=592890
==========

[tapted, 2016-03-11 09:56:20]

https://codereview.chromium.org/1782773002/diff/60001/ui/views/test/views_tes...
File ui/views/test/views_test_helper_mac.mm (right):

https://codereview.chromium.org/1782773002/diff/60001/ui/views/test/views_tes...
ui/views/test/views_test_helper_mac.mm:51: faked_focus_.reset(new
ui::test::ScopedFakeNSWindowFocus);
On 2016/03/10 18:59:16, sky wrote:
> Is there a reason not to do this always? By that I mean move this code to the
> constructor?

Done (see what you think).

This has actually crossed my mind a few times already.. I don't think
interactive_ui_tests should get it so there's some non-fake coverage of
activation flows. Also at least one  test --
NativeWidgetMacInteractiveUITest.ParentWindowTrafficLights -- would regress
since there's some native behaviour that we can't fake. But apart from that, I
think I was just being conservative.

So I've explored how this would look, including logic to prevent the faker
getting installed for interactive_ui_tests - ptal.

[sky, 2016-03-11 16:16:17]

LGTM

[tapted, 2016-03-14 00:40:12]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2016-03-14 00:40:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1782773002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1782773002/120001

[commit-bot: I haz the power, 2016-03-14 02:00:50]

Description was changed from

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

In fact, fake activation for all tests that use ViewsTestHelper, except
those that run as interactive_ui_tests.

Currently for HandleAccelerator, `EXPECT_TRUE(widget->IsActive())` fails
on Mac since desktop widgets are used rather than Aura widgets, so
activation is asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by just giving it to them
by default.

BUG=592890
==========

to

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

In fact, fake activation for all tests that use ViewsTestHelper, except
those that run as interactive_ui_tests.

Currently for HandleAccelerator, `EXPECT_TRUE(widget->IsActive())` fails
on Mac since desktop widgets are used rather than Aura widgets, so
activation is asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by just giving it to them
by default.

BUG=592890
==========

[commit-bot: I haz the power, 2016-03-14 02:00:51]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-03-14 02:02:12]

Description was changed from

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

In fact, fake activation for all tests that use ViewsTestHelper, except
those that run as interactive_ui_tests.

Currently for HandleAccelerator, `EXPECT_TRUE(widget->IsActive())` fails
on Mac since desktop widgets are used rather than Aura widgets, so
activation is asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by just giving it to them
by default.

BUG=592890
==========

to

==========
MacViews: Fix ViewTest.HandleAccelerator by faking window activation

In fact, fake activation for all tests that use ViewsTestHelper, except
those that run as interactive_ui_tests.

Currently for HandleAccelerator, `EXPECT_TRUE(widget->IsActive())` fails
on Mac since desktop widgets are used rather than Aura widgets, so
activation is asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by just giving it to them
by default.

BUG=592890

Committed: https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0
Cr-Commit-Position: refs/heads/master@{#380917}
==========

[commit-bot: I haz the power, 2016-03-14 02:02:13]

Patchset 5 (id:??) landed as
https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0
Cr-Commit-Position: refs/heads/master@{#380917}