Introduce "Preferred" accelerators

ash/accelerators/accelerator_controller_unittest.cc

Index: ash/accelerators/accelerator_controller_unittest.cc
diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc
index 31ebf9b4e41372d262315ef9e27db36153c2af10..7cdef6b10efb4ed4c8fc07f0de0c6dc128c6f7fb 100644
--- a/ash/accelerators/accelerator_controller_unittest.cc
+++ b/ash/accelerators/accelerator_controller_unittest.cc
@@ -18,11 +18,14 @@
 #include "ash/test/ash_test_base.h"
 #include "ash/test/display_manager_test_api.h"
 #include "ash/test/test_screenshot_delegate.h"
+#include "ash/test/test_session_state_animator.h"
 #include "ash/test/test_shell_delegate.h"
 #include "ash/test/test_volume_control_delegate.h"
 #include "ash/volume_control_delegate.h"
+#include "ash/wm/lock_state_controller.h"
 #include "ash/wm/window_state.h"
 #include "ash/wm/window_util.h"
+#include "ash/wm/wm_event.h"
 #include "base/command_line.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/test/test_window_delegate.h"
@@ -1057,25 +1060,94 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAcceleratorsWorkaround139556) {
   EXPECT_FALSE(GetController()->Process(shift_alt_space_press));
 }
 
