Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(199)

Unified Diff: chrome/browser/ui/views/accelerator_table_unittest_mac.mm

Issue 2074643003: MacViews: Views accelerators table should match the Cocoa one. (Closed) Base URL: ssh://bitbucket.browser.yandex-team.ru/chromium/src.git@master
Patch Set: GetWindowKeyboardShortcutTable is now shared between Cocoa and MacViews. Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/views/accelerator_table_unittest_mac.mm
diff --git a/chrome/browser/ui/views/accelerator_table_unittest_mac.mm b/chrome/browser/ui/views/accelerator_table_unittest_mac.mm
new file mode 100644
index 0000000000000000000000000000000000000000..fc52134684c8de51e52983d289b1ec4c34540505
--- /dev/null
+++ b/chrome/browser/ui/views/accelerator_table_unittest_mac.mm
@@ -0,0 +1,146 @@
+// Copyright 2016 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 <stddef.h>
+
+#include <set>
+
+#include "base/stl_util.h"
+#include "build/build_config.h"
+#include "chrome/app/chrome_command_ids.h"
+#import "chrome/browser/global_keyboard_shortcuts_mac.h"
+#include "chrome/browser/ui/views/accelerator_table.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/events/event_constants.h"
+#import "ui/events/keycodes/keyboard_code_conversion_mac.h"
+
+using namespace chrome;
+
+namespace {
+
+struct KeyboardShortcutDataComparator {
+ bool operator()(const KeyboardShortcutData& lhs,
+ const KeyboardShortcutData& rhs) const {
+ return std::tie(lhs.command_key, lhs.shift_key, lhs.cntrl_key, lhs.opt_key,
+ lhs.vkey_code, lhs.key_char, lhs.chrome_command) <
+ std::tie(rhs.command_key, rhs.shift_key, rhs.cntrl_key, rhs.opt_key,
+ rhs.vkey_code, rhs.key_char, rhs.chrome_command);
+ }
+};
+
+std::set<KeyboardShortcutData, KeyboardShortcutDataComparator>
+AsKeyboardShortcutDataSet(const std::vector<AcceleratorMapping>& accelerators) {
+ std::set<KeyboardShortcutData, KeyboardShortcutDataComparator> result;
+ for (auto& entry : accelerators) {
+ unichar character;
+ unichar shifted_character;
+ const int vkey_code = ui::MacKeyCodeForWindowsKeyCode(
+ entry.keycode, entry.modifiers, &shifted_character,
+ &character);
+
+ result.insert({entry.modifiers & ui::EF_COMMAND_DOWN,
+ entry.modifiers & ui::EF_SHIFT_DOWN,
+ entry.modifiers & ui::EF_CONTROL_DOWN,
+ entry.modifiers & ui::EF_ALT_DOWN,
+ vkey_code,
+ 0,
+ entry.command_id});
+ }
+ return result;
+}
+
+void VerifyHasDuplicates(const std::vector<KeyboardShortcutData>& table,
+ const std::string& table_name) {
+ const auto accelerators(AsKeyboardShortcutDataSet(GetAcceleratorList()));
+
+ // These accelerators are impossible to migrate to accelerator_table.cc
+ // (crbug.com/25946).
+ const std::set<KeyboardShortcutData, KeyboardShortcutDataComparator>
+ unique_accelerators = {
+ {true, false, false, false, 0, '}', IDC_SELECT_NEXT_TAB},
+ {true, false, false, false, 0, '{', IDC_SELECT_PREVIOUS_TAB}};
+
+ for (const auto& table_entry : table) {
+ if (unique_accelerators.find(table_entry) != unique_accelerators.end())
+ continue;
+
+ EXPECT_TRUE(accelerators.find(table_entry) != accelerators.end())
+ << "Unique command: " << table_entry.chrome_command << " in table "
+ << table_name;
+ }
+}
+
+void VerifyTableDoesntHaveDuplicates(
+ const std::vector<KeyboardShortcutData>& table,
+ const std::string& table_name) {
+ const auto accelerators(AsKeyboardShortcutDataSet(GetAcceleratorList()));
+
+ for (const auto& table_entry : table) {
+ EXPECT_TRUE(accelerators.find(table_entry) == accelerators.end())
+ << "Duplicate command: " << table_entry.chrome_command
+ << " in table " << table_name;
+ }
+}
+
+} // namespace
+
+// Vefifies that only the whitelisted accelerators could have Control key
+// modifier, while running on macOS.
+TEST(AcceleratorTableTest, CheckMacOSControlAccelerators) {
+ // Only the accelerators that also work in Cocoa browser are allowed to appear
+ // on this whitelist.
+ const std::set<int> whitelisted_control_shortcuts = {
+ IDC_SELECT_NEXT_TAB,
+ IDC_SELECT_PREVIOUS_TAB,
+ IDC_FULLSCREEN,
+ };
+
+ const std::vector<AcceleratorMapping> accelerators(GetAcceleratorList());
+
+ // Control modifier is rarely used on Mac, and all valid uses must be
+ // whitelisted.
+ for (const auto& entry : accelerators) {
+ if (base::ContainsKey(whitelisted_control_shortcuts, entry.command_id))
+ continue;
+ EXPECT_FALSE(entry.modifiers & ui::EF_CONTROL_DOWN)
+ << "Found non-whitelisted accelerator that contains Control "
+ "modifier: " << entry.command_id;
+ }
+
+ // Test that whitelist is not outdated.
+ for (const auto& whitelist_entry : whitelisted_control_shortcuts) {
+ const auto entry =
+ std::find_if(accelerators.begin(), accelerators.end(),
+ [whitelist_entry](const AcceleratorMapping& a) {
+ return a.command_id == whitelist_entry &&
+ (a.modifiers & ui::EF_CONTROL_DOWN) != 0;
+ });
+ EXPECT_NE(entry, accelerators.end())
+ << "Whitelisted accelerator not found in the actual list: "
+ << whitelist_entry;
+ }
+}
+
+// Verifies that Alt-only (or with just Shift) accelerators are not present in
+// the list.
+TEST(AcceleratorTableTest, CheckMacOSAltAccelerators) {
+ const int kNonShiftMask =
+ ui::EF_COMMAND_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN;
+ for (const auto& entry : GetAcceleratorList()) {
+ EXPECT_FALSE((entry.modifiers & kNonShiftMask) == ui::EF_ALT_DOWN)
+ << "Found accelerator that uses solely Alt modifier: "
+ << entry.command_id;
+ }
+}
+
+// Verifies that we're not processing any duplicate accelerators in
+// global_keyboard_shortcuts_mac.mm functions.
+TEST(AcceleratorTableTest, CheckNoDuplicatesGlobalKeyboardShortcutsMac) {
+ VerifyHasDuplicates(GetWindowKeyboardShortcutTable(),
+ "WindowKeyboardShortcutTable");
+ VerifyTableDoesntHaveDuplicates(GetDelayedWindowKeyboardShortcutTable(),
+ "DelayedWindowKeyboardShortcutTable");
+ VerifyTableDoesntHaveDuplicates(GetBrowserKeyboardShortcutTable(),
+ "BrowserKeyboardShortcutTable");
+}

Powered by Google App Engine
This is Rietveld 408576698