Toggle maximize key does not repeat anymore. Fixed support for non-repeatable accelerators.

Before, while holding toggle-maximize (F4) button, we were toggling continuosly causing screen flickering. To fix that, TOGGLE_MAXIMIZED was replaced by TOGGLE_MAXIMIZED_PRESSED and TOGGLE_MAXIMIZED_RELEASED accelerators.

Also, filtering of releasing events has been removed from AcceleratorManager. Now it should be done in AcceleratorTarget. In this case it is AcceleratorController (which is a target for all the accelerators registered threw it). As a result suppressing release keys for NEXT_IME accelerator has been moved from tricky AcceleratorManager::ShouldHandle to NEXT_IME implementation in the AcceleratorController.

Finally, AcceleratorManagerContext class has been introduced, which contains state of AcceleratorManager. This can be accessed to get additional information, such as last_event_type. This is used in NEXT_IME handler.

Ideally, the AcceleratorManagerContext reference could be passed to AcceleratorTarget::AcceleratorPressed, however it would require changes in tens of files, so it is left for another CL.

BUG=135284
TEST=ash_unittests

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

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

5 files changed, 56 insertions, 14 deletions

diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
index 74cce7b..fc050a1 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -332,7 +332,8 @@ bool HandlePrintWindowHierarchy() {
 // AcceleratorController, public:
 
 AcceleratorController::AcceleratorController()
-    : accelerator_manager_(new ui::AcceleratorManager) {
+    : accelerator_manager_(new ui::AcceleratorManager),
+      toggle_maximized_suppressed_(false) {
   Init();
 }
 
@@ -393,9 +394,6 @@ bool AcceleratorController::IsReservedAccelerator(
   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())
@@ -425,6 +423,10 @@ bool AcceleratorController::PerformAction(int action,
   }
   const ui::KeyboardCode key_code = accelerator.key_code();
 
+  const ui::AcceleratorManagerContext& context =
+      accelerator_manager_->GetContext();
+  const ui::EventType last_event_type = context.GetLastEventType();
+
   // You *MUST* return true when some action is performed. Otherwise, this
   // function might be called *twice*, via BrowserView::PreHandleKeyboardEvent
   // and BrowserView::HandleKeyboardEvent, for a single accelerator press.
@@ -513,6 +515,11 @@ bool AcceleratorController::PerformAction(int action,
       ash::Shell::GetInstance()->ToggleAppList();
       return true;
     case DISABLE_CAPS_LOCK:
+      // See: case NEXT_IME.
+      if (last_event_type == ui::ET_KEY_RELEASED) {
+        // We totally ignore this accelerator.
+        return false;
+      }
       if (shell->caps_lock_delegate()->IsCapsLockEnabled())
         shell->caps_lock_delegate()->SetCapsLockEnabled(false);
       return true;
@@ -575,6 +582,14 @@ bool AcceleratorController::PerformAction(int action,
     case SHOW_TASK_MANAGER:
       return HandleShowTaskManager();
     case NEXT_IME:
+      // This check is necessary e.g. not to process the Shift+Alt+
+      // ET_KEY_RELEASED accelerator for Chrome OS (see ash/accelerators/
+      // accelerator_controller.cc) when Shift+Alt+Tab is pressed and then Tab
+      // is released.
+      if (last_event_type == ui::ET_KEY_RELEASED) {
+        // We totally ignore this accelerator.
+        return false;
+      }
       if (ime_control_delegate_.get())
         return ime_control_delegate_->HandleNextIme();
       break;
@@ -660,7 +675,12 @@ bool AcceleratorController::PerformAction(int action,
       }
       break;
     }
-    case TOGGLE_MAXIMIZED: {
+    case TOGGLE_MAXIMIZED_PRESSED: {
+      // We do not want to toggle maximization on the acceleration key
+      // repeating.
+      if (toggle_maximized_suppressed_)
+        return true;
+      toggle_maximized_suppressed_ = true;
       if (key_code == ui::VKEY_F4 && shell->delegate()) {
         shell->delegate()->RecordUserMetricsAction(
             UMA_ACCEL_MAXIMIZE_RESTORE_F4);
@@ -680,6 +700,10 @@ bool AcceleratorController::PerformAction(int action,
         wm::MaximizeWindow(window);
       return true;
     }
+    case TOGGLE_MAXIMIZED_RELEASED: {
+      toggle_maximized_suppressed_ = false;
+      return true;
+    }
     case WINDOW_POSITION_CENTER: {
       aura::Window* window = wm::GetActiveWindow();
       if (window) {
diff --git a/ash/accelerators/accelerator_controller.h b/ash/accelerators/accelerator_controller.h
index 54be6d53..fb3cb7d 100644
--- a/ash/accelerators/accelerator_controller.h
+++ b/ash/accelerators/accelerator_controller.h
@@ -118,6 +118,9 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget {
   // Reserved actions. See accelerator_table.h for details.
   std::set<int> reserved_actions_;
 
+  // If toggling maximized is suppressed until the key release event.
+  bool toggle_maximized_suppressed_;
+
   DISALLOW_COPY_AND_ASSIGN(AcceleratorController);
 };
 
diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc
index a7f3e19..f31510f 100644
--- a/ash/accelerators/accelerator_controller_unittest.cc
+++ b/ash/accelerators/accelerator_controller_unittest.cc
@@ -450,23 +450,33 @@ TEST_F(AcceleratorControllerTest, WindowSnap) {
   {
     gfx::Rect normal_bounds = window->bounds();
 
-    GetController()->PerformAction(TOGGLE_MAXIMIZED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_RELEASED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_PRESSED, dummy);
     EXPECT_TRUE(wm::IsWindowMaximized(window.get()));
     EXPECT_NE(normal_bounds.ToString(), window->bounds().ToString());
 
-    GetController()->PerformAction(TOGGLE_MAXIMIZED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_PRESSED, dummy);
+    EXPECT_TRUE(wm::IsWindowMaximized(window.get()));
+    EXPECT_NE(normal_bounds.ToString(), window->bounds().ToString());
+
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_RELEASED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_PRESSED, dummy);
     EXPECT_FALSE(wm::IsWindowMaximized(window.get()));
     EXPECT_EQ(normal_bounds.ToString(), window->bounds().ToString());
 
-    GetController()->PerformAction(TOGGLE_MAXIMIZED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_RELEASED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_PRESSED, dummy);
     GetController()->PerformAction(WINDOW_SNAP_LEFT, dummy);
     EXPECT_FALSE(wm::IsWindowMaximized(window.get()));
 
-    GetController()->PerformAction(TOGGLE_MAXIMIZED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_RELEASED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_PRESSED, dummy);
     GetController()->PerformAction(WINDOW_SNAP_RIGHT, dummy);
     EXPECT_FALSE(wm::IsWindowMaximized(window.get()));
 
-    GetController()->PerformAction(TOGGLE_MAXIMIZED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_RELEASED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_PRESSED, dummy);
+    GetController()->PerformAction(TOGGLE_MAXIMIZED_RELEASED, dummy);
     EXPECT_TRUE(wm::IsWindowMaximized(window.get()));
     GetController()->PerformAction(WINDOW_MINIMIZE, dummy);
     EXPECT_FALSE(wm::IsWindowMaximized(window.get()));
diff --git a/ash/accelerators/accelerator_table.cc b/ash/accelerators/accelerator_table.cc
index 0a70d9f..ce399d7 100644
--- a/ash/accelerators/accelerator_table.cc
+++ b/ash/accelerators/accelerator_table.cc
@@ -44,7 +44,8 @@ const AcceleratorData kAcceleratorData[] = {
   { true, ui::VKEY_BRIGHTNESS_UP, ui::EF_NONE, BRIGHTNESS_UP },
   { true, ui::VKEY_KBD_BRIGHTNESS_DOWN, ui::EF_NONE, KEYBOARD_BRIGHTNESS_DOWN },
   { true, ui::VKEY_KBD_BRIGHTNESS_UP, ui::EF_NONE, KEYBOARD_BRIGHTNESS_UP },
-  { true, ui::VKEY_F4, ui::EF_NONE, TOGGLE_MAXIMIZED },
+  { true, ui::VKEY_F4, ui::EF_NONE, TOGGLE_MAXIMIZED_PRESSED },
+  { false, ui::VKEY_F4, ui::EF_NONE, TOGGLE_MAXIMIZED_RELEASED },
   { true, ui::VKEY_F4, ui::EF_CONTROL_DOWN, CYCLE_DISPLAY_MODE },
   { true, ui::VKEY_F4, ui::EF_ALT_DOWN, SWAP_PRIMARY_DISPLAY },
   { true, ui::VKEY_L, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, LOCK_SCREEN },
@@ -130,7 +131,8 @@ const AcceleratorData kAcceleratorData[] = {
   { true, ui::VKEY_OEM_4, ui::EF_ALT_DOWN, WINDOW_SNAP_LEFT },
   { true, ui::VKEY_OEM_6, ui::EF_ALT_DOWN, WINDOW_SNAP_RIGHT },
   { true, ui::VKEY_OEM_MINUS, ui::EF_ALT_DOWN, WINDOW_MINIMIZE },
-  { true, ui::VKEY_OEM_PLUS, ui::EF_ALT_DOWN, TOGGLE_MAXIMIZED },
+  { true, ui::VKEY_OEM_PLUS, ui::EF_ALT_DOWN, TOGGLE_MAXIMIZED_PRESSED },
+  { false, ui::VKEY_OEM_PLUS, ui::EF_ALT_DOWN, TOGGLE_MAXIMIZED_RELEASED},
   { true, ui::VKEY_OEM_PLUS, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN,
     WINDOW_POSITION_CENTER },
   { true, ui::VKEY_F2, ui::EF_CONTROL_DOWN, FOCUS_NEXT_PANE },
@@ -178,7 +180,8 @@ const AcceleratorAction kReservedActions[] = {
 #if defined(OS_CHROMEOS)
   POWER_PRESSED,
   POWER_RELEASED,
-  TOGGLE_MAXIMIZED,
+  TOGGLE_MAXIMIZED_PRESSED,
+  TOGGLE_MAXIMIZED_RELEASED,
 #endif
 };
 
diff --git a/ash/accelerators/accelerator_table.h b/ash/accelerators/accelerator_table.h
index a0362324..2b6d211 100644
--- a/ash/accelerators/accelerator_table.h
+++ b/ash/accelerators/accelerator_table.h
@@ -62,8 +62,10 @@ enum AcceleratorAction {
   TAKE_SCREENSHOT,
   TOGGLE_APP_LIST,
   TOGGLE_CAPS_LOCK,
+  TOGGLE_CAPS_LOCK_BY_ALT_LWIN,
   TOGGLE_DESKTOP_BACKGROUND_MODE,
-  TOGGLE_MAXIMIZED,
+  TOGGLE_MAXIMIZED_PRESSED,
+  TOGGLE_MAXIMIZED_RELEASED,
   TOGGLE_ROOT_WINDOW_FULL_SCREEN,
   TOGGLE_SPOKEN_FEEDBACK,
   TOGGLE_WIFI,