-TEST_F(AcceleratorControllerTest, ReservedAccelerators) {
-  // (Shift+)Alt+Tab and Chrome OS top-row keys are reserved.
-  EXPECT_TRUE(GetController()->IsReservedAccelerator(
-      ui::Accelerator(ui::VKEY_TAB, ui::EF_ALT_DOWN)));
-  EXPECT_TRUE(GetController()->IsReservedAccelerator(
-      ui::Accelerator(ui::VKEY_TAB, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN)));
+TEST_F(AcceleratorControllerTest, PreferredReservedAccelerators) {
 #if defined(OS_CHROMEOS)
-  EXPECT_TRUE(GetController()->IsReservedAccelerator(
+  // Power key is reserved on chromeos.
+  EXPECT_TRUE(GetController()->IsReserved(
+      ui::Accelerator(ui::VKEY_POWER, ui::EF_NONE)));
+  EXPECT_FALSE(GetController()->IsPreferred(
       ui::Accelerator(ui::VKEY_POWER, ui::EF_NONE)));
 #endif
-  // Others are not reserved.
-  EXPECT_FALSE(GetController()->IsReservedAccelerator(
+  // ALT+Tab are not reserved but preferred.
+  EXPECT_FALSE(GetController()->IsReserved(
+      ui::Accelerator(ui::VKEY_TAB, ui::EF_ALT_DOWN)));
+  EXPECT_FALSE(GetController()->IsReserved(
+      ui::Accelerator(ui::VKEY_TAB, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN)));
+  EXPECT_TRUE(GetController()->IsPreferred(
+      ui::Accelerator(ui::VKEY_TAB, ui::EF_ALT_DOWN)));
+  EXPECT_TRUE(GetController()->IsPreferred(
+      ui::Accelerator(ui::VKEY_TAB, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN)));
+
+  // Others are not reserved nor preferred
+  EXPECT_FALSE(GetController()->IsReserved(
       ui::Accelerator(ui::VKEY_PRINT, ui::EF_NONE)));
-  EXPECT_FALSE(GetController()->IsReservedAccelerator(
+  EXPECT_FALSE(GetController()->IsReserved(
       ui::Accelerator(ui::VKEY_TAB, ui::EF_NONE)));
-  EXPECT_FALSE(GetController()->IsReservedAccelerator(
+  EXPECT_FALSE(GetController()->IsReserved(
       ui::Accelerator(ui::VKEY_A, ui::EF_NONE)));

[James Cook, 2014/09/19 16:13:15]

How about an EXPECT_FALSE( IsPreferred() ) here also?

[oshima, 2014/09/19 18:55:19]

Done. I was going to add them but apparently forgot them. Thank you for
the catch!

 }
 
+namespace {
+
+class PowerAcceleratorTest : public test::AshTestBase {
+ public:
+  PowerAcceleratorTest() {

[James Cook, 2014/09/19 16:13:15]

nit: match {} style with line below

[oshima, 2014/09/19 18:55:19]

Done.

+  }
+  virtual ~PowerAcceleratorTest() {}
+
+  // test::AshTestBase:
+  virtual void SetUp() OVERRIDE {
+    AshTestBase::SetUp();
+    Shell::GetInstance()->lock_state_controller()->
+        set_animator_for_test(new test::TestSessionStateAnimator);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PowerAcceleratorTest);
+};
+
+} // namespace

[James Cook, 2014/09/19 16:13:15]

nit: two spaces after }

[oshima, 2014/09/19 18:55:19]

Done.

+
+TEST_F(PowerAcceleratorTest, AcceleratorsWithFullsccreen) {

[James Cook, 2014/09/19 16:13:15]

nit: Fullsccreen -> Fullscreen

[oshima, 2014/09/19 18:55:19]

Done.

+  aura::Window* w1 = CreateTestWindowInShellWithId(0);
+  aura::Window* w2 = CreateTestWindowInShellWithId(1);
+  wm::ActivateWindow(w1);
+  ASSERT_EQ(wm::GetActiveWindow(), w1);
+
+  wm::WMEvent fullscreen(wm::WM_EVENT_FULLSCREEN);
+  wm::WindowState* w1_state = wm::GetWindowState(w1);
+  w1_state->OnWMEvent(&fullscreen);
+  ASSERT_TRUE(w1_state->IsFullscreen());
+
+  ui::test::EventGenerator& generator = GetEventGenerator();
+  // Power key (reserved) should always be handled.
+  LockStateController::TestApi test_api(
+      Shell::GetInstance()->lock_state_controller());
+  EXPECT_FALSE(test_api.is_animating_lock());
+  generator.PressKey(ui::VKEY_POWER, ui::EF_NONE);
+  EXPECT_TRUE(test_api.is_animating_lock());
+
+  // A fullscreen can consume ALT-TAB (preferred).

[James Cook, 2014/09/19 16:13:15]

nit: "A fullscreen" -> "A fullscreen window"

[oshima, 2014/09/19 18:55:19]

Done.

+  generator.PressKey(ui::VKEY_TAB, ui::EF_ALT_DOWN);
+  aura::Window* active = wm::GetActiveWindow();
+  ASSERT_EQ(active, w1);

[James Cook, 2014/09/19 16:13:15]

nit: This might be a little clearer if you did something like:

EXPECT_EQ(active, w1);
generator.PressKey(ui::VKEY_TAB, ui::EF_ALT_DOWN);
EXPECT_EQ(active, w1);

just to emphasize that what you're testing is that the active window doesn't
change.  (Yes, I know there's an assert at the top of the function, but it's 15
lines away :-)

[oshima, 2014/09/19 18:55:19]

Done.

+  ASSERT_NE(active, w2);
+
+  // ALT-TAB is non repeatable. Press A to cancel the
+  // repeat record.
+  generator.PressKey(ui::VKEY_A, ui::EF_NONE);
+  generator.ReleaseKey(ui::VKEY_A, ui::EF_NONE);
+
+  // A normal window shouldn't consume preferred accelerator.
+  wm::WMEvent normal(wm::WM_EVENT_NORMAL);
+  w1_state->OnWMEvent(&normal);
+  ASSERT_FALSE(w1_state->IsFullscreen());
+  generator.PressKey(ui::VKEY_TAB, ui::EF_ALT_DOWN);
+  active = wm::GetActiveWindow();
+  EXPECT_NE(w1, active);
+  EXPECT_EQ(w2, active);
+}

[James Cook, 2014/09/19 16:13:15]

nice test. You always write excellent tests.

+
 #if defined(OS_CHROMEOS)
 TEST_F(AcceleratorControllerTest, DisallowedAtModalWindow) {
   std::set<AcceleratorAction> all_actions;