Revert 162240 - See also:

http://code.google.com/p/chromium/issues/detail?id=134093
https://chromiumcodereview.appspot.com/10993067/

This is a better fix than 10993067. This CL undoes that change and moves the fix to the AcceleratorController. This is a more general fix which employs a table of actions that are allowed when in a modal dialog. All other actions are suppressed.

BUG=153077
TEST=System Tray -> Ehternet -> Join Other; while the modal dialog is up, try various ash shortcuts. Observe that the ones are working (like volume up) make sense, but things like new window or moving around windows are inactive.

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

TBR=sschmitz@chromium.org
Review URL: https://codereview.chromium.org/11192016

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

6 files changed, 7 insertions, 210 deletions

diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
index f4e8f26..4817c59 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -359,12 +359,12 @@ void AcceleratorController::Init() {
     actions_allowed_at_lock_screen_.insert(
         kActionsAllowedAtLoginOrLockScreen[i]);
   }
-  for (size_t i = 0; i < kActionsAllowedAtLockScreenLength; ++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)
-    actions_allowed_at_modal_window_.insert(kActionsAllowedAtModalWindow[i]);
-  for (size_t i = 0; i < kReservedActionsLength; ++i)
+  }
+  for (size_t i = 0; i < kReservedActionsLength; ++i) {
     reserved_actions_.insert(kReservedActions[i]);
+  }
 
   RegisterAccelerators(kAcceleratorData, kAcceleratorDataLength);
 
@@ -421,25 +421,18 @@ bool AcceleratorController::PerformAction(int action,
 #if defined(OS_CHROMEOS)
   at_login_screen = shell->delegate() && !shell->delegate()->IsSessionStarted();
 #endif
+  bool at_lock_screen = shell->IsScreenLocked();
+
   if (at_login_screen &&
       actions_allowed_at_login_screen_.find(action) ==
       actions_allowed_at_login_screen_.end()) {
     return false;
   }
-  if (shell->IsScreenLocked() &&
+  if (at_lock_screen &&
       actions_allowed_at_lock_screen_.find(action) ==
       actions_allowed_at_lock_screen_.end()) {
     return false;
   }
-  if (shell->IsModalWindowOpen() &&
-      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 =
diff --git a/ash/accelerators/accelerator_controller.h b/ash/accelerators/accelerator_controller.h
index 672e895..0e2ddf4 100644
--- a/ash/accelerators/accelerator_controller.h
+++ b/ash/accelerators/accelerator_controller.h
@@ -115,8 +115,6 @@ 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_;
-  // Actions allowed when a modal window is up.
-  std::set<int> actions_allowed_at_modal_window_;
   // Reserved actions. See accelerator_table.h for details.
   std::set<int> reserved_actions_;
 
diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc
index 5f11bfb..5482d25 100644
--- a/ash/accelerators/accelerator_controller_unittest.cc
+++ b/ash/accelerators/accelerator_controller_unittest.cc
@@ -1035,147 +1035,5 @@ TEST_F(AcceleratorControllerTest, ReservedAccelerators) {
       ui::Accelerator(ui::VKEY_A, ui::EF_NONE)));
 }
 
-TEST_F(AcceleratorControllerTest, DisallowedAtModalWindow) {
-  std::set<AcceleratorAction> allActions;
-  for (size_t i = 0 ; i < kAcceleratorDataLength; ++i)
-    allActions.insert(kAcceleratorData[i].action);
-  std::set<AcceleratorAction> actionsAllowedAtModalWindow;
-  for (size_t k = 0 ; k < kActionsAllowedAtModalWindowLength; ++k)
-    actionsAllowedAtModalWindow.insert(kActionsAllowedAtModalWindow[k]);
-  for (std::set<AcceleratorAction>::const_iterator it =
-           actionsAllowedAtModalWindow.begin();
-       it != actionsAllowedAtModalWindow.end(); ++it) {
-    EXPECT_FALSE(allActions.find(*it) == allActions.end())
-        << " action from kActionsAllowedAtModalWindow"
-        << " not found in kAcceleratorData. action: " << *it;
-  }
-  scoped_ptr<aura::Window> window(
-      aura::test::CreateTestWindowWithBounds(gfx::Rect(5, 5, 20, 20), NULL));
-  const ui::Accelerator dummy;
-  wm::ActivateWindow(window.get());
-  Shell::GetInstance()->SimulateModalWindowOpenForTesting(true);
-  for (std::set<AcceleratorAction>::const_iterator it = allActions.begin();
-       it != allActions.end(); ++it) {
-    if (actionsAllowedAtModalWindow.find(*it) ==
-        actionsAllowedAtModalWindow.end()) {
-      EXPECT_TRUE(GetController()->PerformAction(*it, dummy))
-          << " for action (disallowed at modal window): " << *it;
-    }
-  }
-#if defined(OS_CHROMEOS)
-  //  Testing of top row (F5-F10) accelerators that should still work
-  //  when a modal window is open
-  //
-  // Screenshot
-  {
-    EXPECT_TRUE(GetController()->Process(
-        ui::Accelerator(ui::VKEY_F5, ui::EF_CONTROL_DOWN)));
-    EXPECT_TRUE(GetController()->Process(
-        ui::Accelerator(ui::VKEY_PRINT, ui::EF_NONE)));
-    EXPECT_TRUE(GetController()->Process(
-        ui::Accelerator(ui::VKEY_F5, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN)));
-    DummyScreenshotDelegate* delegate = new DummyScreenshotDelegate;
-    GetController()->SetScreenshotDelegate(
-        scoped_ptr<ScreenshotDelegate>(delegate).Pass());
-    EXPECT_EQ(0, delegate->handle_take_screenshot_count());
-    EXPECT_TRUE(GetController()->Process(
-        ui::Accelerator(ui::VKEY_F5, ui::EF_CONTROL_DOWN)));
-    EXPECT_EQ(1, delegate->handle_take_screenshot_count());
-    EXPECT_TRUE(GetController()->Process(
-        ui::Accelerator(ui::VKEY_PRINT, ui::EF_NONE)));
-    EXPECT_EQ(2, delegate->handle_take_screenshot_count());
-    EXPECT_TRUE(GetController()->Process(
-        ui::Accelerator(ui::VKEY_F5, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN)));
-    EXPECT_EQ(2, delegate->handle_take_screenshot_count());
-  }
-  // Brightness
-  const ui::Accelerator f6(ui::VKEY_F6, ui::EF_NONE);
-  const ui::Accelerator f7(ui::VKEY_F7, ui::EF_NONE);
-  {
-    EXPECT_FALSE(GetController()->Process(f6));
-    EXPECT_FALSE(GetController()->Process(f7));
-    DummyBrightnessControlDelegate* delegate =
-        new DummyBrightnessControlDelegate(true);
-    GetController()->SetBrightnessControlDelegate(
-        scoped_ptr<BrightnessControlDelegate>(delegate).Pass());
-    EXPECT_FALSE(GetController()->Process(f6));
-    EXPECT_FALSE(GetController()->Process(f7));
-  }
-  EnableInternalDisplay();
-  {
-    EXPECT_FALSE(GetController()->Process(f6));
-    EXPECT_FALSE(GetController()->Process(f7));
-    DummyBrightnessControlDelegate* delegate =
-        new DummyBrightnessControlDelegate(false);
-    GetController()->SetBrightnessControlDelegate(
-        scoped_ptr<BrightnessControlDelegate>(delegate).Pass());
-    EXPECT_EQ(0, delegate->handle_brightness_down_count());
-    EXPECT_FALSE(GetController()->Process(f6));
-    EXPECT_EQ(1, delegate->handle_brightness_down_count());
-    EXPECT_EQ(f6, delegate->last_accelerator());
-    EXPECT_EQ(0, delegate->handle_brightness_up_count());
-    EXPECT_FALSE(GetController()->Process(f7));
-    EXPECT_EQ(1, delegate->handle_brightness_up_count());
-    EXPECT_EQ(f7, delegate->last_accelerator());
-  }
-  {
-    DummyBrightnessControlDelegate* delegate =
-        new DummyBrightnessControlDelegate(true);
-    GetController()->SetBrightnessControlDelegate(
-        scoped_ptr<BrightnessControlDelegate>(delegate).Pass());
-    EXPECT_EQ(0, delegate->handle_brightness_down_count());
-    EXPECT_TRUE(GetController()->Process(f6));
-    EXPECT_EQ(1, delegate->handle_brightness_down_count());
-    EXPECT_EQ(f6, delegate->last_accelerator());
-    EXPECT_EQ(0, delegate->handle_brightness_up_count());
-    EXPECT_TRUE(GetController()->Process(f7));
-    EXPECT_EQ(1, delegate->handle_brightness_up_count());
-    EXPECT_EQ(f7, delegate->last_accelerator());
-  }
-  // Volume
-  const ui::Accelerator f8(ui::VKEY_F8, ui::EF_NONE);
-  const ui::Accelerator f9(ui::VKEY_F9, ui::EF_NONE);
-  const ui::Accelerator f10(ui::VKEY_F10, ui::EF_NONE);
-  {
-    EXPECT_TRUE(GetController()->Process(f8));
-    EXPECT_TRUE(GetController()->Process(f9));
-    EXPECT_TRUE(GetController()->Process(f10));
-    DummyVolumeControlDelegate* delegate =
-        new DummyVolumeControlDelegate(false);
-    ash::Shell::GetInstance()->tray_delegate()->SetVolumeControlDelegate(
-        scoped_ptr<VolumeControlDelegate>(delegate).Pass());
-    EXPECT_EQ(0, delegate->handle_volume_mute_count());
-    EXPECT_FALSE(GetController()->Process(f8));
-    EXPECT_EQ(1, delegate->handle_volume_mute_count());
-    EXPECT_EQ(f8, delegate->last_accelerator());
-    EXPECT_EQ(0, delegate->handle_volume_down_count());
-    EXPECT_FALSE(GetController()->Process(f9));
-    EXPECT_EQ(1, delegate->handle_volume_down_count());
-    EXPECT_EQ(f9, delegate->last_accelerator());
-    EXPECT_EQ(0, delegate->handle_volume_up_count());
-    EXPECT_FALSE(GetController()->Process(f10));
-    EXPECT_EQ(1, delegate->handle_volume_up_count());
-    EXPECT_EQ(f10, delegate->last_accelerator());
-  }
-  {
-    DummyVolumeControlDelegate* delegate = new DummyVolumeControlDelegate(true);
-    ash::Shell::GetInstance()->tray_delegate()->SetVolumeControlDelegate(
-        scoped_ptr<VolumeControlDelegate>(delegate).Pass());
-    EXPECT_EQ(0, delegate->handle_volume_mute_count());
-    EXPECT_TRUE(GetController()->Process(f8));
-    EXPECT_EQ(1, delegate->handle_volume_mute_count());
-    EXPECT_EQ(f8, delegate->last_accelerator());
-    EXPECT_EQ(0, delegate->handle_volume_down_count());
-    EXPECT_TRUE(GetController()->Process(f9));
-    EXPECT_EQ(1, delegate->handle_volume_down_count());
-    EXPECT_EQ(f9, delegate->last_accelerator());
-    EXPECT_EQ(0, delegate->handle_volume_up_count());
-    EXPECT_TRUE(GetController()->Process(f10));
-    EXPECT_EQ(1, delegate->handle_volume_up_count());
-    EXPECT_EQ(f10, delegate->last_accelerator());
-  }
-#endif
-}
-
 }  // namespace test
 }  // namespace ash
