[MacViews] Fix AccessiblePaneViewTest.SetPaneFocusAndRestore.

ui/views/accessible_pane_view_unittest.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/accessible_pane_view.h"
 
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
 
+#if defined(OS_MACOSX)
+#include "ui/base/test/scoped_fake_nswindow_focus.h"
+#endif
+
 namespace views {
 
 // TODO(alicet): bring pane rotation into views and add tests.
 //               See browser_view.cc for details.
 
 typedef ViewsTestBase AccessiblePaneViewTest;
 
 class TestBarView : public AccessiblePaneView,
                     public ButtonListener {
  public:
@@ skipping 71 matching lines @@
                test_view->GetWidget()->GetFocusManager()->GetFocusedView());
   EXPECT_EQ(test_view->child_button(),
             test_view->GetWidget()->GetFocusManager()->GetFocusedView());
   widget->CloseNow();
   widget.reset();
 }
 
 // This test will not work properly in Windows because it uses ::GetNextWindow
 // on deactivate which is rather unpredictable where the focus will land.
 TEST_F(AccessiblePaneViewTest, SetPaneFocusAndRestore) {
+#if defined(OS_MACOSX)
+  // On Aura platforms, this test creates Ash windows and only interacts with
+  // the Ash window manager. On Mac, it creates native windows, but since unit
+  // tests cannot gain key status, fake it out here.
+  ui::test::ScopedFakeNSWindowKeyStatus fake_key;
+#endif
+
   View* test_view_main = new View();
   scoped_ptr<Widget> widget_main(new Widget());
   Widget::InitParams params_main = CreateParams(Widget::InitParams::TYPE_POPUP);
   params_main.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
   params_main.bounds = gfx::Rect(0, 0, 20, 20);
   widget_main->Init(params_main);
   View* root_main = widget_main->GetRootView();
   root_main->AddChildView(test_view_main);
   widget_main->Activate();

[tapted, 2015/08/05 07:07:17]

I think this should fail since widget->CanActivate returns false, no?

[jackhou1, 2015/08/06 00:22:30]

CanActivate() returns false on the other platforms too, it doesn't seem to block
Activate().

[tapted, 2015/08/06 01:21:17]

I think this is a bug though. other platforms should obey CanActivate

[jackhou1, 2015/08/06 01:51:35]

I meant that Activate seems to unconditionally activate, and callers are
expected to check CanActivate first. On Mac, Activate calls through to
-makeKeyWindow which checks -canBecomeKeyWindow which calls CanActivate, but on
Ash, CanActivate is called before showing a Widget, or when focus changes.

[tapted, 2015/08/06 03:01:00]

I think this is only true on ChromeOS (and possibly still a bug). On Windows,
HWNDMessageHandler::OnMouseActivate(..) checks delegate_->CanActivate() and may
return `::MA_NOACTIVATEANDEAT`.

We should get to the bottom of why CanActivate is false. Ideally the faker
should be checking canBecomeKeyWindow (removing the faker and running an
interactive test shouldn't change the behaviour).

It might be something simple like adding 

params_main.activatable = ACTIVATABLE_YES;

before widget_main->Init(..);

But, I'm actually not sure why CanActivate() is false. InitParams::TYPE_POPUP
defaults to can_activate = true in Widget::Init

Maybe the reason is that the Widget is never actually shown?

[jackhou1, 2015/08/06 06:10:57]

I think the test still works because it only creates Ash widgets.
 
Done.
 
Yeah, that works.
You may have mis-read the logic. It's only activatable if it's *not* TYPE_POPUP
or any of those other types.

[tapted, 2015/08/06 06:49:12]

Ah, so it is. Perhaps comment above the `= ACTIVATABLE_YES` line like, "By
default TYPE_POPUP is not activatable". It should be mentioned in the CL
description too, like

The test was also failing because Mac checks WidgetDelegate::CanActivate() when
activating whereas NativeWidgetAura defers this to `ShouldActivate` which is
only checked by wm::BaseFocusRules

[jackhou1, 2015/08/06 07:35:02]

Done.

   test_view_main->GetFocusManager()->SetFocusedView(test_view_main);
   EXPECT_TRUE(widget_main->IsActive());
   EXPECT_TRUE(test_view_main->HasFocus());
 
   TestBarView* test_view_bar = new TestBarView();
   scoped_ptr<Widget> widget_bar(new Widget());
   Widget::InitParams params_bar = CreateParams(Widget::InitParams::TYPE_POPUP);
   params_bar.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
   params_bar.bounds = gfx::Rect(50, 50, 650, 650);
   widget_bar->Init(params_bar);
   View* root_bar = widget_bar->GetRootView();
   root_bar->AddChildView(test_view_bar);
   widget_bar->Show();
   widget_bar->Activate();
 
   // Set pane focus succeeds, focus on child.
   EXPECT_TRUE(test_view_bar->SetPaneFocusAndFocusDefault());
   EXPECT_FALSE(test_view_main->HasFocus());
   EXPECT_FALSE(widget_main->IsActive());
   EXPECT_EQ(test_view_bar, test_view_bar->GetPaneFocusTraversable());
   EXPECT_EQ(test_view_bar->child_button(),
             test_view_bar->GetWidget()->GetFocusManager()->GetFocusedView());
 
+  // Esc should deactivate the widget.
+  // Widget::Deactivate is not supported on Mac.

[tapted, 2015/08/06 03:01:00]

I think it's more correct to say that Deactivation can only be properly relied
on in ChromeOS. E.g. on Windows "Deactivate()" is just
find-some-other-window-and-activate-it (if you're the "last" window you can't be
deactivated).

Here, I think it's fine to skip on Mac because set_allow_deactivate_on_esc(true)
is only called from src/ash, but we can perhaps reflect that in the #if (i.e.
!defined(OS_MACOSX) || defined(USE_ASH)

[jackhou1, 2015/08/06 06:10:57]

Done.

+#if !defined(OS_MACOSX)
   test_view_bar->AcceleratorPressed(test_view_bar->escape_key());
   EXPECT_TRUE(widget_main->IsActive());
   EXPECT_FALSE(widget_bar->IsActive());
+#endif
 
   widget_bar->CloseNow();
   widget_bar.reset();
 
   widget_main->CloseNow();
   widget_main.reset();
 }
 
 TEST_F(AccessiblePaneViewTest, TwoSetPaneFocus) {
   TestBarView* test_view = new TestBarView();
@@ skipping 65 matching lines @@
             test_view->GetWidget()->GetFocusManager()->GetFocusedView());
 
   // ESC
   test_view->AcceleratorPressed(test_view->escape_key());
   EXPECT_EQ(original_test_view->third_child_button(),
             test_view->GetWidget()->GetFocusManager()->GetFocusedView());
   widget->CloseNow();
   widget.reset();
 }
 }  // namespace views