Disable the deprecated Alt+Shift for NEXT_IME

ash/accelerators/accelerator_controller.cc

Index: ash/accelerators/accelerator_controller.cc
diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
index 76f68db4b14eced93cb33460747d9f9740ae7ea7..1eebe2fa9363010888ba811872886e4b0acd2982 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -300,40 +300,44 @@ void HandleNewWindow() {
       false /* is_incognito */);
 }
 
+#if defined(OS_CHROMEOS)
+bool CanHandleNextIme(ImeControlDelegate* ime_control_delegate) {
+  return ime_control_delegate && ime_control_delegate->CanCycleIme();
+}
+#else
+// Non-chromeos platforms still use the old NEXT_IME shortcuts.

[oshima, 2016/06/15 21:13:25]

why we need this? ash is chromeos only and win_ash is for testing purpose only.

[afakhry, 2016/06/16 18:40:49]

Removed them entirely. 

Also, since Alt+Shift is triggered on key release, we need to avoid showing the
Deprecation notification for the wrong shortcuts. So, I did the following:
- Added NEXT/PREVIOUS_IME to non-repeatable accelerators.
- Added a function ShouldShowDeprecatedNextImeNotification() which checks for
the previous key being either Alt or Shift.

 bool CanHandleNextIme(ImeControlDelegate* ime_control_delegate,
-                      const ui::Accelerator& previous_accelerator,
-                      bool current_accelerator_is_deprecated) {
-  if (current_accelerator_is_deprecated) {
-    // We only allow next IME to be triggered if the previous is accelerator key
-    // is ONLY either Shift, Alt, Enter or Space.
-    // The first six cases below are to avoid conflicting accelerators that
-    // contain Alt+Shift (like Alt+Shift+Tab or Alt+Shift+S) to trigger next IME
-    // when the wrong order of key sequences is pressed. crbug.com/527154.
-    // The latter two cases are needed for CJK IME users who tend to press Enter
-    // (or Space) and Shift+Alt almost at the same time to commit an IME string
-    // and then switch from the IME to the English layout. This allows these
-    // users to trigger NEXT_IME even if they press Shift+Alt before releasing
-    // Enter. crbug.com/139556.
-    // TODO(nona|mazda): Fix crbug.com/139556 in a cleaner way.
-    const ui::KeyboardCode previous_key_code = previous_accelerator.key_code();
-    switch (previous_key_code) {
-      case ui::VKEY_SHIFT:
-      case ui::VKEY_LSHIFT:
-      case ui::VKEY_RSHIFT:
-      case ui::VKEY_MENU:
-      case ui::VKEY_LMENU:
-      case ui::VKEY_RMENU:
-      case ui::VKEY_RETURN:
-      case ui::VKEY_SPACE:
-        break;
-
-      default:
-        return false;
-    }
+                      const ui::Accelerator& previous_accelerator) {
+  // We only allow next IME to be triggered if the previous is accelerator key
+  // is ONLY either Shift, Alt, Enter or Space.
+  // The first six cases below are to avoid conflicting accelerators that
+  // contain Alt+Shift (like Alt+Shift+Tab or Alt+Shift+S) to trigger next IME
+  // when the wrong order of key sequences is pressed. crbug.com/527154.
+  // The latter two cases are needed for CJK IME users who tend to press Enter
+  // (or Space) and Shift+Alt almost at the same time to commit an IME string
+  // and then switch from the IME to the English layout. This allows these
+  // users to trigger NEXT_IME even if they press Shift+Alt before releasing
+  // Enter. crbug.com/139556.
+  // TODO(nona|mazda): Fix crbug.com/139556 in a cleaner way.
+  const ui::KeyboardCode previous_key_code = previous_accelerator.key_code();
+  switch (previous_key_code) {
+    case ui::VKEY_SHIFT:
+    case ui::VKEY_LSHIFT:
+    case ui::VKEY_RSHIFT:
+    case ui::VKEY_MENU:
+    case ui::VKEY_LMENU:
+    case ui::VKEY_RMENU:
+    case ui::VKEY_RETURN:
+    case ui::VKEY_SPACE:
+      break;
+
+    default:
+      return false;
   }
 
   return ime_control_delegate && ime_control_delegate->CanCycleIme();
 }
+#endif  // defined(OS_CHROMEOS)
 
 void HandleNextIme(ImeControlDelegate* ime_control_delegate) {
   base::RecordAction(UserMetricsAction("Accel_Next_Ime"));
@@ -1016,15 +1020,12 @@ bool AcceleratorController::CanPerformAction(
       return CanHandleNewIncognitoWindow();
     case NEXT_IME: {
 #if defined(OS_CHROMEOS)
-      bool accelerator_is_deprecated =
-          (deprecated_accelerators_.count(accelerator) != 0);
+      return CanHandleNextIme(ime_control_delegate_.get());
 #else
       // On non-chromeos, the NEXT_IME deprecated accelerators are always used.
-      bool accelerator_is_deprecated = true;
+      return CanHandleNextIme(ime_control_delegate_.get(),
+                              previous_accelerator);
 #endif  // defined(OS_CHROMEOS)
-
-      return CanHandleNextIme(ime_control_delegate_.get(), previous_accelerator,
-                              accelerator_is_deprecated);
     }
     case PREVIOUS_IME:
       return CanHandlePreviousIme(ime_control_delegate_.get());