diff --git a/ash/accelerators/accelerator_table.cc b/ash/accelerators/accelerator_table.cc
index bd50541..35a60ae 100644
--- a/ash/accelerators/accelerator_table.cc
+++ b/ash/accelerators/accelerator_table.cc
@@ -253,41 +253,4 @@ const AcceleratorAction kActionsAllowedAtLockScreen[] = {
 const size_t kActionsAllowedAtLockScreenLength =
     arraysize(kActionsAllowedAtLockScreen);
 
-const AcceleratorAction kActionsAllowedAtModalWindow[] = {
-  BRIGHTNESS_DOWN,
-  BRIGHTNESS_UP,
-  DISABLE_CAPS_LOCK,
-  EXIT,
-  KEYBOARD_BRIGHTNESS_DOWN,
-  KEYBOARD_BRIGHTNESS_UP,
-  MAGNIFY_SCREEN_ZOOM_IN,
-  MAGNIFY_SCREEN_ZOOM_OUT,
-  MEDIA_NEXT_TRACK,
-  MEDIA_PLAY_PAUSE,
-  MEDIA_PREV_TRACK,
-  NEXT_IME,
-  OPEN_FEEDBACK_PAGE,
-  POWER_PRESSED,
-  POWER_RELEASED,
-  PREVIOUS_IME,
-  SHOW_KEYBOARD_OVERLAY,
-  SWAP_PRIMARY_DISPLAY,
-  SWITCH_IME,
-  TAKE_SCREENSHOT,
-  TAKE_PARTIAL_SCREENSHOT,
-  TOGGLE_CAPS_LOCK,
-  TOGGLE_SPOKEN_FEEDBACK,
-  TOGGLE_WIFI,
-  VOLUME_DOWN,
-  VOLUME_MUTE,
-  VOLUME_UP,
-#if defined(OS_CHROMEOS)
-  CYCLE_DISPLAY_MODE,
-  LOCK_SCREEN,
-#endif
-};
-
-const size_t kActionsAllowedAtModalWindowLength =
-    arraysize(kActionsAllowedAtModalWindow);
-
 }  // namespace ash
