Fix for Ash shortcuts unexpectedly working in system model dialog

ash/accelerators/accelerator_controller.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 <algorithm>
 #include <cmath>
 #include <string>
 
@@ skipping 344 matching lines @@
 void AcceleratorController::Init() {
   for (size_t i = 0; i < kActionsAllowedAtLoginOrLockScreenLength; ++i) {
     actions_allowed_at_login_screen_.insert(
         kActionsAllowedAtLoginOrLockScreen[i]);
     actions_allowed_at_lock_screen_.insert(
         kActionsAllowedAtLoginOrLockScreen[i]);
   }
   for (size_t i = 0; i < kActionsAllowedAtLockScreenLength; ++i) {
     actions_allowed_at_lock_screen_.insert(kActionsAllowedAtLockScreen[i]);
   }
+  for (size_t i = 0; i < kActionsAllowedAtModalWindowLength; ++i) {

[Daniel Erat, 2012/10/16 03:03:50]

nit: not your fault, but i won't object if you want to clean it up here: chrome
style is typically to omit curly brackets for single-line if statements

[sschmitz, 2012/10/16 18:18:23]

Done.

+    actions_allowed_at_modal_window_.insert(kActionsAllowedAtModalWindow[i]);
+  }
   for (size_t i = 0; i < kReservedActionsLength; ++i) {
     reserved_actions_.insert(kReservedActions[i]);
   }
 
   RegisterAccelerators(kAcceleratorData, kAcceleratorDataLength);
 
   if (DebugShortcutsEnabled())
     RegisterAccelerators(kDebugAcceleratorData, kDebugAcceleratorDataLength);
 }
 
@@ skipping 40 matching lines @@
 }
 
 bool AcceleratorController::PerformAction(int action,
                                           const ui::Accelerator& accelerator) {
   ash::Shell* shell = ash::Shell::GetInstance();
   bool at_login_screen = false;
 #if defined(OS_CHROMEOS)
   at_login_screen = shell->delegate() && !shell->delegate()->IsSessionStarted();
 #endif
   bool at_lock_screen = shell->IsScreenLocked();
+  bool at_modal_window = shell->IsModalWindowOpen();

[Daniel Erat, 2012/10/16 03:03:50]

nit: i don't think that there's any reason to create temporary variables for
this and at_lock_screen; you can just move the shell->Is...() calls into the if
conditions that are currently using the variables.  at_login_screen is probably
just a variable because it has the #ifdef-ed chrome os code.

[sschmitz, 2012/10/16 18:18:23]

Done.

 
   if (at_login_screen &&
       actions_allowed_at_login_screen_.find(action) ==
       actions_allowed_at_login_screen_.end()) {
     return false;
   }
   if (at_lock_screen &&
       actions_allowed_at_lock_screen_.find(action) ==
       actions_allowed_at_lock_screen_.end()) {
     return false;
   }
+  if (at_modal_window &&
+      actions_allowed_at_modal_window_.find(action) ==
+      actions_allowed_at_modal_window_.end()) {
+    // Note: we return true. This indicates the shortcut is handled
+    // and will not be passed to the modal window. This is important
+    // for things like Alt+Tab that would cause an undesired effect
+    // in the modal window by cycling through its window elements.
+    return true;
+  }
   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.
   switch (action) {
@@ skipping 436 matching lines @@
     accelerators_.insert(
         std::make_pair(accelerator, accelerators[i].action));
   }
 }
 
 bool AcceleratorController::CanHandleAccelerators() const {
   return true;
 }
 
 }  // namespace ash