Add test to check search key based accelerators.

ash/common/accelerators/accelerator_table_unittest.cc

 // Copyright (c) 2012 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 <set>
 
 #include "ash/common/accelerators/accelerator_table.h"
 #include "base/strings/string_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace ash {
 
 namespace {
 
 struct Cmp {
   bool operator()(const AcceleratorData& lhs, const AcceleratorData& rhs) {
     if (lhs.trigger_on_press != rhs.trigger_on_press)
       return lhs.trigger_on_press < rhs.trigger_on_press;
     if (lhs.keycode != rhs.keycode)
       return lhs.keycode < rhs.keycode;
     return lhs.modifiers < rhs.modifiers;
     // Do not check |action|.
   }
 };
 
+const int kDebugModifier =

[Daniel Erat, 2017/04/05 23:32:51]

this looks like it's only used in one place. since the table explicitly
shouldn't be updated, would it be cleaner to remove this and just inline the
values for the single entry that needs them?

[wutao, 2017/04/06 17:38:51]

Changed method to number and hash based, so do not need this any more.

+    ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN | ui::EF_SHIFT_DOWN;
+
+// This table snapshot was added on 2017-04-05 to white list these
+// accelerators having no Search in their modifiers. Please do not modify this
+// table. All new accelerators must base on Search and should be approved by UX.
+const AcceleratorData kAcceleratorData_WhiteList20170405[] = {
+    {true, ui::VKEY_SPACE, ui::EF_CONTROL_DOWN, PREVIOUS_IME},
+    {false, ui::VKEY_SPACE, ui::EF_CONTROL_DOWN, PREVIOUS_IME},
+    {true, ui::VKEY_CONVERT, ui::EF_NONE, SWITCH_IME},
+    {true, ui::VKEY_NONCONVERT, ui::EF_NONE, SWITCH_IME},
+    {true, ui::VKEY_DBE_SBCSCHAR, ui::EF_NONE, SWITCH_IME},
+    {true, ui::VKEY_DBE_DBCSCHAR, ui::EF_NONE, SWITCH_IME},
+    {true, ui::VKEY_HANGUL, ui::EF_NONE, SWITCH_IME},
+    {true, ui::VKEY_TAB, ui::EF_ALT_DOWN, CYCLE_FORWARD_MRU},
+    {true, ui::VKEY_TAB, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN,
+     CYCLE_BACKWARD_MRU},
+    {true, ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_NONE, TOGGLE_OVERVIEW},
+    {true, ui::VKEY_BROWSER_SEARCH, ui::EF_NONE, TOGGLE_APP_LIST},
+    {true, ui::VKEY_WLAN, ui::EF_NONE, TOGGLE_WIFI},
+    {true, ui::VKEY_KBD_BRIGHTNESS_DOWN, ui::EF_NONE, KEYBOARD_BRIGHTNESS_DOWN},
+    {true, ui::VKEY_KBD_BRIGHTNESS_UP, ui::EF_NONE, KEYBOARD_BRIGHTNESS_UP},
+    {true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_CONTROL_DOWN, TOGGLE_MIRROR_MODE},
+    {true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_ALT_DOWN, SWAP_PRIMARY_DISPLAY},
+    {true, ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_CONTROL_DOWN, TAKE_SCREENSHOT},
+    {true, ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+     TAKE_PARTIAL_SCREENSHOT},
+    {true, ui::VKEY_MEDIA_LAUNCH_APP1, ui::EF_ALT_DOWN | ui::EF_CONTROL_DOWN,
+     TAKE_WINDOW_SCREENSHOT},
+    {true, ui::VKEY_BRIGHTNESS_DOWN, ui::EF_NONE, BRIGHTNESS_DOWN},
+    {true, ui::VKEY_BRIGHTNESS_DOWN, ui::EF_ALT_DOWN, KEYBOARD_BRIGHTNESS_DOWN},
+    {true, ui::VKEY_BRIGHTNESS_UP, ui::EF_NONE, BRIGHTNESS_UP},
+    {true, ui::VKEY_BRIGHTNESS_UP, ui::EF_ALT_DOWN, KEYBOARD_BRIGHTNESS_UP},
+    {true, ui::VKEY_BRIGHTNESS_DOWN, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+     MAGNIFY_SCREEN_ZOOM_OUT},
+    {true, ui::VKEY_BRIGHTNESS_UP, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+     MAGNIFY_SCREEN_ZOOM_IN},
+    {true, ui::VKEY_F13, ui::EF_NONE, LOCK_PRESSED},
+    {false, ui::VKEY_F13, ui::EF_NONE, LOCK_RELEASED},
+    {true, ui::VKEY_SLEEP, ui::EF_NONE, LOCK_PRESSED},
+    {false, ui::VKEY_SLEEP, ui::EF_NONE, LOCK_RELEASED},
+    {true, ui::VKEY_POWER, ui::EF_NONE, POWER_PRESSED},
+    {false, ui::VKEY_POWER, ui::EF_NONE, POWER_RELEASED},
+    {true, ui::VKEY_M, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, OPEN_FILE_MANAGER},
+    {true, ui::VKEY_OEM_2, ui::EF_CONTROL_DOWN, OPEN_GET_HELP},
+    {true, ui::VKEY_OEM_2, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+     OPEN_GET_HELP},
+    {true, ui::VKEY_T, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN, OPEN_CROSH},
+    {true, ui::VKEY_I, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+     TOUCH_HUD_MODE_CHANGE},
+    {true, ui::VKEY_I,
+     ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN | ui::EF_SHIFT_DOWN,
+     TOUCH_HUD_CLEAR},
+    {true, ui::VKEY_P, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+     TOUCH_HUD_PROJECTION_TOGGLE},
+    {true, ui::VKEY_Z, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+     TOGGLE_SPOKEN_FEEDBACK},
+    {true, ui::VKEY_OEM_COMMA, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+     SWITCH_TO_PREVIOUS_USER},
+    {true, ui::VKEY_OEM_PERIOD, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+     SWITCH_TO_NEXT_USER},
+    {false, ui::VKEY_LSHIFT, ui::EF_NONE, DISABLE_CAPS_LOCK},
+    {false, ui::VKEY_SHIFT, ui::EF_NONE, DISABLE_CAPS_LOCK},
+    {false, ui::VKEY_RSHIFT, ui::EF_NONE, DISABLE_CAPS_LOCK},
+    {false, ui::VKEY_LWIN, ui::EF_ALT_DOWN, TOGGLE_CAPS_LOCK},
+    {true, ui::VKEY_VOLUME_MUTE, ui::EF_NONE, VOLUME_MUTE},
+    {true, ui::VKEY_VOLUME_DOWN, ui::EF_NONE, VOLUME_DOWN},
+    {true, ui::VKEY_VOLUME_UP, ui::EF_NONE, VOLUME_UP},
+    {true, ui::VKEY_SPACE, ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN, NEXT_IME},
+    {true, ui::VKEY_I, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, OPEN_FEEDBACK_PAGE},
+    {true, ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, EXIT},
+    {true, ui::VKEY_N, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+     NEW_INCOGNITO_WINDOW},
+    {true, ui::VKEY_N, ui::EF_CONTROL_DOWN, NEW_WINDOW},
+    {true, ui::VKEY_T, ui::EF_CONTROL_DOWN, NEW_TAB},
+    {true, ui::VKEY_OEM_MINUS, ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN,
+     SCALE_UI_UP},
+    {true, ui::VKEY_OEM_PLUS, ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN,
+     SCALE_UI_DOWN},
+    {true, ui::VKEY_0, ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN, SCALE_UI_RESET},
+    {true, ui::VKEY_BROWSER_REFRESH, ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN,
+     ROTATE_SCREEN},
+    {true, ui::VKEY_BROWSER_REFRESH,
+     ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN, ROTATE_WINDOW},
+    {true, ui::VKEY_T, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, RESTORE_TAB},
+    {true, ui::VKEY_PRINT, ui::EF_NONE, TAKE_SCREENSHOT},
+    {false, ui::VKEY_LWIN, ui::EF_NONE, TOGGLE_APP_LIST},
+    {true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_NONE, TOGGLE_FULLSCREEN},
+    {true, ui::VKEY_MEDIA_LAUNCH_APP2, ui::EF_SHIFT_DOWN, TOGGLE_FULLSCREEN},
+    {true, ui::VKEY_L, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, FOCUS_SHELF},
+    {true, ui::VKEY_HELP, ui::EF_NONE, SHOW_KEYBOARD_OVERLAY},
+    {true, ui::VKEY_OEM_2, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+     SHOW_KEYBOARD_OVERLAY},
+    {true, ui::VKEY_OEM_2,
+     ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
+     SHOW_KEYBOARD_OVERLAY},
+    {true, ui::VKEY_F14, ui::EF_NONE, SHOW_KEYBOARD_OVERLAY},
+    {true, ui::VKEY_N, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN,
+     SHOW_MESSAGE_CENTER_BUBBLE},
+    {true, ui::VKEY_P, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, SHOW_STYLUS_TOOLS},
+    {true, ui::VKEY_S, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN,
+     SHOW_SYSTEM_TRAY_BUBBLE},
+    {true, ui::VKEY_1, ui::EF_ALT_DOWN, LAUNCH_APP_0},
+    {true, ui::VKEY_2, ui::EF_ALT_DOWN, LAUNCH_APP_1},
+    {true, ui::VKEY_3, ui::EF_ALT_DOWN, LAUNCH_APP_2},
+    {true, ui::VKEY_4, ui::EF_ALT_DOWN, LAUNCH_APP_3},
+    {true, ui::VKEY_5, ui::EF_ALT_DOWN, LAUNCH_APP_4},
+    {true, ui::VKEY_6, ui::EF_ALT_DOWN, LAUNCH_APP_5},
+    {true, ui::VKEY_7, ui::EF_ALT_DOWN, LAUNCH_APP_6},
+    {true, ui::VKEY_8, ui::EF_ALT_DOWN, LAUNCH_APP_7},
+    {true, ui::VKEY_9, ui::EF_ALT_DOWN, LAUNCH_LAST_APP},
+    {true, ui::VKEY_OEM_4, ui::EF_ALT_DOWN, WINDOW_CYCLE_SNAP_LEFT},
+    {true, ui::VKEY_OEM_6, ui::EF_ALT_DOWN, WINDOW_CYCLE_SNAP_RIGHT},
+    {true, ui::VKEY_OEM_MINUS, ui::EF_ALT_DOWN, WINDOW_MINIMIZE},
+    {true, ui::VKEY_OEM_PLUS, ui::EF_ALT_DOWN, TOGGLE_MAXIMIZED},
+    {true, ui::VKEY_OEM_PLUS, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN,
+     WINDOW_POSITION_CENTER},
+    {true, ui::VKEY_BROWSER_FORWARD, ui::EF_CONTROL_DOWN, FOCUS_NEXT_PANE},
+    {true, ui::VKEY_BROWSER_BACK, ui::EF_CONTROL_DOWN, FOCUS_PREVIOUS_PANE},
+    {true, ui::VKEY_MEDIA_NEXT_TRACK, ui::EF_NONE, MEDIA_NEXT_TRACK},
+    {true, ui::VKEY_MEDIA_PLAY_PAUSE, ui::EF_NONE, MEDIA_PLAY_PAUSE},
+    {true, ui::VKEY_MEDIA_PREV_TRACK, ui::EF_NONE, MEDIA_PREV_TRACK},
+    {true, ui::VKEY_U, kDebugModifier, PRINT_UI_HIERARCHIES},
+};
+
+const size_t kAcceleratorDataWhiteListLength =
+    arraysize(kAcceleratorData_WhiteList20170405);
+
 }  // namespace
 
 TEST(AcceleratorTableTest, CheckDuplicatedAccelerators) {
   std::set<AcceleratorData, Cmp> accelerators;
   for (size_t i = 0; i < kAcceleratorDataLength; ++i) {
     const AcceleratorData& entry = kAcceleratorData[i];
     EXPECT_TRUE(accelerators.insert(entry).second)
-        << "Duplicated accelerator: " << entry.trigger_on_press << ", "
-        << entry.keycode << ", " << (entry.modifiers & ui::EF_SHIFT_DOWN)
-        << ", " << (entry.modifiers & ui::EF_CONTROL_DOWN) << ", "
-        << (entry.modifiers & ui::EF_ALT_DOWN);
+        << "Duplicated accelerator:"
+        << " trigger_on_press=" << entry.trigger_on_press

[Daniel Erat, 2017/04/05 23:32:51]

since this logging code appears multiple times, how about adding an
AcceleratorDataToString() helper method in the anon namespace at the top of the
file that formats and returns a string?

[wutao, 2017/04/06 17:38:51]

Done.

+        << " keycode=" << entry.keycode
+        << " SEARCH=" << (entry.modifiers & ui::EF_COMMAND_DOWN)
+        << " SHIFT=" << (entry.modifiers & ui::EF_SHIFT_DOWN)
+        << " CONTROL=" << (entry.modifiers & ui::EF_CONTROL_DOWN)
+        << " ALT=" << (entry.modifiers & ui::EF_ALT_DOWN);
   }
 }
 
 TEST(AcceleratorTableTest, CheckDuplicatedReservedActions) {
   std::set<AcceleratorAction> actions;
   for (size_t i = 0; i < kReservedActionsLength; ++i) {
     EXPECT_TRUE(actions.insert(kReservedActions[i]).second)
         << "Duplicated action: " << kReservedActions[i];
   }
 }