diff --git a/ash/accelerators/accelerator_table.h b/ash/accelerators/accelerator_table.h
index 54c5d43..14b4327 100644
--- a/ash/accelerators/accelerator_table.h
+++ b/ash/accelerators/accelerator_table.h
@@ -136,12 +136,6 @@ ASH_EXPORT extern const AcceleratorAction kActionsAllowedAtLockScreen[];
 // The number of elements in kActionsAllowedAtLockScreen.
 ASH_EXPORT extern const size_t kActionsAllowedAtLockScreenLength;
 
-// Actions allowed while a modal window is up.
-ASH_EXPORT extern const AcceleratorAction kActionsAllowedAtModalWindow[];
-
-// The number of elements in kActionsAllowedAtModalWindow.
-ASH_EXPORT extern const size_t kActionsAllowedAtModalWindowLength;
-
 }  // namespace ash
 
 #endif  // ASH_ACCELERATORS_ACCELERATOR_TABLE_H_
diff --git a/ash/accelerators/accelerator_table_unittest.cc b/ash/accelerators/accelerator_table_unittest.cc
index 6467a9b..8ec0645 100644
--- a/ash/accelerators/accelerator_table_unittest.cc
+++ b/ash/accelerators/accelerator_table_unittest.cc
@@ -58,13 +58,4 @@ TEST(AcceleratorTableTest, CheckDuplicatedActionsAllowedAtLoginOrLockScreen) {
   }
 }
 
-TEST(AcceleratorTableTest, CheckDuplicatedActionsAllowedAtModalWindow) {
-  std::set<AcceleratorAction> actions;
-  for (size_t i = 0; i < kActionsAllowedAtModalWindowLength; ++i) {
-    EXPECT_TRUE(actions.insert(kActionsAllowedAtModalWindow[i]).second)
-        << "Duplicated action: " << kActionsAllowedAtModalWindow[i]
-        << " at index: " << i;
-  }
-}
-
 }  // namespace ash