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..cb6c7730d3143149bd0d84a103c61ba30d257fd8 |
--- /dev/null |
+++ b/chrome/browser/ui/views/accelerator_table_unittest_mac.mm |
@@ -0,0 +1,182 @@ |
+// 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 "build/build_config.h" |
+#include "chrome/app/chrome_command_ids.h" |
+#include "chrome/browser/global_keyboard_shortcuts_mac.h" |
tapted
2016/10/31 10:16:57
nit: import
themblsha
2016/10/31 16:56:29
Done.
|
+#include "chrome/browser/ui/views/accelerator_table.h" |
+#include "testing/gtest/include/gtest/gtest.h" |
+#include "ui/events/event_constants.h" |
+#include "ui/events/keycodes/keyboard_code_conversion_mac.h" |
tapted
2016/10/31 10:16:57
nit: import
themblsha
2016/10/31 16:56:29
Done.
|
+ |
+namespace chrome { |
+ |
+// 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> accelerator_list(GetAcceleratorList()); |
+ |
+ // Control modifier is rarely used on Mac, and all valid uses must be |
+ // whitelisted. |
+ for (const auto& entry : accelerator_list) { |
+ if (whitelisted_control_shortcuts.count(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(accelerator_list.begin(), accelerator_list.end(), |
+ [whitelist_entry](const AcceleratorMapping& a) { |
+ return a.command_id == whitelist_entry && |
+ a.modifiers & ui::EF_CONTROL_DOWN; |
tapted
2016/10/31 10:16:57
nit: (a.modifiers & ui::EF_CONTROL_DOWN) != 0
themblsha
2016/10/31 16:56:29
Done.
|
+ }); |
+ EXPECT_TRUE(entry != accelerator_list.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; |
+ } |
+} |
+ |
+// Vefifies that accelerator filtering works correctly and we have an |
+// alternative shortcut for each filtered-out accelerator. |
+TEST(AcceleratorTableTest, CheckMacOSFilteredOutAcceleratorsHaveAlternatives) { |
+ // It's okay not to have an alternative accelerator for these commands as |
+ // there's no alternative for them in the Cocoa browser. |
+ const std::set<int> whitelisted_unique_shortcuts = { |
+ IDC_FOCUS_TOOLBAR, |
+ IDC_FOCUS_BOOKMARKS, |
+ IDC_FOCUS_INFOBARS, |
+ IDC_SHOW_APP_MENU, |
+ }; |
+ |
+ const std::vector<AcceleratorMapping> accelerator_list(GetAcceleratorList()); |
+ const std::vector<AcceleratorMapping> full_accelerator_list( |
+ GetUnfilteredAcceleratorListForTesting()); |
+ |
+ // Ensure that we still have alternative mappings for all filtered-out |
+ // accelerators. |
+ std::vector<AcceleratorMapping> filtered_list(full_accelerator_list); |
+ auto remove_start = |
+ std::remove_if(filtered_list.begin(), filtered_list.end(), |
+ [&accelerator_list](const AcceleratorMapping& m1) { |
+ // Remove all accelerators that are present in |accelerator_list| |
+ return std::find_if(accelerator_list.begin(), accelerator_list.end(), |
tapted
2016/10/31 10:16:57
This is a bit mind-blowing. I'd suggest declaring
themblsha
2016/10/31 16:56:29
Whoa, std::tie makes comparators sooo easy. Replac
|
+ [&m1](const AcceleratorMapping& m2) { |
+ return m1.command_id == m2.command_id && |
+ m1.keycode == m2.keycode && |
+ m1.modifiers == m2.modifiers; |
+ }) != accelerator_list.end(); |
+ }); |
+ filtered_list.erase(remove_start, filtered_list.end()); |
+ EXPECT_GT(filtered_list.size(), 1u); |
+ |
+ for (const auto& removed_entry : filtered_list) { |
+ if (whitelisted_unique_shortcuts.count(removed_entry.command_id)) |
+ continue; |
+ |
+ const auto entry = |
+ std::find_if(accelerator_list.begin(), accelerator_list.end(), |
+ [&removed_entry](const AcceleratorMapping& m) { |
+ return removed_entry.command_id == m.command_id; |
+ }); |
+ EXPECT_TRUE(entry != accelerator_list.end()) |
+ << "Filtered command doesn't have an alternative mapping: " |
+ << removed_entry.command_id; |
+ } |
+ |
+ for (const auto& whitelist_entry : whitelisted_unique_shortcuts) { |
+ const auto entry = |
+ std::find_if(filtered_list.begin(), filtered_list.end(), |
+ [&whitelist_entry](const AcceleratorMapping& m) { |
+ return whitelist_entry == m.command_id; |
+ }); |
+ EXPECT_TRUE(entry != filtered_list.end()) |
+ << "Whitelisted accelerator not found in the filtered-out list: " |
+ << whitelist_entry; |
+ } |
+} |
+ |
+namespace { |
+ |
+static void VerifyTable( |
tapted
2016/10/31 10:16:57
nit: no `static`
themblsha
2016/10/31 16:56:29
Done.
|
+ const KeyboardShortcutData* (*get_keyboard_shortcut_table)(size_t*), |
+ const std::string& table_name) { |
+ const std::vector<AcceleratorMapping> accelerator_list(GetAcceleratorList()); |
+ |
+ size_t num_shortcuts = 0; |
+ const KeyboardShortcutData *it = get_keyboard_shortcut_table(&num_shortcuts); |
+ for (size_t i = 0; i < num_shortcuts; ++i, ++it) { |
+ int modifiers = 0; |
+ if (it->command_key) |
+ modifiers |= ui::EF_COMMAND_DOWN; |
+ if (it->shift_key) |
+ modifiers |= ui::EF_SHIFT_DOWN; |
+ if (it->cntrl_key) |
+ modifiers |= ui::EF_CONTROL_DOWN; |
+ if (it->opt_key) |
+ modifiers |= ui::EF_ALT_DOWN; |
+ |
+ for (const auto& entry : accelerator_list) { |
+ unichar character; |
+ unichar shifted_character; |
+ const int vkey_code = ui::MacKeyCodeForWindowsKeyCode( |
+ entry.keycode, entry.modifiers, &shifted_character, &character); |
+ |
+ if (it->vkey_code) { |
+ if (modifiers == entry.modifiers && it->vkey_code == vkey_code && |
+ it->chrome_command == entry.command_id) { |
+ EXPECT_TRUE(false) << "Duplicate command: " << entry.command_id |
+ << " in table " << table_name; |
+ } |
+ } else { |
+ if (modifiers == entry.modifiers && |
+ (it->key_char == character || it->key_char == shifted_character) && |
+ it->chrome_command == entry.command_id) { |
+ EXPECT_TRUE(false) << "Duplicate command: " << entry.command_id |
+ << " in table " << table_name; |
+ } |
+ } |
+ } |
+ } |
+} |
+ |
+} // namespace |
+ |
+// Verifies that we're not processing any duplicate accelerators in |
+// global_keyboard_shortcuts_mac.mm functions. |
+TEST(AcceleratorTableTest, CheckNoDuplicatesGlobalKeyboardShortcutsMac) { |
+ VerifyTable(GetWindowKeyboardShortcutTable, "WindowKeyboardShortcutTable"); |
+ VerifyTable(GetDelayedWindowKeyboardShortcutTable, |
+ "DelayedWindowKeyboardShortcutTable"); |
+ VerifyTable(GetBrowserKeyboardShortcutTable, "BrowserKeyboardShortcutTable"); |
+} |
+ |
+} // namespace chrome |