Make Alt+Tab accelerator behavior compatible to Windows.

Handle Alt+Tab and Shift+Alt+Tab very early in Ash unless the current target window is in the full screen mode.

BUG=132724
TEST=manual + aura_shell_unittests


Review URL: https://chromiumcodereview.appspot.com/10562028

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@143137 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 86 insertions, 10 deletions

diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
index 3a6ebe3..89af040 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -261,6 +261,9 @@ void AcceleratorController::Init() {
   for (size_t i = 0; i < kActionsAllowedAtLockScreenLength; ++i) {
     actions_allowed_at_lock_screen_.insert(kActionsAllowedAtLockScreen[i]);
   }
+  for (size_t i = 0; i < kReservedActionsLength; ++i) {
+    reserved_actions_.insert(kReservedActions[i]);
+  }
 
   for (size_t i = 0; i < kAcceleratorDataLength; ++i) {
     ui::Accelerator accelerator(kAcceleratorData[i].keycode,
@@ -302,6 +305,22 @@ bool AcceleratorController::IsRegistered(
   return accelerator_manager_->GetCurrentTarget(accelerator) != NULL;
 }
 
+bool AcceleratorController::IsReservedAccelerator(
+    const ui::Accelerator& accelerator) const {
+  const ui::Accelerator remapped_accelerator = ime_control_delegate_.get() ?
+      ime_control_delegate_->RemapAccelerator(accelerator) : accelerator;
+
+  if (!accelerator_manager_->ShouldHandle(remapped_accelerator))
+    return false;
+
+  std::map<ui::Accelerator, int>::const_iterator iter =
+      accelerators_.find(remapped_accelerator);
+  if (iter == accelerators_.end())
+    return false;  // not an accelerator.
+
+  return reserved_actions_.find(iter->second) != reserved_actions_.end();
+}
+
 bool AcceleratorController::PerformAction(int action,
                                           const ui::Accelerator& accelerator) {
   ash::Shell* shell = ash::Shell::GetInstance();
diff --git a/ash/accelerators/accelerator_controller.h b/ash/accelerators/accelerator_controller.h
index 11a0673..c85d549 100644
--- a/ash/accelerators/accelerator_controller.h
+++ b/ash/accelerators/accelerator_controller.h
@@ -59,6 +59,9 @@ 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;
+
   // Performs the specified action. The |accelerator| may provide additional
   // data the action needs. Returns whether an action was performed
   // successfully.
@@ -113,6 +116,8 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget {
   std::set<int> actions_allowed_at_login_screen_;
   // Actions allowed when the screen is locked.
   std::set<int> actions_allowed_at_lock_screen_;
+  // Reserved actions. See accelerator_table.h for details.
+  std::set<int> reserved_actions_;
 
   DISALLOW_COPY_AND_ASSIGN(AcceleratorController);
 };
diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc
index 1b1f895..3fcd8ef 100644
--- a/ash/accelerators/accelerator_controller_unittest.cc
+++ b/ash/accelerators/accelerator_controller_unittest.cc
@@ -875,5 +875,17 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) {
 #endif
 }
 
+TEST_F(AcceleratorControllerTest, ReservedAccelerators) {
+  // (Shift+)Alt+Tab is reserved, while (Shift+)F5 is not.
+  EXPECT_FALSE(GetController()->IsReservedAccelerator(
+      ui::Accelerator(ui::VKEY_F5, ui::EF_NONE)));
+  EXPECT_TRUE(GetController()->IsReservedAccelerator(
+      ui::Accelerator(ui::VKEY_TAB, ui::EF_ALT_DOWN)));
+  EXPECT_FALSE(GetController()->IsReservedAccelerator(
+      ui::Accelerator(ui::VKEY_F5, ui::EF_SHIFT_DOWN)));
+  EXPECT_TRUE(GetController()->IsReservedAccelerator(
+      ui::Accelerator(ui::VKEY_TAB, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN)));
+}
+
 }  // namespace test
 }  // namespace ash
diff --git a/ash/accelerators/accelerator_filter.cc b/ash/accelerators/accelerator_filter.cc
index 21f8a41..01e717f 100644
--- a/ash/accelerators/accelerator_filter.cc
+++ b/ash/accelerators/accelerator_filter.cc
@@ -6,31 +6,47 @@
 
 #include "ash/accelerators/accelerator_controller.h"
 #include "ash/shell.h"
+#include "ash/wm/window_util.h"
 #include "ui/aura/event.h"
 #include "ui/aura/root_window.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/accelerators/accelerator_manager.h"
 
+namespace ash {
 namespace {
 
 const int kModifierFlagMask = (ui::EF_SHIFT_DOWN |
                                ui::EF_CONTROL_DOWN |
                                ui::EF_ALT_DOWN);
 
-// Returns true if an Ash accelerator should be processed now.
-bool ShouldProcessAcceleratorsNow(aura::Window* target) {
+// Returns true if the |accelerator| should be processed now, inside Ash's env
+// event filter.
+bool ShouldProcessAcceleratorsNow(const ui::Accelerator& accelerator,
+                                  aura::Window* target) {
   if (!target)
     return true;
-  if (target == ash::Shell::GetPrimaryRootWindow())
+  if (target == Shell::GetPrimaryRootWindow())
     return true;
-  // Unless |target| is the root window, return false to let the custom focus
-  // manager (see ash/shell.cc) handle Ash accelerators.
-  return false;
+
+  // A full screen window should be able to handle all key events including the
+  // reserved ones.
+  if (wm::IsWindowFullscreen(target)) {
+    // 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.
+    return false;
+  }
+
+  // Unless |target| is in the full screen state, handle reserved accelerators
+  // such as Alt+Tab now.
+  return Shell::GetInstance()->accelerator_controller()->IsReservedAccelerator(
+      accelerator);
 }
 
 }  // namespace
 
-namespace ash {
 namespace internal {
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -52,12 +68,13 @@ bool AcceleratorFilter::PreHandleKeyEvent(aura::Window* target,
     return false;
   if (event->is_char())
     return false;
-  if (!ShouldProcessAcceleratorsNow(target))
-    return false;
 
   ui::Accelerator accelerator(event->key_code(),
                               event->flags() & kModifierFlagMask);
   accelerator.set_type(type);
+
+  if (!ShouldProcessAcceleratorsNow(accelerator, target))
+    return false;
   return Shell::GetInstance()->accelerator_controller()->Process(accelerator);
 }
 
diff --git a/ash/accelerators/accelerator_table.cc b/ash/accelerators/accelerator_table.cc
index 62862ce..b32b2cb 100644
--- a/ash/accelerators/accelerator_table.cc
+++ b/ash/accelerators/accelerator_table.cc
@@ -130,6 +130,13 @@ const AcceleratorData kAcceleratorData[] = {
 
 const size_t kAcceleratorDataLength = arraysize(kAcceleratorData);
 
+const AcceleratorAction kReservedActions[] = {
+  CYCLE_BACKWARD_MRU,
+  CYCLE_FORWARD_MRU,
+};
+
+const size_t kReservedActionsLength = arraysize(kReservedActions);
+
 const AcceleratorAction kActionsAllowedAtLoginOrLockScreen[] = {
   BRIGHTNESS_DOWN,
   BRIGHTNESS_UP,
diff --git a/ash/accelerators/accelerator_table.h b/ash/accelerators/accelerator_table.h
index cfd65bb..25b118e 100644
--- a/ash/accelerators/accelerator_table.h
+++ b/ash/accelerators/accelerator_table.h
@@ -90,6 +90,13 @@ ASH_EXPORT extern const AcceleratorData kAcceleratorData[];
 // The number of elements in kAcceleratorData.
 ASH_EXPORT extern const size_t kAcceleratorDataLength;
 
+// Actions that should be handled very early in Ash unless the current target
+// window is full-screen.
+ASH_EXPORT extern const AcceleratorAction kReservedActions[];
+
+// The number of elements in kReservedActions.
+ASH_EXPORT extern const size_t kReservedActionsLength;
+
 // Actions allowed while user is not signed in or screen is locked.
 ASH_EXPORT extern const AcceleratorAction kActionsAllowedAtLoginOrLockScreen[];
 
diff --git a/ash/accelerators/accelerator_table_unittest.cc b/ash/accelerators/accelerator_table_unittest.cc
index 730d633..8ec0645 100644
--- a/ash/accelerators/accelerator_table_unittest.cc
+++ b/ash/accelerators/accelerator_table_unittest.cc
@@ -38,6 +38,14 @@ TEST(AcceleratorTableTest, CheckDuplicatedAccelerators) {
   }
 }
 
+TEST(AcceleratorTableTest, CheckDuplicatedReservedActions) {
+  std::set<AcceleratorAction> actions;
+  for (size_t i = 0; i < kReservedActionsLength; ++i) {
+    EXPECT_TRUE(actions.insert(kReservedActions[i]).second)
+        << "Duplicated action: " << kReservedActions[i];
+  }
+}
+
 TEST(AcceleratorTableTest, CheckDuplicatedActionsAllowedAtLoginOrLockScreen) {
   std::set<AcceleratorAction> actions;
   for (size_t i = 0; i < kActionsAllowedAtLoginOrLockScreenLength; ++i) {
diff --git a/ui/base/accelerators/accelerator_manager.h b/ui/base/accelerators/accelerator_manager.h
index 9f20a10..0b50ed4 100644
--- a/ui/base/accelerators/accelerator_manager.h
+++ b/ui/base/accelerators/accelerator_manager.h
@@ -71,9 +71,10 @@ class UI_EXPORT AcceleratorManager {
   // Whether the given |accelerator| has a priority handler associated with it.
   bool HasPriorityHandler(const Accelerator& accelerator) const;
 
- private:
+  // Returns false when Process() should never handle the |accelerator|.
   bool ShouldHandle(const Accelerator& accelerator) const;
 
+ private:
   // The accelerators and associated targets.
   typedef std::list<AcceleratorTarget*> AcceleratorTargetList;
   // This construct pairs together a |bool| (denoting whether the list contains