diff options
author | afakhry <afakhry@chromium.org> | 2014-11-21 13:06:16 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-11-21 21:06:42 +0000 |
commit | 5546373f24680054ad4e1eca425b5b1b7291dfe9 (patch) | |
tree | 0a6a169e4256477a15c168038766f5cd3d8fc4c5 | |
parent | f3c56d51dac268caa4186b27eb3f738ce85b8d20 (diff) | |
download | chromium_src-5546373f24680054ad4e1eca425b5b1b7291dfe9.zip chromium_src-5546373f24680054ad4e1eca425b5b1b7291dfe9.tar.gz chromium_src-5546373f24680054ad4e1eca425b5b1b7291dfe9.tar.bz2 |
Regression: Search+Key pops up app launcher
We decided to fix this issue even though it only happens when the
following 'wrong' key sequence is pressed:
- SearchKey [pressed]
- Key [pressed]
- SearchKey [released]
- Key [released]
Resulted wrong behavior: the AppLancher appears.
The Cause:
----------
When the search key is used as a modifier, the following pressed key is
rewritten and consumed. It doesn't get forwarded to
AcceleratorController managed by the AcceleratorManager in the
FocusManager. The AcceleratorController needs to know the previous
Accelerator to determine the correct behavior.
The 'Suggested' Fix:
--------------------
Track the system-wide current and previous key accelerators and require
that the ToggleAppList would only be triggered if LWIN [pressed] is
immediately followed by LWIN [released].
BUG=410835
TEST=manual, ash_unittests
Review URL: https://codereview.chromium.org/727583002
Cr-Commit-Position: refs/heads/master@{#305278}
-rw-r--r-- | ash/accelerators/accelerator_controller.cc | 39 | ||||
-rw-r--r-- | ash/accelerators/accelerator_controller.h | 12 | ||||
-rw-r--r-- | ash/accelerators/accelerator_controller_unittest.cc | 251 | ||||
-rw-r--r-- | ash/accelerators/accelerator_filter_unittest.cc | 6 | ||||
-rw-r--r-- | ash/shell.cc | 3 | ||||
-rw-r--r-- | athena/input/accelerator_manager_impl.cc | 4 | ||||
-rw-r--r-- | athena/input/accelerator_manager_impl.h | 2 | ||||
-rw-r--r-- | chrome/browser/chromeos/accessibility/sticky_keys_browsertest.cc | 31 | ||||
-rw-r--r-- | ui/base/BUILD.gn | 2 | ||||
-rw-r--r-- | ui/base/accelerators/accelerator_history.cc | 30 | ||||
-rw-r--r-- | ui/base/accelerators/accelerator_history.h | 47 | ||||
-rw-r--r-- | ui/base/ui_base.gyp | 2 | ||||
-rw-r--r-- | ui/views/focus/focus_manager.cc | 24 | ||||
-rw-r--r-- | ui/wm/core/accelerator_filter.cc | 10 | ||||
-rw-r--r-- | ui/wm/core/accelerator_filter.h | 7 |
15 files changed, 309 insertions, 161 deletions
diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc index ab70c5e..04a6db3 100644 --- a/ash/accelerators/accelerator_controller.cc +++ b/ash/accelerators/accelerator_controller.cc @@ -429,16 +429,22 @@ bool HandleToggleAppList(ui::KeyboardCode key_code, // If something else was pressed between the Search key (LWIN) // being pressed and released, then ignore the release of the // Search key. - if (key_code == ui::VKEY_LWIN && - (previous_event_type == ui::ET_KEY_RELEASED || - previous_key_code != ui::VKEY_LWIN)) + + if ((key_code != ui::VKEY_BROWSER_SEARCH || + accelerator.type() != ui::ET_KEY_PRESSED) && + (key_code != ui::VKEY_LWIN || + previous_event_type != ui::ET_KEY_PRESSED || + previous_key_code != ui::VKEY_LWIN || + accelerator.type() != ui::ET_KEY_RELEASED)) { return false; + } + if (key_code == ui::VKEY_LWIN) base::RecordAction(base::UserMetricsAction("Accel_Search_LWin")); + // When spoken feedback is enabled, we should neither toggle the list nor // consume the key since Search+Shift is one of the shortcuts the a11y // feature uses. crbug.com/132296 - DCHECK_EQ(ui::VKEY_LWIN, accelerator.key_code()); if (Shell::GetInstance()->accessibility_delegate()-> IsSpokenFeedbackEnabled()) return false; @@ -704,26 +710,14 @@ bool HandleVolumeUp(const ui::Accelerator& accelerator) { #endif // defined(OS_CHROMEOS) -class AutoSet { - public: - AutoSet(ui::Accelerator* scoped, ui::Accelerator new_value) - : scoped_(scoped), new_value_(new_value) {} - ~AutoSet() { *scoped_ = new_value_; } - - private: - ui::Accelerator* scoped_; - const ui::Accelerator new_value_; - - DISALLOW_COPY_AND_ASSIGN(AutoSet); -}; - } // namespace //////////////////////////////////////////////////////////////////////////////// // AcceleratorController, public: AcceleratorController::AcceleratorController() - : accelerator_manager_(new ui::AcceleratorManager) { + : accelerator_manager_(new ui::AcceleratorManager), + accelerator_history_(new ui::AcceleratorHistory) { Init(); } @@ -747,8 +741,6 @@ void AcceleratorController::UnregisterAll(ui::AcceleratorTarget* target) { } bool AcceleratorController::Process(const ui::Accelerator& accelerator) { - AutoSet auto_set(&previous_accelerator_, accelerator); - if (ime_control_delegate_) { return accelerator_manager_->Process( ime_control_delegate_->RemapAccelerator(accelerator)); @@ -830,7 +822,6 @@ bool AcceleratorController::CanHandleAccelerators() const { // AcceleratorController, private: void AcceleratorController::Init() { - previous_accelerator_.set_type(ui::ET_UNKNOWN); for (size_t i = 0; i < kActionsAllowedAtLoginOrLockScreenLength; ++i) { actions_allowed_at_login_screen_.insert( kActionsAllowedAtLoginOrLockScreen[i]); @@ -896,8 +887,10 @@ bool AcceleratorController::PerformAction(AcceleratorAction action, const ui::KeyboardCode key_code = accelerator.key_code(); // Type of the previous accelerator. Used by NEXT_IME and DISABLE_CAPS_LOCK. - const ui::EventType previous_event_type = previous_accelerator_.type(); - const ui::KeyboardCode previous_key_code = previous_accelerator_.key_code(); + const ui::Accelerator& previous_accelerator = + accelerator_history_->previous_accelerator(); + const ui::EventType previous_event_type = previous_accelerator.type(); + const ui::KeyboardCode previous_key_code = previous_accelerator.key_code(); // You *MUST* return true when some action is performed. Otherwise, this // function might be called *twice*, via BrowserView::PreHandleKeyboardEvent diff --git a/ash/accelerators/accelerator_controller.h b/ash/accelerators/accelerator_controller.h index 0ca515b..82d1a36 100644 --- a/ash/accelerators/accelerator_controller.h +++ b/ash/accelerators/accelerator_controller.h @@ -16,6 +16,7 @@ #include "base/gtest_prod_util.h" #include "base/memory/scoped_ptr.h" #include "ui/base/accelerators/accelerator.h" +#include "ui/base/accelerators/accelerator_history.h" namespace ui { class AcceleratorManager; @@ -112,8 +113,8 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget { return &exit_warning_handler_; } - const ui::Accelerator& previous_accelerator_for_test() const { - return previous_accelerator_; + ui::AcceleratorHistory* accelerator_history() { + return accelerator_history_.get(); } // Overridden from ui::AcceleratorTarget: @@ -149,6 +150,9 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget { scoped_ptr<ui::AcceleratorManager> accelerator_manager_; + // A tracker for the current and previous accelerators. + scoped_ptr<ui::AcceleratorHistory> accelerator_history_; + // TODO(derat): BrightnessControlDelegate is also used by the system tray; // move it outside of this class. scoped_ptr<BrightnessControlDelegate> brightness_control_delegate_; @@ -157,10 +161,6 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget { keyboard_brightness_control_delegate_; scoped_ptr<ScreenshotDelegate> screenshot_delegate_; - // Remember previous accelerator as some accelerator needs to be fired - // with a specific sequence. - ui::Accelerator previous_accelerator_; - // Handles the exit accelerator which requires a double press to exit and // shows a popup with an explanation. ExitWarningHandler exit_warning_handler_; diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc index 3edd20a..66a66e4 100644 --- a/ash/accelerators/accelerator_controller_unittest.cc +++ b/ash/accelerators/accelerator_controller_unittest.cc @@ -227,6 +227,22 @@ class AcceleratorControllerTest : public test::AshTestBase { static AcceleratorController* GetController(); + static bool ProcessInController(const ui::Accelerator& accelerator) { + GetController()->accelerator_history()-> + StoreCurrentAccelerator(accelerator); + return GetController()->Process(accelerator); + } + + static const ui::Accelerator& GetPreviousAccelerator() { + return GetController()->accelerator_history()-> + previous_accelerator(); + } + + static const ui::Accelerator& GetCurrentAccelerator() { + return GetController()->accelerator_history()-> + current_accelerator(); + } + // Several functions to access ExitWarningHandler (as friend). static void StubForTest(ExitWarningHandler* ewh) { ewh->stub_timer_for_test_ = true; @@ -279,12 +295,12 @@ TEST_F(AcceleratorControllerTest, ExitWarningHandlerTestDoublePress) { StubForTest(ewh); EXPECT_TRUE(is_idle(ewh)); EXPECT_FALSE(is_ui_shown(ewh)); - EXPECT_TRUE(GetController()->Process(press)); - EXPECT_FALSE(GetController()->Process(release)); + EXPECT_TRUE(ProcessInController(press)); + EXPECT_FALSE(ProcessInController(release)); EXPECT_FALSE(is_idle(ewh)); EXPECT_TRUE(is_ui_shown(ewh)); - EXPECT_TRUE(GetController()->Process(press)); // second press before timer. - EXPECT_FALSE(GetController()->Process(release)); + EXPECT_TRUE(ProcessInController(press)); // second press before timer. + EXPECT_FALSE(ProcessInController(release)); SimulateTimerExpired(ewh); EXPECT_TRUE(is_exiting(ewh)); EXPECT_FALSE(is_ui_shown(ewh)); @@ -301,8 +317,8 @@ TEST_F(AcceleratorControllerTest, ExitWarningHandlerTestSinglePress) { StubForTest(ewh); EXPECT_TRUE(is_idle(ewh)); EXPECT_FALSE(is_ui_shown(ewh)); - EXPECT_TRUE(GetController()->Process(press)); - EXPECT_FALSE(GetController()->Process(release)); + EXPECT_TRUE(ProcessInController(press)); + EXPECT_FALSE(ProcessInController(release)); EXPECT_FALSE(is_idle(ewh)); EXPECT_TRUE(is_ui_shown(ewh)); SimulateTimerExpired(ewh); @@ -332,7 +348,7 @@ TEST_F(AcceleratorControllerTest, Register) { GetController()->Register(accelerator_a, &target); // The registered accelerator is processed. - EXPECT_TRUE(GetController()->Process(accelerator_a)); + EXPECT_TRUE(ProcessInController(accelerator_a)); EXPECT_EQ(1, target.accelerator_pressed_count()); } @@ -345,7 +361,7 @@ TEST_F(AcceleratorControllerTest, RegisterMultipleTarget) { // If multiple targets are registered with the same accelerator, the target // registered later processes the accelerator. - EXPECT_TRUE(GetController()->Process(accelerator_a)); + EXPECT_TRUE(ProcessInController(accelerator_a)); EXPECT_EQ(0, target1.accelerator_pressed_count()); EXPECT_EQ(1, target2.accelerator_pressed_count()); } @@ -360,13 +376,13 @@ TEST_F(AcceleratorControllerTest, Unregister) { // Unregistering a different accelerator does not affect the other // accelerator. GetController()->Unregister(accelerator_b, &target); - EXPECT_TRUE(GetController()->Process(accelerator_a)); + EXPECT_TRUE(ProcessInController(accelerator_a)); EXPECT_EQ(1, target.accelerator_pressed_count()); // The unregistered accelerator is no longer processed. target.reset(); GetController()->Unregister(accelerator_a, &target); - EXPECT_FALSE(GetController()->Process(accelerator_a)); + EXPECT_FALSE(ProcessInController(accelerator_a)); EXPECT_EQ(0, target.accelerator_pressed_count()); } @@ -382,12 +398,12 @@ TEST_F(AcceleratorControllerTest, UnregisterAll) { GetController()->UnregisterAll(&target1); // All the accelerators registered for |target1| are no longer processed. - EXPECT_FALSE(GetController()->Process(accelerator_a)); - EXPECT_FALSE(GetController()->Process(accelerator_b)); + EXPECT_FALSE(ProcessInController(accelerator_a)); + EXPECT_FALSE(ProcessInController(accelerator_b)); EXPECT_EQ(0, target1.accelerator_pressed_count()); // UnregisterAll with a different target does not affect the other target. - EXPECT_TRUE(GetController()->Process(accelerator_c)); + EXPECT_TRUE(ProcessInController(accelerator_c)); EXPECT_EQ(1, target2.accelerator_pressed_count()); } @@ -397,12 +413,12 @@ TEST_F(AcceleratorControllerTest, Process) { GetController()->Register(accelerator_a, &target1); // The registered accelerator is processed. - EXPECT_TRUE(GetController()->Process(accelerator_a)); + EXPECT_TRUE(ProcessInController(accelerator_a)); EXPECT_EQ(1, target1.accelerator_pressed_count()); // The non-registered accelerator is not processed. const ui::Accelerator accelerator_b(ui::VKEY_B, ui::EF_NONE); - EXPECT_FALSE(GetController()->Process(accelerator_b)); + EXPECT_FALSE(ProcessInController(accelerator_b)); } TEST_F(AcceleratorControllerTest, IsRegistered) { @@ -695,17 +711,17 @@ TEST_F(AcceleratorControllerTest, Previous) { generator.ReleaseKey(ui::VKEY_VOLUME_MUTE, ui::EF_NONE); EXPECT_EQ(ui::VKEY_VOLUME_MUTE, - GetController()->previous_accelerator_for_test().key_code()); + GetPreviousAccelerator().key_code()); EXPECT_EQ(ui::EF_NONE, - GetController()->previous_accelerator_for_test().modifiers()); + GetPreviousAccelerator().modifiers()); generator.PressKey(ui::VKEY_TAB, ui::EF_CONTROL_DOWN); generator.ReleaseKey(ui::VKEY_TAB, ui::EF_CONTROL_DOWN); EXPECT_EQ(ui::VKEY_TAB, - GetController()->previous_accelerator_for_test().key_code()); + GetPreviousAccelerator().key_code()); EXPECT_EQ(ui::EF_CONTROL_DOWN, - GetController()->previous_accelerator_for_test().modifiers()); + GetPreviousAccelerator().modifiers()); } TEST_F(AcceleratorControllerTest, DontRepeatToggleFullscreen) { @@ -800,13 +816,14 @@ TEST_F(AcceleratorControllerTest, MAYBE_ProcessOnce) { TEST_F(AcceleratorControllerTest, GlobalAccelerators) { // CycleBackward - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_TAB, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN))); // CycleForward EXPECT_TRUE( - GetController()->Process(ui::Accelerator(ui::VKEY_TAB, ui::EF_ALT_DOWN))); + ProcessInController(ui::Accelerator( + ui::VKEY_TAB, ui::EF_ALT_DOWN))); // CycleLinear - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_NONE))); #if defined(OS_CHROMEOS) @@ -815,22 +832,24 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) { { test::TestScreenshotDelegate* delegate = GetScreenshotDelegate(); delegate->set_can_take_screenshot(false); - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_CONTROL_DOWN))); EXPECT_TRUE( - GetController()->Process(ui::Accelerator(ui::VKEY_PRINT, ui::EF_NONE))); - EXPECT_TRUE(GetController()->Process(ui::Accelerator( + ProcessInController(ui::Accelerator( + ui::VKEY_PRINT, ui::EF_NONE))); + EXPECT_TRUE(ProcessInController(ui::Accelerator( ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN))); delegate->set_can_take_screenshot(true); EXPECT_EQ(0, delegate->handle_take_screenshot_count()); - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_CONTROL_DOWN))); EXPECT_EQ(1, delegate->handle_take_screenshot_count()); EXPECT_TRUE( - GetController()->Process(ui::Accelerator(ui::VKEY_PRINT, ui::EF_NONE))); + ProcessInController(ui::Accelerator( + ui::VKEY_PRINT, ui::EF_NONE))); EXPECT_EQ(2, delegate->handle_take_screenshot_count()); - EXPECT_TRUE(GetController()->Process(ui::Accelerator( + EXPECT_TRUE(ProcessInController(ui::Accelerator( ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN))); EXPECT_EQ(2, delegate->handle_take_screenshot_count()); } @@ -842,15 +861,15 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) { ash::Shell::GetInstance()->system_tray_delegate()->SetVolumeControlDelegate( scoped_ptr<VolumeControlDelegate>(delegate).Pass()); EXPECT_EQ(0, delegate->handle_volume_mute_count()); - EXPECT_TRUE(GetController()->Process(volume_mute)); + EXPECT_TRUE(ProcessInController(volume_mute)); EXPECT_EQ(1, delegate->handle_volume_mute_count()); EXPECT_EQ(volume_mute, delegate->last_accelerator()); EXPECT_EQ(0, delegate->handle_volume_down_count()); - EXPECT_TRUE(GetController()->Process(volume_down)); + EXPECT_TRUE(ProcessInController(volume_down)); EXPECT_EQ(1, delegate->handle_volume_down_count()); EXPECT_EQ(volume_down, delegate->last_accelerator()); EXPECT_EQ(0, delegate->handle_volume_up_count()); - EXPECT_TRUE(GetController()->Process(volume_up)); + EXPECT_TRUE(ProcessInController(volume_up)); EXPECT_EQ(1, delegate->handle_volume_up_count()); EXPECT_EQ(volume_up, delegate->last_accelerator()); } @@ -864,11 +883,11 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) { GetController()->SetBrightnessControlDelegate( scoped_ptr<BrightnessControlDelegate>(delegate).Pass()); EXPECT_EQ(0, delegate->handle_brightness_down_count()); - EXPECT_TRUE(GetController()->Process(brightness_down)); + EXPECT_TRUE(ProcessInController(brightness_down)); EXPECT_EQ(1, delegate->handle_brightness_down_count()); EXPECT_EQ(brightness_down, delegate->last_accelerator()); EXPECT_EQ(0, delegate->handle_brightness_up_count()); - EXPECT_TRUE(GetController()->Process(brightness_up)); + EXPECT_TRUE(ProcessInController(brightness_up)); EXPECT_EQ(1, delegate->handle_brightness_up_count()); EXPECT_EQ(brightness_up, delegate->last_accelerator()); } @@ -879,18 +898,18 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) { const ui::Accelerator alt_brightness_up(ui::VKEY_BRIGHTNESS_UP, ui::EF_ALT_DOWN); { - EXPECT_TRUE(GetController()->Process(alt_brightness_down)); - EXPECT_TRUE(GetController()->Process(alt_brightness_up)); + EXPECT_TRUE(ProcessInController(alt_brightness_down)); + EXPECT_TRUE(ProcessInController(alt_brightness_up)); DummyKeyboardBrightnessControlDelegate* delegate = new DummyKeyboardBrightnessControlDelegate; GetController()->SetKeyboardBrightnessControlDelegate( scoped_ptr<KeyboardBrightnessControlDelegate>(delegate).Pass()); EXPECT_EQ(0, delegate->handle_keyboard_brightness_down_count()); - EXPECT_TRUE(GetController()->Process(alt_brightness_down)); + EXPECT_TRUE(ProcessInController(alt_brightness_down)); EXPECT_EQ(1, delegate->handle_keyboard_brightness_down_count()); EXPECT_EQ(alt_brightness_down, delegate->last_accelerator()); EXPECT_EQ(0, delegate->handle_keyboard_brightness_up_count()); - EXPECT_TRUE(GetController()->Process(alt_brightness_up)); + EXPECT_TRUE(ProcessInController(alt_brightness_up)); EXPECT_EQ(1, delegate->handle_keyboard_brightness_up_count()); EXPECT_EQ(alt_brightness_up, delegate->last_accelerator()); } @@ -903,7 +922,7 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) { StubForTest(ewh); EXPECT_TRUE(is_idle(ewh)); EXPECT_FALSE(is_ui_shown(ewh)); - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN))); EXPECT_FALSE(is_idle(ewh)); EXPECT_TRUE(is_ui_shown(ewh)); @@ -914,35 +933,35 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) { #endif // New tab - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_T, ui::EF_CONTROL_DOWN))); // New incognito window - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_N, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN))); // New window - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_N, ui::EF_CONTROL_DOWN))); // Restore tab - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_T, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN))); // Show task manager - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_SHIFT_DOWN))); #if defined(OS_CHROMEOS) // Open file manager - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_M, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN))); // Lock screen // NOTE: Accelerators that do not work on the lock screen need to be // tested before the sequence below is invoked because it causes a side // effect of locking the screen. - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_L, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN))); #endif } @@ -954,37 +973,55 @@ TEST_F(AcceleratorControllerTest, GlobalAcceleratorsToggleAppList) { // The press event should not open the AppList, the release should instead. EXPECT_FALSE( - GetController()->Process(ui::Accelerator(ui::VKEY_LWIN, ui::EF_NONE))); + ProcessInController(ui::Accelerator(ui::VKEY_LWIN, ui::EF_NONE))); EXPECT_EQ(ui::VKEY_LWIN, - GetController()->previous_accelerator_for_test().key_code()); + GetCurrentAccelerator().key_code()); + + EXPECT_FALSE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); EXPECT_TRUE( - GetController()->Process(ReleaseAccelerator(ui::VKEY_LWIN, ui::EF_NONE))); + ProcessInController(ReleaseAccelerator(ui::VKEY_LWIN, ui::EF_NONE))); EXPECT_TRUE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); + EXPECT_EQ(ui::VKEY_LWIN, + GetPreviousAccelerator().key_code()); + // When spoken feedback is on, the AppList should not toggle. delegate->ToggleSpokenFeedback(ui::A11Y_NOTIFICATION_NONE); EXPECT_FALSE( - GetController()->Process(ui::Accelerator(ui::VKEY_LWIN, ui::EF_NONE))); + ProcessInController(ui::Accelerator(ui::VKEY_LWIN, ui::EF_NONE))); EXPECT_FALSE( - GetController()->Process(ReleaseAccelerator(ui::VKEY_LWIN, ui::EF_NONE))); + ProcessInController(ReleaseAccelerator( + ui::VKEY_LWIN, ui::EF_NONE))); delegate->ToggleSpokenFeedback(ui::A11Y_NOTIFICATION_NONE); EXPECT_TRUE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); EXPECT_FALSE( - GetController()->Process(ui::Accelerator(ui::VKEY_LWIN, ui::EF_NONE))); + ProcessInController(ui::Accelerator(ui::VKEY_LWIN, ui::EF_NONE))); EXPECT_TRUE( - GetController()->Process(ReleaseAccelerator(ui::VKEY_LWIN, ui::EF_NONE))); + ProcessInController(ReleaseAccelerator( + ui::VKEY_LWIN, ui::EF_NONE))); EXPECT_FALSE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); // When spoken feedback is on, the AppList should not toggle. delegate->ToggleSpokenFeedback(ui::A11Y_NOTIFICATION_NONE); EXPECT_FALSE( - GetController()->Process(ui::Accelerator(ui::VKEY_LWIN, ui::EF_NONE))); + ProcessInController(ui::Accelerator(ui::VKEY_LWIN, ui::EF_NONE))); EXPECT_FALSE( - GetController()->Process(ReleaseAccelerator(ui::VKEY_LWIN, ui::EF_NONE))); + ProcessInController(ReleaseAccelerator( + ui::VKEY_LWIN, ui::EF_NONE))); delegate->ToggleSpokenFeedback(ui::A11Y_NOTIFICATION_NONE); EXPECT_FALSE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); + +#if defined(OS_CHROMEOS) + // The press of VKEY_BROWSER_SEARCH should toggle the AppList + EXPECT_TRUE(ProcessInController(ui::Accelerator(ui::VKEY_BROWSER_SEARCH, + ui::EF_NONE))); + EXPECT_TRUE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); + EXPECT_FALSE(ProcessInController(ReleaseAccelerator(ui::VKEY_BROWSER_SEARCH, + ui::EF_NONE))); + EXPECT_TRUE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); +#endif } TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) { @@ -996,28 +1033,28 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) { const ui::Accelerator wide_half_1(ui::VKEY_DBE_SBCSCHAR, ui::EF_NONE); const ui::Accelerator wide_half_2(ui::VKEY_DBE_DBCSCHAR, ui::EF_NONE); const ui::Accelerator hangul(ui::VKEY_HANGUL, ui::EF_NONE); - EXPECT_FALSE(GetController()->Process(control_space)); - EXPECT_FALSE(GetController()->Process(convert)); - EXPECT_FALSE(GetController()->Process(non_convert)); - EXPECT_FALSE(GetController()->Process(wide_half_1)); - EXPECT_FALSE(GetController()->Process(wide_half_2)); - EXPECT_FALSE(GetController()->Process(hangul)); + EXPECT_FALSE(ProcessInController(control_space)); + EXPECT_FALSE(ProcessInController(convert)); + EXPECT_FALSE(ProcessInController(non_convert)); + EXPECT_FALSE(ProcessInController(wide_half_1)); + EXPECT_FALSE(ProcessInController(wide_half_2)); + EXPECT_FALSE(ProcessInController(hangul)); DummyImeControlDelegate* delegate = new DummyImeControlDelegate; GetController()->SetImeControlDelegate( scoped_ptr<ImeControlDelegate>(delegate).Pass()); EXPECT_EQ(0, delegate->handle_previous_ime_count()); - EXPECT_TRUE(GetController()->Process(control_space)); + EXPECT_TRUE(ProcessInController(control_space)); EXPECT_EQ(1, delegate->handle_previous_ime_count()); EXPECT_EQ(0, delegate->handle_switch_ime_count()); - EXPECT_TRUE(GetController()->Process(convert)); + EXPECT_TRUE(ProcessInController(convert)); EXPECT_EQ(1, delegate->handle_switch_ime_count()); - EXPECT_TRUE(GetController()->Process(non_convert)); + EXPECT_TRUE(ProcessInController(non_convert)); EXPECT_EQ(2, delegate->handle_switch_ime_count()); - EXPECT_TRUE(GetController()->Process(wide_half_1)); + EXPECT_TRUE(ProcessInController(wide_half_1)); EXPECT_EQ(3, delegate->handle_switch_ime_count()); - EXPECT_TRUE(GetController()->Process(wide_half_2)); + EXPECT_TRUE(ProcessInController(wide_half_2)); EXPECT_EQ(4, delegate->handle_switch_ime_count()); - EXPECT_TRUE(GetController()->Process(hangul)); + EXPECT_TRUE(ProcessInController(hangul)); EXPECT_EQ(5, delegate->handle_switch_ime_count()); } @@ -1034,11 +1071,11 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) { GetController()->SetImeControlDelegate( scoped_ptr<ImeControlDelegate>(delegate).Pass()); EXPECT_EQ(0, delegate->handle_next_ime_count()); - EXPECT_FALSE(GetController()->Process(shift_alt_press)); - EXPECT_FALSE(GetController()->Process(shift_alt)); + EXPECT_FALSE(ProcessInController(shift_alt_press)); + EXPECT_FALSE(ProcessInController(shift_alt)); EXPECT_EQ(1, delegate->handle_next_ime_count()); - EXPECT_FALSE(GetController()->Process(alt_shift_press)); - EXPECT_FALSE(GetController()->Process(alt_shift)); + EXPECT_FALSE(ProcessInController(alt_shift_press)); + EXPECT_FALSE(ProcessInController(alt_shift)); EXPECT_EQ(2, delegate->handle_next_ime_count()); // We should NOT switch IME when e.g. Shift+Alt+X is pressed and X is @@ -1049,10 +1086,10 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) { const ReleaseAccelerator shift_alt_x(ui::VKEY_X, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN); - EXPECT_FALSE(GetController()->Process(shift_alt_press)); - EXPECT_FALSE(GetController()->Process(shift_alt_x_press)); - EXPECT_FALSE(GetController()->Process(shift_alt_x)); - EXPECT_FALSE(GetController()->Process(shift_alt)); + EXPECT_FALSE(ProcessInController(shift_alt_press)); + EXPECT_FALSE(ProcessInController(shift_alt_x_press)); + EXPECT_FALSE(ProcessInController(shift_alt_x)); + EXPECT_FALSE(ProcessInController(shift_alt)); EXPECT_EQ(2, delegate->handle_next_ime_count()); // But we _should_ if X is either VKEY_RETURN or VKEY_SPACE. @@ -1064,10 +1101,10 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) { ui::VKEY_RETURN, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN); - EXPECT_FALSE(GetController()->Process(shift_alt_press)); - EXPECT_FALSE(GetController()->Process(shift_alt_return_press)); - EXPECT_FALSE(GetController()->Process(shift_alt_return)); - EXPECT_FALSE(GetController()->Process(shift_alt)); + EXPECT_FALSE(ProcessInController(shift_alt_press)); + EXPECT_FALSE(ProcessInController(shift_alt_return_press)); + EXPECT_FALSE(ProcessInController(shift_alt_return)); + EXPECT_FALSE(ProcessInController(shift_alt)); EXPECT_EQ(3, delegate->handle_next_ime_count()); const ui::Accelerator shift_alt_space_press( @@ -1077,10 +1114,10 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) { ui::VKEY_SPACE, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN); - EXPECT_FALSE(GetController()->Process(shift_alt_press)); - EXPECT_FALSE(GetController()->Process(shift_alt_space_press)); - EXPECT_FALSE(GetController()->Process(shift_alt_space)); - EXPECT_FALSE(GetController()->Process(shift_alt)); + EXPECT_FALSE(ProcessInController(shift_alt_press)); + EXPECT_FALSE(ProcessInController(shift_alt_space_press)); + EXPECT_FALSE(ProcessInController(shift_alt_space)); + EXPECT_FALSE(ProcessInController(shift_alt)); EXPECT_EQ(4, delegate->handle_next_ime_count()); } @@ -1096,11 +1133,11 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) { GetController()->SetImeControlDelegate( scoped_ptr<ImeControlDelegate>(delegate).Pass()); EXPECT_EQ(0, delegate->handle_next_ime_count()); - EXPECT_FALSE(GetController()->Process(shift_alt_press)); - EXPECT_FALSE(GetController()->Process(shift_alt)); + EXPECT_FALSE(ProcessInController(shift_alt_press)); + EXPECT_FALSE(ProcessInController(shift_alt)); EXPECT_EQ(1, delegate->handle_next_ime_count()); - EXPECT_FALSE(GetController()->Process(alt_shift_press)); - EXPECT_FALSE(GetController()->Process(alt_shift)); + EXPECT_FALSE(ProcessInController(alt_shift_press)); + EXPECT_FALSE(ProcessInController(alt_shift)); EXPECT_EQ(2, delegate->handle_next_ime_count()); // We should NOT switch IME when e.g. Shift+Alt+X is pressed and X is @@ -1111,10 +1148,10 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAccelerators) { const ReleaseAccelerator shift_alt_x(ui::VKEY_X, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN); - EXPECT_FALSE(GetController()->Process(shift_alt_press)); - EXPECT_FALSE(GetController()->Process(shift_alt_x_press)); - EXPECT_FALSE(GetController()->Process(shift_alt_x)); - EXPECT_FALSE(GetController()->Process(shift_alt)); + EXPECT_FALSE(ProcessInController(shift_alt_press)); + EXPECT_FALSE(ProcessInController(shift_alt_x_press)); + EXPECT_FALSE(ProcessInController(shift_alt_x)); + EXPECT_FALSE(ProcessInController(shift_alt)); EXPECT_EQ(2, delegate->handle_next_ime_count()); } #endif @@ -1127,11 +1164,11 @@ TEST_F(AcceleratorControllerTest, ImeGlobalAcceleratorsWorkaround139556) { const ui::Accelerator shift_alt_return_press( ui::VKEY_RETURN, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN); - EXPECT_FALSE(GetController()->Process(shift_alt_return_press)); + EXPECT_FALSE(ProcessInController(shift_alt_return_press)); const ui::Accelerator shift_alt_space_press( ui::VKEY_SPACE, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN); - EXPECT_FALSE(GetController()->Process(shift_alt_space_press)); + EXPECT_FALSE(ProcessInController(shift_alt_space_press)); } TEST_F(AcceleratorControllerTest, PreferredReservedAccelerators) { @@ -1269,21 +1306,23 @@ TEST_F(AcceleratorControllerTest, DisallowedAtModalWindow) { { test::TestScreenshotDelegate* delegate = GetScreenshotDelegate(); delegate->set_can_take_screenshot(false); - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_CONTROL_DOWN))); EXPECT_TRUE( - GetController()->Process(ui::Accelerator(ui::VKEY_PRINT, ui::EF_NONE))); - EXPECT_TRUE(GetController()->Process(ui::Accelerator( + ProcessInController(ui::Accelerator( + ui::VKEY_PRINT, ui::EF_NONE))); + EXPECT_TRUE(ProcessInController(ui::Accelerator( ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN))); delegate->set_can_take_screenshot(true); EXPECT_EQ(0, delegate->handle_take_screenshot_count()); - EXPECT_TRUE(GetController()->Process( + EXPECT_TRUE(ProcessInController( ui::Accelerator(ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_CONTROL_DOWN))); EXPECT_EQ(1, delegate->handle_take_screenshot_count()); EXPECT_TRUE( - GetController()->Process(ui::Accelerator(ui::VKEY_PRINT, ui::EF_NONE))); + ProcessInController(ui::Accelerator( + ui::VKEY_PRINT, ui::EF_NONE))); EXPECT_EQ(2, delegate->handle_take_screenshot_count()); - EXPECT_TRUE(GetController()->Process(ui::Accelerator( + EXPECT_TRUE(ProcessInController(ui::Accelerator( ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN))); EXPECT_EQ(2, delegate->handle_take_screenshot_count()); } @@ -1296,11 +1335,11 @@ TEST_F(AcceleratorControllerTest, DisallowedAtModalWindow) { GetController()->SetBrightnessControlDelegate( scoped_ptr<BrightnessControlDelegate>(delegate).Pass()); EXPECT_EQ(0, delegate->handle_brightness_down_count()); - EXPECT_TRUE(GetController()->Process(brightness_down)); + EXPECT_TRUE(ProcessInController(brightness_down)); EXPECT_EQ(1, delegate->handle_brightness_down_count()); EXPECT_EQ(brightness_down, delegate->last_accelerator()); EXPECT_EQ(0, delegate->handle_brightness_up_count()); - EXPECT_TRUE(GetController()->Process(brightness_up)); + EXPECT_TRUE(ProcessInController(brightness_up)); EXPECT_EQ(1, delegate->handle_brightness_up_count()); EXPECT_EQ(brightness_up, delegate->last_accelerator()); } @@ -1309,22 +1348,22 @@ TEST_F(AcceleratorControllerTest, DisallowedAtModalWindow) { const ui::Accelerator volume_down(ui::VKEY_VOLUME_DOWN, ui::EF_NONE); const ui::Accelerator volume_up(ui::VKEY_VOLUME_UP, ui::EF_NONE); { - EXPECT_TRUE(GetController()->Process(volume_mute)); - EXPECT_TRUE(GetController()->Process(volume_down)); - EXPECT_TRUE(GetController()->Process(volume_up)); + EXPECT_TRUE(ProcessInController(volume_mute)); + EXPECT_TRUE(ProcessInController(volume_down)); + EXPECT_TRUE(ProcessInController(volume_up)); TestVolumeControlDelegate* delegate = new TestVolumeControlDelegate; ash::Shell::GetInstance()->system_tray_delegate()->SetVolumeControlDelegate( scoped_ptr<VolumeControlDelegate>(delegate).Pass()); EXPECT_EQ(0, delegate->handle_volume_mute_count()); - EXPECT_TRUE(GetController()->Process(volume_mute)); + EXPECT_TRUE(ProcessInController(volume_mute)); EXPECT_EQ(1, delegate->handle_volume_mute_count()); EXPECT_EQ(volume_mute, delegate->last_accelerator()); EXPECT_EQ(0, delegate->handle_volume_down_count()); - EXPECT_TRUE(GetController()->Process(volume_down)); + EXPECT_TRUE(ProcessInController(volume_down)); EXPECT_EQ(1, delegate->handle_volume_down_count()); EXPECT_EQ(volume_down, delegate->last_accelerator()); EXPECT_EQ(0, delegate->handle_volume_up_count()); - EXPECT_TRUE(GetController()->Process(volume_up)); + EXPECT_TRUE(ProcessInController(volume_up)); EXPECT_EQ(1, delegate->handle_volume_up_count()); EXPECT_EQ(volume_up, delegate->last_accelerator()); } diff --git a/ash/accelerators/accelerator_filter_unittest.cc b/ash/accelerators/accelerator_filter_unittest.cc index d91d084..6684488 100644 --- a/ash/accelerators/accelerator_filter_unittest.cc +++ b/ash/accelerators/accelerator_filter_unittest.cc @@ -17,6 +17,7 @@ #include "ui/aura/test/aura_test_base.h" #include "ui/aura/test/test_windows.h" #include "ui/aura/window.h" +#include "ui/base/accelerators/accelerator_history.h" #include "ui/events/event.h" #include "ui/events/test/event_generator.h" #include "ui/gfx/rect.h" @@ -87,8 +88,11 @@ TEST_F(AcceleratorFilterTest, TestCapsLockMask) { // Tests if special hardware keys like brightness and volume are consumed as // expected by the shell. TEST_F(AcceleratorFilterTest, CanConsumeSystemKeys) { + scoped_ptr<ui::AcceleratorHistory> + accelerator_history(new ui::AcceleratorHistory()); ::wm::AcceleratorFilter filter( - scoped_ptr< ::wm::AcceleratorDelegate>(new AcceleratorDelegate).Pass()); + scoped_ptr< ::wm::AcceleratorDelegate>(new AcceleratorDelegate).Pass(), + accelerator_history.get()); aura::Window* root_window = Shell::GetPrimaryRootWindow(); // Normal keys are not consumed. diff --git a/ash/shell.cc b/ash/shell.cc index f8e6aab..5d097b6 100644 --- a/ash/shell.cc +++ b/ash/shell.cc @@ -927,7 +927,8 @@ void Shell::Init(const ShellInitParams& init_params) { AddPreTargetHandler(input_method_filter_.get()); accelerator_filter_.reset(new ::wm::AcceleratorFilter( - scoped_ptr< ::wm::AcceleratorDelegate>(new AcceleratorDelegate).Pass())); + scoped_ptr< ::wm::AcceleratorDelegate>(new AcceleratorDelegate).Pass(), + accelerator_controller_->accelerator_history())); AddPreTargetHandler(accelerator_filter_.get()); event_transformation_handler_.reset(new EventTransformationHandler); diff --git a/athena/input/accelerator_manager_impl.cc b/athena/input/accelerator_manager_impl.cc index ad03023..c1e037a 100644 --- a/athena/input/accelerator_manager_impl.cc +++ b/athena/input/accelerator_manager_impl.cc @@ -262,7 +262,8 @@ void AcceleratorManagerImpl::Init() { new AcceleratorDelegate(this)); accelerator_filter_.reset( - new wm::AcceleratorFilter(accelerator_delegate.Pass())); + new wm::AcceleratorFilter(accelerator_delegate.Pass(), + accelerator_history_.get())); toplevel->AddPreTargetHandler(accelerator_filter_.get()); } @@ -289,6 +290,7 @@ AcceleratorManagerImpl::AcceleratorManagerImpl( AcceleratorWrapper* accelerator_wrapper, bool global) : accelerator_wrapper_(accelerator_wrapper), + accelerator_history_(new ui::AcceleratorHistory), debug_accelerators_enabled_(switches::IsDebugAcceleratorsEnabled()), global_(global) { } diff --git a/athena/input/accelerator_manager_impl.h b/athena/input/accelerator_manager_impl.h index f261263..bd0c9a7 100644 --- a/athena/input/accelerator_manager_impl.h +++ b/athena/input/accelerator_manager_impl.h @@ -11,6 +11,7 @@ #include "base/macros.h" #include "ui/base/accelerators/accelerator.h" +#include "ui/base/accelerators/accelerator_history.h" #include "ui/events/event_target_iterator.h" namespace aura { @@ -75,6 +76,7 @@ class AcceleratorManagerImpl : public AcceleratorManager, std::map<ui::Accelerator, InternalData> accelerators_; scoped_ptr<AcceleratorWrapper> accelerator_wrapper_; + scoped_ptr<ui::AcceleratorHistory> accelerator_history_; scoped_ptr<wm::AcceleratorFilter> accelerator_filter_; scoped_ptr<wm::NestedAcceleratorController> nested_accelerator_controller_; bool debug_accelerators_enabled_; diff --git a/chrome/browser/chromeos/accessibility/sticky_keys_browsertest.cc b/chrome/browser/chromeos/accessibility/sticky_keys_browsertest.cc index 0d2a6b6..d1f8471 100644 --- a/chrome/browser/chromeos/accessibility/sticky_keys_browsertest.cc +++ b/chrome/browser/chromeos/accessibility/sticky_keys_browsertest.cc @@ -21,11 +21,19 @@ #include "chrome/test/base/interactive_test_utils.h" #include "ui/aura/window_event_dispatcher.h" #include "ui/events/keycodes/keyboard_codes.h" +#include "ui/events/test/event_generator.h" #include "ui/gfx/native_widget_types.h" namespace chromeos { class StickyKeysBrowserTest : public InProcessBrowserTest { + public: + void SetUpOnMainThread() override { + content::BrowserTestBase::SetUpOnMainThread(); + event_generator_.reset( + new ui::test::EventGenerator(browser()->window()->GetNativeWindow())); + } + protected: StickyKeysBrowserTest() {} virtual ~StickyKeysBrowserTest() {} @@ -43,18 +51,12 @@ class StickyKeysBrowserTest : public InProcessBrowserTest { } void SendKeyPress(ui::KeyboardCode key) { - gfx::NativeWindow root_window = - ash::Shell::GetInstance()->GetPrimaryRootWindow(); - ASSERT_TRUE( - ui_test_utils::SendKeyPressToWindowSync(root_window, - key, - false, // control - false, // shift - false, // alt - false)); // command + event_generator_->PressKey(key, ui::EF_NONE); + event_generator_->ReleaseKey(key, ui::EF_NONE); } content::NotificationRegistrar registrar_; + scoped_ptr<ui::test::EventGenerator> event_generator_; DISALLOW_COPY_AND_ASSIGN(StickyKeysBrowserTest); }; @@ -153,6 +155,11 @@ IN_PROC_BROWSER_TEST_F(StickyKeysBrowserTest, SearchLeftOmnibox) { // Give the omnibox focus. omnibox->ShowURL(); + // Make sure that the AppList is not erronously displayed and the omnibox + // doesn't lost focus + EXPECT_FALSE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); + EXPECT_TRUE(omnibox->GetNativeView()->HasFocus()); + // Type 'foo'. SendKeyPress(ui::VKEY_F); SendKeyPress(ui::VKEY_O); @@ -164,10 +171,16 @@ IN_PROC_BROWSER_TEST_F(StickyKeysBrowserTest, SearchLeftOmnibox) { ASSERT_EQ(3U, start); ASSERT_EQ(3U, end); + EXPECT_FALSE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); + EXPECT_TRUE(omnibox->GetNativeView()->HasFocus()); + // Hit Home by sequencing Search (left Windows) and Left (arrow). SendKeyPress(ui::VKEY_LWIN); SendKeyPress(ui::VKEY_LEFT); + EXPECT_FALSE(ash::Shell::GetInstance()->GetAppListTargetVisibility()); + EXPECT_TRUE(omnibox->GetNativeView()->HasFocus()); + // Verify caret moved to the beginning. omnibox->GetSelectionBounds(&start, &end); ASSERT_EQ(0U, start); diff --git a/ui/base/BUILD.gn b/ui/base/BUILD.gn index 78f32fe..0143c06 100644 --- a/ui/base/BUILD.gn +++ b/ui/base/BUILD.gn @@ -16,6 +16,8 @@ component("base") { sources = [ "accelerators/accelerator.cc", "accelerators/accelerator.h", + "accelerators/accelerator_history.cc", + "accelerators/accelerator_history.h", "accelerators/accelerator_manager.cc", "accelerators/accelerator_manager.h", "accelerators/menu_label_accelerator_util_linux.cc", diff --git a/ui/base/accelerators/accelerator_history.cc b/ui/base/accelerators/accelerator_history.cc new file mode 100644 index 0000000..3c8821d --- /dev/null +++ b/ui/base/accelerators/accelerator_history.cc @@ -0,0 +1,30 @@ +// Copyright 2014 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 "ui/base/accelerators/accelerator_history.h" +#include "ui/events/event_constants.h" + +namespace ui { + +// ---------------------------------------------------------------------- +// Public Methods +// ---------------------------------------------------------------------- + +AcceleratorHistory::AcceleratorHistory() + : current_accelerator_(), + previous_accelerator_() { +} + +AcceleratorHistory::~AcceleratorHistory() { +} + +void AcceleratorHistory::StoreCurrentAccelerator( + const Accelerator& accelerator) { + if (accelerator != current_accelerator_) { + previous_accelerator_ = current_accelerator_; + current_accelerator_ = accelerator; + } +} + +} // namespace ui diff --git a/ui/base/accelerators/accelerator_history.h b/ui/base/accelerators/accelerator_history.h new file mode 100644 index 0000000..435e752 --- /dev/null +++ b/ui/base/accelerators/accelerator_history.h @@ -0,0 +1,47 @@ +// Copyright 2014 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. + +#ifndef UI_BASE_ACCELERATORS_ACCELERATOR_HISTORY_H_ +#define UI_BASE_ACCELERATORS_ACCELERATOR_HISTORY_H_ + +#include "base/memory/singleton.h" +#include "ui/base/accelerators/accelerator.h" +#include "ui/base/ui_base_export.h" +#include "ui/events/event.h" +#include "ui/events/event_handler.h" + +namespace ui { + +// Keeps track of the system-wide current and the most recent previous +// key accelerators. +class UI_BASE_EXPORT AcceleratorHistory { + public: + AcceleratorHistory(); + ~AcceleratorHistory(); + + // Returns the most recent recorded accelerator. + const Accelerator& current_accelerator() const { + return current_accelerator_; + } + + // Returns the most recent previously recorded accelerator that is different + // than the current. + const Accelerator& previous_accelerator() const { + return previous_accelerator_; + } + + // Stores the given |accelerator| only if it's different than the currently + // stored one. + void StoreCurrentAccelerator(const Accelerator& accelerator); + + private: + Accelerator current_accelerator_; + Accelerator previous_accelerator_; + + DISALLOW_COPY_AND_ASSIGN(AcceleratorHistory); +}; + +}; // namespace ui + +#endif // UI_BASE_ACCELERATORS_ACCELERATOR_HISTORY_H_ diff --git a/ui/base/ui_base.gyp b/ui/base/ui_base.gyp index 6b64c7a..928cf2c 100644 --- a/ui/base/ui_base.gyp +++ b/ui/base/ui_base.gyp @@ -41,6 +41,8 @@ # Note: file list duplicated in GN build. 'accelerators/accelerator.cc', 'accelerators/accelerator.h', + 'accelerators/accelerator_history.cc', + 'accelerators/accelerator_history.h', 'accelerators/accelerator_manager.cc', 'accelerators/accelerator_manager.h', 'accelerators/menu_label_accelerator_util_linux.cc', diff --git a/ui/views/focus/focus_manager.cc b/ui/views/focus/focus_manager.cc index 8dec8ea..706b318 100644 --- a/ui/views/focus/focus_manager.cc +++ b/ui/views/focus/focus_manager.cc @@ -30,6 +30,18 @@ namespace views { namespace { +#if defined(OS_MACOSX) || defined(OS_CHROMEOS) +static const int kEventFlagsMask = ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN | + ui::EF_ALT_DOWN | ui::EF_COMMAND_DOWN; +#else +static const int kEventFlagsMask = ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN | + ui::EF_ALT_DOWN; +#endif + +static inline int CalculateModifiers(const ui::KeyEvent& event) { + return event.flags() & kEventFlagsMask; +} + } // namespace bool FocusManager::shortcut_handling_suspended_ = false; @@ -59,17 +71,7 @@ bool FocusManager::OnKeyEvent(const ui::KeyEvent& event) { if (shortcut_handling_suspended()) return true; - int modifiers = ui::EF_NONE; - if (event.IsShiftDown()) - modifiers |= ui::EF_SHIFT_DOWN; - if (event.IsControlDown()) - modifiers |= ui::EF_CONTROL_DOWN; - if (event.IsAltDown()) - modifiers |= ui::EF_ALT_DOWN; -#if defined(OS_MACOSX) || defined(OS_CHROMEOS) - if (event.IsCommandDown()) - modifiers |= ui::EF_COMMAND_DOWN; -#endif + int modifiers = CalculateModifiers(event); ui::Accelerator accelerator(event.key_code(), modifiers); accelerator.set_type(event.type()); accelerator.set_is_repeat(event.IsRepeat()); diff --git a/ui/wm/core/accelerator_filter.cc b/ui/wm/core/accelerator_filter.cc index 51c6106..12e9131 100644 --- a/ui/wm/core/accelerator_filter.cc +++ b/ui/wm/core/accelerator_filter.cc @@ -5,6 +5,7 @@ #include "ui/wm/core/accelerator_filter.h" #include "ui/base/accelerators/accelerator.h" +#include "ui/base/accelerators/accelerator_history.h" #include "ui/events/event.h" #include "ui/wm/core/accelerator_delegate.h" @@ -37,8 +38,12 @@ bool IsSystemKey(ui::KeyboardCode key_code) { //////////////////////////////////////////////////////////////////////////////// // AcceleratorFilter, public: -AcceleratorFilter::AcceleratorFilter(scoped_ptr<AcceleratorDelegate> delegate) - : delegate_(delegate.Pass()) { +AcceleratorFilter::AcceleratorFilter( + scoped_ptr<AcceleratorDelegate> delegate, + ui::AcceleratorHistory* accelerator_history) + : delegate_(delegate.Pass()), + accelerator_history_(accelerator_history) { + DCHECK(accelerator_history); } AcceleratorFilter::~AcceleratorFilter() { @@ -56,6 +61,7 @@ void AcceleratorFilter::OnKeyEvent(ui::KeyEvent* event) { } ui::Accelerator accelerator = CreateAcceleratorFromKeyEvent(*event); + accelerator_history_->StoreCurrentAccelerator(accelerator); AcceleratorDelegate::KeyType key_type = IsSystemKey(event->key_code()) ? AcceleratorDelegate::KEY_TYPE_SYSTEM diff --git a/ui/wm/core/accelerator_filter.h b/ui/wm/core/accelerator_filter.h index 44579ef..79317e1 100644 --- a/ui/wm/core/accelerator_filter.h +++ b/ui/wm/core/accelerator_filter.h @@ -12,6 +12,7 @@ namespace ui { class Accelerator; +class AcceleratorHistory; } namespace wm { @@ -21,7 +22,10 @@ class AcceleratorDelegate; // keyboard accelerators. class WM_EXPORT AcceleratorFilter : public ui::EventHandler { public: - AcceleratorFilter(scoped_ptr<AcceleratorDelegate> delegate); + // AcceleratorFilter doesn't own |accelerator_history|, it's owned by + // AcceleratorController. + AcceleratorFilter(scoped_ptr<AcceleratorDelegate> delegate, + ui::AcceleratorHistory* accelerator_history); ~AcceleratorFilter() override; // Overridden from ui::EventHandler: @@ -29,6 +33,7 @@ class WM_EXPORT AcceleratorFilter : public ui::EventHandler { private: scoped_ptr<AcceleratorDelegate> delegate_; + ui::AcceleratorHistory* accelerator_history_; DISALLOW_COPY_AND_ASSIGN(AcceleratorFilter); }; |