Introduce "Preferred" accelerators

ash/accelerators/accelerator_controller_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 "ash/accelerators/accelerator_controller.h"
 
 #include "ash/accelerators/accelerator_table.h"
 #include "ash/accessibility_delegate.h"
 #include "ash/ash_switches.h"
 #include "ash/display/display_manager.h"
 #include "ash/ime_control_delegate.h"
 #include "ash/screen_util.h"
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
 #include "ash/system/brightness_control_delegate.h"
 #include "ash/system/keyboard_brightness/keyboard_brightness_control_delegate.h"
 #include "ash/system/tray/system_tray_delegate.h"
 #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"
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window.h"
 #include "ui/events/event.h"
 #include "ui/events/event_processor.h"
 #include "ui/events/test/event_generator.h"
 #include "ui/gfx/screen.h"
 #include "ui/views/widget/widget.h"
@@ skipping 1014 matching lines @@
   const ui::Accelerator shift_alt_return_press(
       ui::VKEY_RETURN,
       ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN);
   EXPECT_FALSE(GetController()->Process(shift_alt_return_press));
   const ui::Accelerator shift_alt_space_press(
       ui::VKEY_SPACE,
       ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN);
   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;
   for (size_t i = 0 ; i < kAcceleratorDataLength; ++i)
     all_actions.insert(kAcceleratorData[i].action);
 #if !defined(NDEBUG)
   std::set<AcceleratorAction> all_desktop_actions;
   for (size_t i = 0 ; i < kDesktopAcceleratorDataLength; ++i)
     all_desktop_actions.insert(kDesktopAcceleratorData[i].action);
 #endif
@@ skipping 155 matching lines @@
     window.reset(CreateTestWindowInShellWithBounds(gfx::Rect(5, 5, 20, 20)));
     wm::ActivateWindow(window.get());
     GetController()->PerformAction(WINDOW_MINIMIZE, dummy);
     delegate->TriggerAccessibilityAlert(A11Y_ALERT_NONE);
     GetController()->PerformAction(kActionsNeedingWindow[i], dummy);
     EXPECT_NE(delegate->GetLastAccessibilityAlert(), A11Y_ALERT_WINDOW_NEEDED);
   }
 }
 
 }  // namespace ash