Introduce "Preferred" accelerators, which may be processed by fullscreen, but...

Introduce "Preferred" accelerators, which may be processed by fullscreen, but will otherwise be processed first.

BUG=398905
TEST=covered by unit tests

Review URL: https://codereview.chromium.org/582143004

Cr-Commit-Position: refs/heads/master@{#295823}

ash/accelerators/accelerator_controller.cc

@@ -782,6 +782,8 @@ void AcceleratorController::Init() {
     actions_allowed_at_lock_screen_.insert(kActionsAllowedAtLockScreen[i]);
   for (size_t i = 0; i < kActionsAllowedAtModalWindowLength; ++i)
     actions_allowed_at_modal_window_.insert(kActionsAllowedAtModalWindow[i]);
+  for (size_t i = 0; i < kPreferredActionsLength; ++i)
+    preferred_actions_.insert(kPreferredActions[i]);
   for (size_t i = 0; i < kReservedActionsLength; ++i)
     reserved_actions_.insert(kReservedActions[i]);
   for (size_t i = 0; i < kNonrepeatableActionsLength; ++i)
@@ -840,7 +842,20 @@ bool AcceleratorController::IsRegistered(
   return accelerator_manager_->GetCurrentTarget(accelerator) != NULL;
 }
 
-bool AcceleratorController::IsReservedAccelerator(
+bool AcceleratorController::IsPreferred(
+    const ui::Accelerator& accelerator) const {
+  const ui::Accelerator remapped_accelerator = ime_control_delegate_.get() ?
+      ime_control_delegate_->RemapAccelerator(accelerator) : accelerator;
+
+  std::map<ui::Accelerator, int>::const_iterator iter =
+      accelerators_.find(remapped_accelerator);
+  if (iter == accelerators_.end())
+    return false;  // not an accelerator.
+
+  return preferred_actions_.find(iter->second) != preferred_actions_.end();
+}
+
+bool AcceleratorController::IsReserved(
     const ui::Accelerator& accelerator) const {
   const ui::Accelerator remapped_accelerator = ime_control_delegate_.get() ?
       ime_control_delegate_->RemapAccelerator(accelerator) : accelerator;

ash/accelerators/accelerator_controller.h

@@ -77,8 +77,14 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget {
   // Returns true if the |accelerator| is registered.
   bool IsRegistered(const ui::Accelerator& accelerator) const;
 
-  // Returns true if the |accelerator| is one of the |reserved_actions_|.
-  bool IsReservedAccelerator(const ui::Accelerator& accelerator) const;
+  // Returns true if the |accelerator| is preferred. A preferred accelerator
+  // is handled before being passed to an window/web contents, unless
+  // the window is in fullscreen state.
+  bool IsPreferred(const ui::Accelerator& accelerator) const;
+
+  // Returns true if the |accelerator| is reserved. A reserved accelerator
+  // is always handled and will never be passed to an window/web contents.
+  bool IsReserved(const ui::Accelerator& accelerator) const;
 
   // Performs the specified action. The |accelerator| may provide additional
   // data the action needs. Returns whether an action was performed
@@ -164,6 +170,8 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget {
   std::set<int> actions_allowed_at_lock_screen_;
   // Actions allowed when a modal window is up.
   std::set<int> actions_allowed_at_modal_window_;
+  // Preferred actions. See accelerator_table.h for details.
+  std::set<int> preferred_actions_;
   // Reserved actions. See accelerator_table.h for details.
   std::set<int> reserved_actions_;
   // Actions which will not be repeated while holding the accelerator key.

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,23 +1060,99 @@ 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()->IsPreferred(
+      ui::Accelerator(ui::VKEY_PRINT, ui::EF_NONE)));
+  EXPECT_FALSE(GetController()->IsReserved(
+      ui::Accelerator(ui::VKEY_TAB, ui::EF_NONE)));
+  EXPECT_FALSE(GetController()->IsPreferred(
       ui::Accelerator(ui::VKEY_TAB, ui::EF_NONE)));
-  EXPECT_FALSE(GetController()->IsReservedAccelerator(
+  EXPECT_FALSE(GetController()->IsReserved(
       ui::Accelerator(ui::VKEY_A, ui::EF_NONE)));
+  EXPECT_FALSE(GetController()->IsPreferred(
+      ui::Accelerator(ui::VKEY_A, ui::EF_NONE)));
+}
+
+namespace {
+
+class PreferredReservedAcceleratorsTest : public test::AshTestBase {
+ public:
+  PreferredReservedAcceleratorsTest() {}
+  virtual ~PreferredReservedAcceleratorsTest() {}
+
+  // 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(PreferredReservedAcceleratorsTest);
+};
+
+}  // namespace
+
+TEST_F(PreferredReservedAcceleratorsTest, AcceleratorsWithFullscreen) {
+  aura::Window* w1 = CreateTestWindowInShellWithId(0);
+  aura::Window* w2 = CreateTestWindowInShellWithId(1);
+  wm::ActivateWindow(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();
+#if defined(OS_CHROMEOS)
+  // 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());
+#endif
+
+  // A fullscreen window can consume ALT-TAB (preferred).
+  ASSERT_EQ(w1, wm::GetActiveWindow());
+  generator.PressKey(ui::VKEY_TAB, ui::EF_ALT_DOWN);
+  ASSERT_EQ(w1, wm::GetActiveWindow());
+  ASSERT_NE(w2, wm::GetActiveWindow());
+
+  // 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());
+
+  EXPECT_EQ(w1, wm::GetActiveWindow());
+  generator.PressKey(ui::VKEY_TAB, ui::EF_ALT_DOWN);
+  ASSERT_NE(w1, wm::GetActiveWindow());
+  ASSERT_EQ(w2, wm::GetActiveWindow());
 }
 
 #if defined(OS_CHROMEOS)

ash/accelerators/accelerator_delegate.cc

@@ -59,26 +59,28 @@ bool AcceleratorDelegate::ShouldProcessAcceleratorNow(
       root_windows.end())
     return true;
 
-  // A full screen window should be able to handle all key events including the
-  // reserved ones.
   aura::Window* top_level = ::wm::GetToplevelWindow(target);
+  Shell* shell = Shell::GetInstance();
+
+  // Reserved accelerators (such as Power button) always have a prority.
+  if (shell->accelerator_controller()->IsReserved(accelerator))
+    return true;
 
+  // A full screen window has a right to handle all key events including the
+  // reserved ones.
   if (top_level && wm::GetWindowState(top_level)->IsFullscreen()) {
-    // TODO(yusukes): On Chrome OS, only browser and flash windows can be full
-    // screen. Launching an app in "open full-screen" mode is not supported yet.
-    // That makes the IsWindowFullscreen() check above almost meaningless
-    // because a browser and flash window do handle Ash accelerators anyway
-    // before they're passed to a page or flash content.
+    // On ChromeOS, fullscreen windows are either browser or apps, which
+    // send key events to a web content first, then will process keys
+    // if the web content didn't consume them.
     return false;
   }
 
-  if (Shell::GetInstance()->GetAppListTargetVisibility())
+  // Handle preferred accelerators (such as ALT-TAB) before sending
+  // to the target.
+  if (shell->accelerator_controller()->IsPreferred(accelerator))
     return true;
 
-  // Unless |target| is in the full screen state, handle reserved accelerators
-  // such as Alt+Tab now.
-  return Shell::GetInstance()->accelerator_controller()->IsReservedAccelerator(
-      accelerator);
+  return shell->GetAppListTargetVisibility();
 }
 
 }  // namespace ash

ash/accelerators/accelerator_table.cc

@@ -231,13 +231,20 @@ const AcceleratorData kDebugAcceleratorData[] = {
 
 const size_t kDebugAcceleratorDataLength = arraysize(kDebugAcceleratorData);
 
-const AcceleratorAction kReservedActions[] = {
+const AcceleratorAction kPreferredActions[] = {
   // Window cycling accelerators.
   CYCLE_BACKWARD_MRU,  // Shift+Alt+Tab
   CYCLE_FORWARD_MRU,  // Alt+Tab
+};
+
+const size_t kPreferredActionsLength = arraysize(kPreferredActions);
+
+const AcceleratorAction kReservedActions[] = {
 #if defined(OS_CHROMEOS)
   POWER_PRESSED,
   POWER_RELEASED,
+#else
+  DUMMY_FOR_RESERVED,
 #endif
 };
 

ash/accelerators/accelerator_table.h

@@ -12,13 +12,18 @@
 
 namespace ash {
 
-// There are four classes of accelerators in Ash:
+// There are five classes of accelerators in Ash:
 //
 // Ash (OS) reserved:
 // * Neither packaged apps nor web pages can cancel.
-// * For example, Alt-Tab window cycling.
+// * For example, power button.
 // * See kReservedActions below.
 //
+// Ash (OS) preferred:
+// * Fullscreen window can consume, but normal window can't.
+// * For example, Alt-Tab window cycling.
+// * See kPreferredActions below.
+//
 // Chrome OS system keys:
 // * For legacy reasons, v1 apps can process and cancel. Otherwise handled
 //   directly by Ash.
@@ -136,6 +141,8 @@ enum AcceleratorAction {
   OPEN_FILE_MANAGER,
   SWITCH_TO_NEXT_USER,
   SWITCH_TO_PREVIOUS_USER,
+#else
+  DUMMY_FOR_RESERVED,
 #endif
 };
 
@@ -163,6 +170,10 @@ ASH_EXPORT extern const size_t kDebugAcceleratorDataLength;
 
 // Actions that should be handled very early in Ash unless the current target
 // window is full-screen.
+ASH_EXPORT extern const AcceleratorAction kPreferredActions[];
+ASH_EXPORT extern const size_t kPreferredActionsLength;
+
+// Actions that are always handled in Ash.
 ASH_EXPORT extern const AcceleratorAction kReservedActions[];
 ASH_EXPORT extern const size_t kReservedActionsLength;