diff options
-rw-r--r-- | ash/accelerators/accelerator_controller.cc | 8 | ||||
-rw-r--r-- | ash/accelerators/accelerator_controller.h | 8 | ||||
-rw-r--r-- | ash/accelerators/accelerator_controller_unittest.cc | 106 | ||||
-rw-r--r-- | ash/accelerators/accelerator_table.cc | 13 | ||||
-rw-r--r-- | ash/accelerators/accelerator_table.h | 3 | ||||
-rw-r--r-- | ash/accelerators/exit_warning_handler.cc | 64 | ||||
-rw-r--r-- | ash/accelerators/exit_warning_handler.h | 54 | ||||
-rw-r--r-- | chrome/browser/ui/views/accelerator_table.cc | 2 |
8 files changed, 144 insertions, 114 deletions
diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc index 43e2dac..175c79d 100644 --- a/ash/accelerators/accelerator_controller.cc +++ b/ash/accelerators/accelerator_controller.cc @@ -368,7 +368,8 @@ void AcceleratorControllerContext::UpdateContext( // AcceleratorController, public: AcceleratorController::AcceleratorController() - : accelerator_manager_(new ui::AcceleratorManager) { + : accelerator_manager_(new ui::AcceleratorManager), + exit_warning_handler_(&context_) { Init(); } @@ -577,9 +578,8 @@ bool AcceleratorController::PerformAction(int action, case OPEN_FEEDBACK_PAGE: ash::Shell::GetInstance()->delegate()->OpenFeedbackPage(); return true; - case EXIT_PRESSED: - case EXIT_RELEASED: - exit_warning_handler_.HandleExitKey(action == EXIT_PRESSED); + case EXIT: + exit_warning_handler_.HandleAccelerator(); return true; case NEW_INCOGNITO_WINDOW: Shell::GetInstance()->delegate()->NewWindow(true /* is_incognito */); diff --git a/ash/accelerators/accelerator_controller.h b/ash/accelerators/accelerator_controller.h index b1e4bfc..61076e6 100644 --- a/ash/accelerators/accelerator_controller.h +++ b/ash/accelerators/accelerator_controller.h @@ -41,6 +41,9 @@ class ASH_EXPORT AcceleratorControllerContext { // event type of the previous accelerator. void UpdateContext(const ui::Accelerator& accelerator); + const ui::Accelerator& current_accelerator() const { + return current_accelerator_; + } const ui::Accelerator& previous_accelerator() const { return previous_accelerator_; } @@ -141,7 +144,6 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget { // TODO(derat): BrightnessControlDelegate is also used by the system tray; // move it outside of this class. scoped_ptr<BrightnessControlDelegate> brightness_control_delegate_; - ExitWarningHandler exit_warning_handler_; scoped_ptr<ImeControlDelegate> ime_control_delegate_; scoped_ptr<KeyboardBrightnessControlDelegate> keyboard_brightness_control_delegate_; @@ -150,6 +152,10 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget { // Contextual information, eg. if the current accelerator is repeated. AcceleratorControllerContext context_; + // Handles the exit accelerator which requires a long hold to exit and + // shows a popup with an explanation. + ExitWarningHandler exit_warning_handler_; + // A map from accelerators to the AcceleratorAction values, which are used in // the implementation. std::map<ui::Accelerator, int> accelerators_; diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc index 933d0a5..216dbb5 100644 --- a/ash/accelerators/accelerator_controller_unittest.cc +++ b/ash/accelerators/accelerator_controller_unittest.cc @@ -322,23 +322,26 @@ class AcceleratorControllerTest : public test::AshTestBase { static bool ProcessWithContext(const ui::Accelerator& accelerator); // Several functions to access ExitWarningHandler (as friend). - static void StubForTest(ExitWarningHandler& ewh) { - ewh.stub_timers_for_test_ = true; + static void StubForTest(ExitWarningHandler* ewh) { + ewh->stub_timers_for_test_ = true; } - static void SimulateTimer1Expired(ExitWarningHandler& ewh) { - ewh.Timer1Action(); + static void Reset(ExitWarningHandler* ewh) { + ewh->state_ = ExitWarningHandler::IDLE; } - static void SimulateTimer2Expired(ExitWarningHandler& ewh) { - ewh.Timer2Action(); + static void SimulateTimer1Expired(ExitWarningHandler* ewh) { + ewh->Timer1Action(); } - static bool is_ui_shown(ExitWarningHandler& ewh) { - return !!ewh.widget_; + static void SimulateTimer2Expired(ExitWarningHandler* ewh) { + ewh->Timer2Action(); } - static bool is_idle(ExitWarningHandler& ewh) { - return ewh.state_ == ExitWarningHandler::IDLE; + static bool is_ui_shown(ExitWarningHandler* ewh) { + return !!ewh->widget_; } - static bool is_exiting(ExitWarningHandler& ewh) { - return ewh.state_ == ExitWarningHandler::EXITING; + static bool is_idle(ExitWarningHandler* ewh) { + return ewh->state_ == ExitWarningHandler::IDLE; + } + static bool is_exiting(ExitWarningHandler* ewh) { + return ewh->state_ == ExitWarningHandler::EXITING; } private: @@ -356,71 +359,87 @@ bool AcceleratorControllerTest::ProcessWithContext( return controller->Process(accelerator); } -// Quick double press of exit key => exiting +#if !defined(OS_WIN) +// Quick double press of exit shortcut => exiting TEST_F(AcceleratorControllerTest, ExitWarningHandlerTestDoublePress) { - ExitWarningHandler ewh; + ui::Accelerator press(ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN); + ui::Accelerator release(press); + release.set_type(ui::ET_KEY_RELEASED); + ExitWarningHandler* ewh = GetController()->GetExitWarningHandlerForTest(); + ASSERT_TRUE(!!ewh); StubForTest(ewh); EXPECT_TRUE(is_idle(ewh)); EXPECT_FALSE(is_ui_shown(ewh)); - - ewh.HandleExitKey(true); + EXPECT_TRUE(ProcessWithContext(press)); + EXPECT_FALSE(ProcessWithContext(release)); EXPECT_TRUE(is_ui_shown(ewh)); - ewh.HandleExitKey(false); - ewh.HandleExitKey(true); // double press + EXPECT_TRUE(ProcessWithContext(press)); // double press + EXPECT_FALSE(ProcessWithContext(release)); SimulateTimer1Expired(ewh); // simulate double press timer expired SimulateTimer2Expired(ewh); // simulate long hold timer expired - ewh.HandleExitKey(false); EXPECT_FALSE(is_ui_shown(ewh)); EXPECT_TRUE(is_exiting(ewh)); + Reset(ewh); } -// Long hold of exit key => exiting +// Long hold of exit shortcut => exiting TEST_F(AcceleratorControllerTest, ExitWarningHandlerTestLongHold) { - ExitWarningHandler ewh; + ui::Accelerator press(ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN); + ExitWarningHandler* ewh = GetController()->GetExitWarningHandlerForTest(); + ASSERT_TRUE(!!ewh); StubForTest(ewh); EXPECT_TRUE(is_idle(ewh)); EXPECT_FALSE(is_ui_shown(ewh)); - - ewh.HandleExitKey(true); + EXPECT_TRUE(ProcessWithContext(press)); // hold EXPECT_TRUE(is_ui_shown(ewh)); SimulateTimer1Expired(ewh); // simulate double press timer expired SimulateTimer2Expired(ewh); // simulate long hold timer expired - ewh.HandleExitKey(false); // release after long hold EXPECT_FALSE(is_ui_shown(ewh)); EXPECT_TRUE(is_exiting(ewh)); + Reset(ewh); } -// Release of exit key before hold time limit => cancel +// Release of exit shortcut before hold time limit => cancel TEST_F(AcceleratorControllerTest, ExitWarningHandlerTestEarlyRelease) { - ExitWarningHandler ewh; + ui::Accelerator press(ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN); + ui::Accelerator release(press); + release.set_type(ui::ET_KEY_RELEASED); + ExitWarningHandler* ewh = GetController()->GetExitWarningHandlerForTest(); + ASSERT_TRUE(!!ewh); StubForTest(ewh); EXPECT_TRUE(is_idle(ewh)); EXPECT_FALSE(is_ui_shown(ewh)); - - ewh.HandleExitKey(true); + EXPECT_TRUE(ProcessWithContext(press)); EXPECT_TRUE(is_ui_shown(ewh)); SimulateTimer1Expired(ewh); // simulate double press timer expired - ewh.HandleExitKey(false); // release before long hold limit + EXPECT_FALSE(ProcessWithContext(release)); SimulateTimer2Expired(ewh); // simulate long hold timer expired EXPECT_FALSE(is_ui_shown(ewh)); EXPECT_TRUE(is_idle(ewh)); + Reset(ewh); } -// Release of exit key before double press limit => cancel. +// Second Press after double press time limit => cancel TEST_F(AcceleratorControllerTest, ExitWarningHandlerTestQuickRelease) { - ExitWarningHandler ewh; + ui::Accelerator press(ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN); + ui::Accelerator release(press); + release.set_type(ui::ET_KEY_RELEASED); + ExitWarningHandler* ewh = GetController()->GetExitWarningHandlerForTest(); + ASSERT_TRUE(!!ewh); StubForTest(ewh); EXPECT_TRUE(is_idle(ewh)); EXPECT_FALSE(is_ui_shown(ewh)); - - ewh.HandleExitKey(true); + EXPECT_TRUE(ProcessWithContext(press)); + EXPECT_FALSE(ProcessWithContext(release)); EXPECT_TRUE(is_ui_shown(ewh)); - ewh.HandleExitKey(false); // release before double press limit SimulateTimer1Expired(ewh); // simulate double press timer expired + EXPECT_TRUE(ProcessWithContext(press)); SimulateTimer2Expired(ewh); // simulate long hold timer expired EXPECT_FALSE(is_ui_shown(ewh)); EXPECT_TRUE(is_idle(ewh)); + Reset(ewh); } +#endif // !defined(OS_WIN) TEST_F(AcceleratorControllerTest, Register) { const ui::Accelerator accelerator_a(ui::VKEY_A, ui::EF_NONE); @@ -948,17 +967,18 @@ TEST_F(AcceleratorControllerTest, GlobalAccelerators) { // Exit ExitWarningHandler* ewh = GetController()->GetExitWarningHandlerForTest(); ASSERT_TRUE(!!ewh); - StubForTest(*ewh); - EXPECT_TRUE(is_idle(*ewh)); - EXPECT_FALSE(is_ui_shown(*ewh)); + StubForTest(ewh); + EXPECT_TRUE(is_idle(ewh)); + EXPECT_FALSE(is_ui_shown(ewh)); EXPECT_TRUE(ProcessWithContext( ui::Accelerator(ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN))); - EXPECT_FALSE(is_idle(*ewh)); - EXPECT_TRUE(is_ui_shown(*ewh)); - SimulateTimer1Expired(*ewh); - SimulateTimer2Expired(*ewh); - EXPECT_FALSE(is_ui_shown(*ewh)); - EXPECT_TRUE(is_exiting(*ewh)); + EXPECT_FALSE(is_idle(ewh)); + EXPECT_TRUE(is_ui_shown(ewh)); + SimulateTimer1Expired(ewh); + SimulateTimer2Expired(ewh); + EXPECT_FALSE(is_ui_shown(ewh)); + EXPECT_TRUE(is_exiting(ewh)); + Reset(ewh); #endif // New tab diff --git a/ash/accelerators/accelerator_table.cc b/ash/accelerators/accelerator_table.cc index ec32f2c..e60408e 100644 --- a/ash/accelerators/accelerator_table.cc +++ b/ash/accelerators/accelerator_table.cc @@ -73,8 +73,7 @@ const AcceleratorData kAcceleratorData[] = { #endif // defined(OS_CHROMEOS) { true, ui::VKEY_I, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, OPEN_FEEDBACK_PAGE }, #if !defined(OS_WIN) - { true, ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, EXIT_PRESSED }, - { false, ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, EXIT_RELEASED }, + { true, ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, EXIT }, #endif { true, ui::VKEY_I, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN, TOUCH_HUD_MODE_CHANGE }, @@ -280,8 +279,7 @@ const size_t kActionsAllowedAtLoginOrLockScreenLength = arraysize(kActionsAllowedAtLoginOrLockScreen); const AcceleratorAction kActionsAllowedAtLockScreen[] = { - EXIT_PRESSED, - EXIT_RELEASED + EXIT, }; const size_t kActionsAllowedAtLockScreenLength = @@ -291,8 +289,7 @@ const AcceleratorAction kActionsAllowedAtModalWindow[] = { BRIGHTNESS_DOWN, BRIGHTNESS_UP, DISABLE_CAPS_LOCK, - EXIT_PRESSED, - EXIT_RELEASED, + EXIT, KEYBOARD_BRIGHTNESS_DOWN, KEYBOARD_BRIGHTNESS_UP, MAGNIFY_SCREEN_ZOOM_IN, @@ -335,6 +332,7 @@ const AcceleratorAction kNonrepeatableActions[] = { CYCLE_BACKWARD_MRU, CYCLE_FORWARD_LINEAR, CYCLE_FORWARD_MRU, + EXIT, PRINT_UI_HIERARCHIES, // Don't fill the logs if the key is held down. ROTATE_SCREEN, ROTATE_WINDOW, @@ -357,8 +355,7 @@ const AcceleratorAction kActionsAllowedInAppMode[] = { CYCLE_FORWARD_LINEAR, CYCLE_FORWARD_MRU, DISABLE_CAPS_LOCK, - EXIT_PRESSED, - EXIT_RELEASED, + EXIT, KEYBOARD_BRIGHTNESS_DOWN, KEYBOARD_BRIGHTNESS_UP, MAGNIFY_SCREEN_ZOOM_IN, // Control+F7 diff --git a/ash/accelerators/accelerator_table.h b/ash/accelerators/accelerator_table.h index ee635f4..7e5e456 100644 --- a/ash/accelerators/accelerator_table.h +++ b/ash/accelerators/accelerator_table.h @@ -28,8 +28,7 @@ enum AcceleratorAction { DEBUG_TOGGLE_SHOW_FPS_COUNTER, DEBUG_TOGGLE_SHOW_PAINT_RECTS, DISABLE_CAPS_LOCK, - EXIT_PRESSED, - EXIT_RELEASED, + EXIT, FOCUS_LAUNCHER, FOCUS_NEXT_PANE, FOCUS_PREVIOUS_PANE, diff --git a/ash/accelerators/exit_warning_handler.cc b/ash/accelerators/exit_warning_handler.cc index a285560..aa26683 100644 --- a/ash/accelerators/exit_warning_handler.cc +++ b/ash/accelerators/exit_warning_handler.cc @@ -4,6 +4,7 @@ #include "ash/accelerators/exit_warning_handler.h" +#include "ash/accelerators/accelerator_controller.h" #include "ash/shell.h" #include "ash/shell_delegate.h" #include "ash/shell_window_ids.h" @@ -26,7 +27,7 @@ namespace ash { namespace { const int64 kDoublePressTimeOutMilliseconds = 300; -const int64 kHoldTimeOutMilliseconds = 1700; +const int64 kHoldTimeOutMilliseconds = 1000; const SkColor kForegroundColor = 0xFFFFFFFF; const SkColor kBackgroundColor = 0xE0808080; const int kHorizontalMarginAroundText = 100; @@ -73,10 +74,11 @@ class ExitWarningWidgetDelegateView : public views::WidgetDelegateView { } // namespace -ExitWarningHandler::ExitWarningHandler() - : state_(IDLE), - widget_(NULL), - stub_timers_for_test_(false) { +ExitWarningHandler::ExitWarningHandler(AcceleratorControllerContext* context) + : context_(context), + state_(IDLE), + widget_(NULL), + stub_timers_for_test_(false) { } ExitWarningHandler::~ExitWarningHandler() { @@ -84,30 +86,24 @@ ExitWarningHandler::~ExitWarningHandler() { Hide(); } -void ExitWarningHandler::HandleExitKey(bool press) { +void ExitWarningHandler::HandleAccelerator() { + if (!context_) + return; switch (state_) { case IDLE: - if (press) { - state_ = WAIT_FOR_QUICK_RELEASE; - Show(); - StartTimers(); - } - break; - case WAIT_FOR_QUICK_RELEASE: - if (!press) - state_ = WAIT_FOR_DOUBLE_PRESS; + state_ = WAIT_FOR_DOUBLE_PRESS; + accelerator_ = context_->current_accelerator(); + Show(); + StartTimers(); break; case WAIT_FOR_DOUBLE_PRESS: - if (press) { - state_ = EXITING; - CancelTimers(); - Hide(); - Shell::GetInstance()->delegate()->Exit(); - } + state_ = EXITING; + CancelTimers(); + Hide(); + Shell::GetInstance()->delegate()->Exit(); break; case WAIT_FOR_LONG_HOLD: - if (!press) - state_ = CANCELED; + state_ = CANCELED; break; case CANCELED: case EXITING: @@ -119,21 +115,25 @@ void ExitWarningHandler::HandleExitKey(bool press) { } void ExitWarningHandler::Timer1Action() { - if (state_ == WAIT_FOR_QUICK_RELEASE) + if (state_ == WAIT_FOR_DOUBLE_PRESS) state_ = WAIT_FOR_LONG_HOLD; - else if (state_ == WAIT_FOR_DOUBLE_PRESS) - state_ = CANCELED; } void ExitWarningHandler::Timer2Action() { + Hide(); if (state_ == CANCELED) { state_ = IDLE; - Hide(); - } - else if (state_ == WAIT_FOR_LONG_HOLD) { - state_ = EXITING; - Hide(); - Shell::GetInstance()->delegate()->Exit(); + } else if (state_ == WAIT_FOR_LONG_HOLD) { + if (accelerator_ == context_->current_accelerator()) { + // We detect if the user has released any one of the keys that + // make up the shortcut by comparing "our" accelerator to the one + // from the current context and do not exit in that case. + state_ = EXITING; + Shell::GetInstance()->delegate()->Exit(); + } + else { + state_ = IDLE; + } } } diff --git a/ash/accelerators/exit_warning_handler.h b/ash/accelerators/exit_warning_handler.h index 5b3d718e..3cb96f2 100644 --- a/ash/accelerators/exit_warning_handler.h +++ b/ash/accelerators/exit_warning_handler.h @@ -7,6 +7,7 @@ #include "ash/ash_export.h" #include "base/timer.h" +#include "ui/base/accelerators/accelerator.h" namespace views { class Widget; @@ -19,12 +20,16 @@ namespace ash { // a period of time. During that time we show a popup informing the // user of this. We exit only if the user holds the shortcut longer // than this time limit. -// An expert user may quickly release and then press again (double press) -// for immediate exit. The press, release and press must happen within -// the double press time limit. -// If the user releases (without double press) before the required hold -// time, we will cancel the exit, but show the ui until the hold time limit -// has expired to avoid a short popup flash. +// An expert user may double press the shortcut for immediate exit. +// The double press must happen within the double press time limit. +// Unless the user performs a double press, we show the ui until the +// hold time limit has expired to avoid a short popup flash. +// +// Notes: +// +// The corresponding accelerator must be non-repeatable (see +// kNonrepeatableActions in accelerator_table.cc). Otherwise the "Double Press +// Exit" will be activated just by holding it down, i.e. probably every time. // // State Transition Diagrams: // @@ -33,58 +38,56 @@ namespace ash { // // IDLE // | Press -// WAIT_FOR_QUICK_RELEASE action: show ui & start timers -// | Release (DT < T1) -// WAIT_FOR_DOUBLE_PRESS +// WAIT_FOR_DOUBLE_PRESS action: show ui & start timers // | Press (DT < T1) // EXITING action: hide ui, stop timers, exit // // IDLE // | Press -// WAIT_FOR_QUICK_RELEASE action: show ui & start timers +// WAIT_FOR_DOUBLE_PRESS action: show ui & start timers // | T1 timer expires // WAIT_FOR_LONG_HOLD -// | T2 Timer exipres +// | T2 Timer exipres and +// | accelerator was held (matches current accelerator from context) // EXITING action: hide ui, exit // // IDLE // | Press -// WAIT_FOR_QUICK_RELEASE action: show ui & start timers -// | T1 timer expiers +// WAIT_FOR_DOUBLE_PRESS action: show ui & start timers +// | T1 timer expires // WAIT_FOR_LONG_HOLD -// | Release -// CANCELED -// | T2 timer expires +// | T2 Timer exipres and +// | accelerator was not held // IDLE action: hide ui // // IDLE // | Press -// WAIT_FOR_QUICK_RELEASE action: show ui & start timers -// | Release (DT < T1) -// WAIT_FOR_DOUBLE_PRESS +// WAIT_FOR_DOUBLE_PRESS action: show ui & start timers // | T1 timer expires +// WAIT_FOR_LONG_HOLD +// | Press // CANCELED // | T2 timer expires // IDLE action: hide ui // +class AcceleratorControllerContext; class AcceleratorControllerTest; class ASH_EXPORT ExitWarningHandler { public: - ExitWarningHandler(); + explicit ExitWarningHandler(AcceleratorControllerContext* context); ~ExitWarningHandler(); - // Handles shortcut key press and release (Ctrl-Shift-Q). - void HandleExitKey(bool press); + // Handles accelerator for exit (Ctrl-Shift-Q). + void HandleAccelerator(); private: friend class AcceleratorControllerTest; enum State { IDLE, - WAIT_FOR_QUICK_RELEASE, WAIT_FOR_DOUBLE_PRESS, WAIT_FOR_LONG_HOLD, CANCELED, @@ -107,6 +110,11 @@ class ASH_EXPORT ExitWarningHandler { void Show(); void Hide(); + // AcceleratorControllerContext is used when a timer expires to test + // whether the user has held the accelerator key combination or has + // released (at least) one of the keys. + AcceleratorControllerContext* context_; + ui::Accelerator accelerator_; State state_; views::Widget* widget_; // owned by |this|. base::OneShotTimer<ExitWarningHandler> timer1_; // short; double press diff --git a/chrome/browser/ui/views/accelerator_table.cc b/chrome/browser/ui/views/accelerator_table.cc index 66b238c..a8b416c 100644 --- a/chrome/browser/ui/views/accelerator_table.cc +++ b/chrome/browser/ui/views/accelerator_table.cc @@ -164,7 +164,7 @@ struct ChromeCmdId2AshActionId { }; const ChromeCmdId2AshActionId kChromeCmdId2AshActionId[] = { { IDC_FEEDBACK, ash::OPEN_FEEDBACK_PAGE }, - { IDC_EXIT, ash::EXIT_PRESSED }, + { IDC_EXIT, ash::EXIT }, { IDC_NEW_INCOGNITO_WINDOW, ash::NEW_INCOGNITO_WINDOW }, { IDC_NEW_TAB, ash::NEW_TAB }, { IDC_NEW_WINDOW, ash::NEW_WINDOW }, |