@@ skipping 26 matching lines @@
         << "Duplicated action: " << kRepeatableActions[i] << " at index: " << i;
   }
 }
 
 TEST(AcceleratorTableTest, CheckDeprecatedAccelerators) {
   std::set<AcceleratorData, Cmp> deprecated_actions;
   for (size_t i = 0; i < kDeprecatedAcceleratorsLength; ++i) {
     // A deprecated action can never appear twice in the list.
     const AcceleratorData& entry = kDeprecatedAccelerators[i];
     EXPECT_TRUE(deprecated_actions.insert(entry).second)
-        << "Duplicate deprecated accelerator: " << entry.trigger_on_press
-        << ", " << entry.keycode << ", "
-        << (entry.modifiers & ui::EF_SHIFT_DOWN) << ", "
-        << (entry.modifiers & ui::EF_CONTROL_DOWN) << ", "
-        << (entry.modifiers & ui::EF_ALT_DOWN);
+        << "Duplicate deprecated accelerator:"
+        << " trigger_on_press=" << entry.trigger_on_press
+        << " keycode=" << entry.keycode
+        << " SEARCH=" << (entry.modifiers & ui::EF_COMMAND_DOWN)
+        << " SHIFT=" << (entry.modifiers & ui::EF_SHIFT_DOWN)
+        << " CONTROL=" << (entry.modifiers & ui::EF_CONTROL_DOWN)
+        << " ALT=" << (entry.modifiers & ui::EF_ALT_DOWN);
   }
 
   std::set<AcceleratorAction> actions;
   for (size_t i = 0; i < kDeprecatedAcceleratorsDataLength; ++i) {
     // There must never be any duplicated actions.
     const DeprecatedAcceleratorData& data = kDeprecatedAcceleratorsData[i];
     EXPECT_TRUE(actions.insert(data.action).second) << "Deprecated action: "
                                                     << data.action;
 
     // The UMA histogram name must be of the format "Ash.Accelerators.*"
     std::string uma_histogram(data.uma_histogram_name);
     EXPECT_TRUE(base::StartsWith(uma_histogram, "Ash.Accelerators.",
                                  base::CompareCase::SENSITIVE));
   }
 }
 
+// If this test fails, please change the accelerator modifiers to include the
+// Search key. All new accelerators should be approved by UX.
+TEST(AcceleratorTableTest, CheckSearchBasedAccelerators) {
+  std::set<AcceleratorData, Cmp> achived_accelerators;
+  for (size_t i = 0; i < kAcceleratorDataWhiteListLength; ++i)
+    achived_accelerators.insert(kAcceleratorData_WhiteList20170405[i]);
+
+  for (size_t i = 0; i < kAcceleratorDataLength; ++i) {
+    const AcceleratorData& entry = kAcceleratorData[i];
+    if (achived_accelerators.find(entry) != achived_accelerators.end())
+      continue;
+    EXPECT_TRUE(entry.modifiers & ui::EF_COMMAND_DOWN)
+        << "Non-Search-based accelerator:"
+        << " keycode=" << entry.keycode
+        << " SHIFT=" << (entry.modifiers & ui::EF_SHIFT_DOWN)
+        << " CONTROL=" << (entry.modifiers & ui::EF_CONTROL_DOWN)
+        << " ALT=" << (entry.modifiers & ui::EF_ALT_DOWN);
+  }
+}
+
 }  // namespace ash