Commit 5883ad59 authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

[Chrome OS] Revert recent changes to IDC_EXIT accelerator handling.

In db110f78, IDC_EXIT was changed to be
handled by Chrome instead of Ash. This reverts that change.

Originally, I thought Chrome would have to register the IDC_EXIT command
so that menus would show that shortcut in the relevant row. However,
the Exit command isn't shown in the Chrome OS app menu so it should be
fine to skip registering it completely and let Ash handle it.

Also verified that ctrl+shift+q is still treated as reserved, i.e. cannot
be eaten by a webpage. This site is useful for manually testing that:
  http://unixpapa.com/js/testkey.html


Bug: 834092
Change-Id: Ie6de31a0efa36da0f67def3ba7a5124b6c527bd3
Reviewed-on: https://chromium-review.googlesource.com/1024876
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553613}
parent c06c5deb
...@@ -2,7 +2,9 @@ ...@@ -2,7 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "base/run_loop.h"
#include "base/test/histogram_tester.h" #include "base/test/histogram_tester.h"
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h" #include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/browser_shutdown.h" #include "chrome/browser/browser_shutdown.h"
#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chrome_notification_types.h"
...@@ -10,11 +12,13 @@ ...@@ -10,11 +12,13 @@
#include "chrome/browser/ui/browser_commands.h" #include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/browser_list_observer.h" #include "chrome/browser/ui/browser_list_observer.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h" #include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/notification_service.h" #include "content/public/browser/notification_service.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/events/test/event_generator.h"
using testing::_; using testing::_;
using testing::AtLeast; using testing::AtLeast;
...@@ -76,3 +80,34 @@ IN_PROC_BROWSER_TEST_F(BrowserShutdownBrowserTest, ...@@ -76,3 +80,34 @@ IN_PROC_BROWSER_TEST_F(BrowserShutdownBrowserTest,
1); 1);
} }
#endif // !defined(OS_CHROMEOS) #endif // !defined(OS_CHROMEOS)
// EventGenerator doesn't work on Mac. See https://crbug.com/814675
#if defined(OS_MACOSX)
#define MAYBE_ShutdownConfirmation DISABLED_ShutdownConfirmation
#else
#define MAYBE_ShutdownConfirmation ShutdownConfirmation
#endif
// On Chrome OS, the shutdown accelerator is handled by Ash and requires
// confirmation, so Chrome shouldn't try to shut down after it's been hit one
// time. Regression test for crbug.com/834092
IN_PROC_BROWSER_TEST_F(BrowserShutdownBrowserTest, MAYBE_ShutdownConfirmation) {
#if defined(OS_MACOSX)
const int modifiers = ui::EF_COMMAND_DOWN;
#else
const int modifiers = ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN;
#endif
ui::test::EventGenerator generator(browser()->window()->GetNativeWindow());
// Press the accelerator for quitting.
generator.PressKey(ui::VKEY_Q, modifiers);
generator.ReleaseKey(ui::VKEY_Q, modifiers);
base::RunLoop().RunUntilIdle();
#if defined(OS_CHROMEOS)
EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
#else
EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
#endif
}
...@@ -24,9 +24,9 @@ constexpr int kAshAcceleratorsTotalNum = 98; ...@@ -24,9 +24,9 @@ constexpr int kAshAcceleratorsTotalNum = 98;
// The hash of Ash accelerators. // The hash of Ash accelerators.
constexpr char kAshAcceleratorsHash[] = "71c96531d7639ba0ecf255a59546c243"; constexpr char kAshAcceleratorsHash[] = "71c96531d7639ba0ecf255a59546c243";
// The total number of Chrome accelerators (available on Chrome OS). // The total number of Chrome accelerators (available on Chrome OS).
constexpr int kChromeAcceleratorsTotalNum = 92; constexpr int kChromeAcceleratorsTotalNum = 91;
// The hash of Chrome accelerators (available on Chrome OS). // The hash of Chrome accelerators (available on Chrome OS).
constexpr char kChromeAcceleratorsHash[] = "b6e72a2f2c8e86938e645663e2b0d1fe"; constexpr char kChromeAcceleratorsHash[] = "cde825b73b85f0ff34a1ff78086e61c8";
const char* BooleanToString(bool value) { const char* BooleanToString(bool value) {
return value ? "true" : "false"; return value ? "true" : "false";
......
...@@ -152,6 +152,9 @@ const AcceleratorMapping kAcceleratorMap[] = { ...@@ -152,6 +152,9 @@ const AcceleratorMapping kAcceleratorMap[] = {
// handled via WM_APPCOMMAND. // handled via WM_APPCOMMAND.
{ui::VKEY_BROWSER_SEARCH, ui::EF_NONE, IDC_FOCUS_SEARCH}, {ui::VKEY_BROWSER_SEARCH, ui::EF_NONE, IDC_FOCUS_SEARCH},
{ui::VKEY_M, ui::EF_SHIFT_DOWN | kPlatformModifier, IDC_SHOW_AVATAR_MENU}, {ui::VKEY_M, ui::EF_SHIFT_DOWN | kPlatformModifier, IDC_SHOW_AVATAR_MENU},
#if !defined(OS_MACOSX)
{ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_EXIT},
#endif // !OS_MACOSX
#endif // !OS_CHROMEOS #endif // !OS_CHROMEOS
#if defined(GOOGLE_CHROME_BUILD) && !defined(OS_MACOSX) #if defined(GOOGLE_CHROME_BUILD) && !defined(OS_MACOSX)
...@@ -240,7 +243,6 @@ const AcceleratorMapping kAcceleratorMap[] = { ...@@ -240,7 +243,6 @@ const AcceleratorMapping kAcceleratorMap[] = {
{ui::VKEY_J, ui::EF_CONTROL_DOWN, IDC_SHOW_DOWNLOADS}, {ui::VKEY_J, ui::EF_CONTROL_DOWN, IDC_SHOW_DOWNLOADS},
{ui::VKEY_H, ui::EF_CONTROL_DOWN, IDC_SHOW_HISTORY}, {ui::VKEY_H, ui::EF_CONTROL_DOWN, IDC_SHOW_HISTORY},
{ui::VKEY_U, ui::EF_CONTROL_DOWN, IDC_VIEW_SOURCE}, {ui::VKEY_U, ui::EF_CONTROL_DOWN, IDC_VIEW_SOURCE},
{ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_EXIT},
#if !defined(OS_CHROMEOS) #if !defined(OS_CHROMEOS)
// On Chrome OS, these keys are assigned to change UI scale. // On Chrome OS, these keys are assigned to change UI scale.
{ui::VKEY_OEM_MINUS, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, {ui::VKEY_OEM_MINUS, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
......
...@@ -76,7 +76,7 @@ TEST(AcceleratorTableTest, CheckDuplicatedAcceleratorsAsh) { ...@@ -76,7 +76,7 @@ TEST(AcceleratorTableTest, CheckDuplicatedAcceleratorsAsh) {
ash_entry.action == ash::OPEN_FEEDBACK_PAGE || ash_entry.action == ash::OPEN_FEEDBACK_PAGE ||
#endif #endif
ash_entry.action == ash::RESTORE_TAB || ash_entry.action == ash::RESTORE_TAB ||
ash_entry.action == ash::NEW_TAB || ash_entry.action == ash::EXIT) { ash_entry.action == ash::NEW_TAB) {
AcceleratorMapping entry; AcceleratorMapping entry;
entry.keycode = ash_entry.keycode; entry.keycode = ash_entry.keycode;
entry.modifiers = ash_entry.modifiers; entry.modifiers = ash_entry.modifiers;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment