Do not use F11/12 for desks shortcut.

This was an workaround as these keys are remapped.
With new mapping, they're not remapped, so virtual desks
can just use OEM_PLUS / MINUS

Bug: 1064383
Change-Id: I7dc7b3dc8419e5489e043dbed1a4687ec3e63d4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2327866Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798760}

ash/accelerators/accelerator_controller_impl.cc

@@ -1798,8 +1798,11 @@ void AcceleratorControllerImpl::Init() {
   RegisterAccelerators(kAcceleratorData, kAcceleratorDataLength);
 
   if (::features::IsNewShortcutMappingEnabled()) {
-    RegisterAccelerators(kNewAdditionalAcceleratorData,
-                         kNewAdditionalAcceleratorDataLength);
+    RegisterAccelerators(kEnableWithNewMappingAcceleratorData,
+                         kEnableWithNewMappingAcceleratorDataLength);
+  } else {
+    RegisterAccelerators(kDisableWithNewMappingAcceleratorData,
+                         kDisableWithNewMappingAcceleratorDataLength);
   }
 
   RegisterDeprecatedAccelerators();

ash/accelerators/accelerator_table_unittest.cc

@@ -64,6 +64,11 @@ TEST(AcceleratorTableTest, CheckDuplicatedAccelerators) {
     EXPECT_TRUE(accelerators.insert(entry).second)
         << "Duplicated accelerator: " << AcceleratorDataToString(entry);
   }
+  for (size_t i = 0; i < kDisableWithNewMappingAcceleratorDataLength; ++i) {
+    const AcceleratorData& entry = kDisableWithNewMappingAcceleratorData[i];
+    EXPECT_TRUE(accelerators.insert(entry).second)
+        << "Duplicated accelerator: " << AcceleratorDataToString(entry);
+  }
 }
 
 TEST(AcceleratorTableTest, CheckDuplicatedReservedActions) {
@@ -144,6 +149,12 @@ TEST(AcceleratorTableTest, CheckSearchBasedAccelerators) {
       continue;
     non_search_accelerators.emplace_back(entry);
   }
+  for (size_t i = 0; i < kDisableWithNewMappingAcceleratorDataLength; ++i) {
+    const AcceleratorData& entry = kDisableWithNewMappingAcceleratorData[i];
+    if (entry.modifiers & ui::EF_COMMAND_DOWN)
+      continue;
+    non_search_accelerators.emplace_back(entry);
+  }
 
   const int accelerators_number = non_search_accelerators.size();
   EXPECT_EQ(accelerators_number, kNonSearchAcceleratorsNum)

ash/public/cpp/accelerators.cc

@@ -199,16 +199,6 @@ const AcceleratorData kAcceleratorData[] = {
     {true, ui::VKEY_U, kDebugModifier, PRINT_UI_HIERARCHIES},
 
     // Virtual Desks shortcuts.
-    // Desk creation and removal:
-    // Due to https://crbug.com/976487, Search + "=" is always automatically
-    // rewritten to F12, and so is Search + "-" to F11. So we had to implement
-    // the following two shortcuts as Shift + F11/F12 until we resolve the above
-    // issue, accepting the fact that these two shortcuts might sometimes be
-    // consumed by apps and pages (since they're not search-based).
-    // TODO(afakhry): Change the following to Search+Shift+"+"/"-" once
-    // https://crbug.com/976487 is fixed.
-    {true, ui::VKEY_F12, ui::EF_SHIFT_DOWN, DESKS_NEW_DESK},
-    {true, ui::VKEY_F11, ui::EF_SHIFT_DOWN, DESKS_REMOVE_CURRENT_DESK},
     // Desk activation:
     {true, ui::VKEY_OEM_4, ui::EF_COMMAND_DOWN, DESKS_ACTIVATE_DESK},
     {true, ui::VKEY_OEM_6, ui::EF_COMMAND_DOWN, DESKS_ACTIVATE_DESK},
@@ -226,7 +216,29 @@ const AcceleratorData kAcceleratorData[] = {
 
 const size_t kAcceleratorDataLength = base::size(kAcceleratorData);
 
-const AcceleratorData kNewAdditionalAcceleratorData[] = {
+const AcceleratorData kDisableWithNewMappingAcceleratorData[] = {
+    // Desk creation and removal:
+    // Due to https://crbug.com/976487, Search + "=" is always automatically
+    // rewritten to F12, and so is Search + "-" to F11. So we had to implement
+    // the following two shortcuts as Shift + F11/F12 until we resolve the above
+    // issue, accepting the fact that these two shortcuts might sometimes be
+    // consumed by apps and pages (since they're not search-based).
+    // TODO(afakhry): Change the following to Search+Shift+"+"/"-" once
+    // https://crbug.com/976487 is fixed.
+    {true, ui::VKEY_F12, ui::EF_SHIFT_DOWN, DESKS_NEW_DESK},
+    {true, ui::VKEY_F11, ui::EF_SHIFT_DOWN, DESKS_REMOVE_CURRENT_DESK},
+};
+
+const size_t kDisableWithNewMappingAcceleratorDataLength =
+    base::size(kDisableWithNewMappingAcceleratorData);
+
+const AcceleratorData kEnableWithNewMappingAcceleratorData[] = {
+    // Desk creation and removal:
+    {true, ui::VKEY_OEM_PLUS, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN,
+     DESKS_NEW_DESK},
+    {true, ui::VKEY_OEM_MINUS, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN,
+     DESKS_REMOVE_CURRENT_DESK},
+
     // Desk activation:
     {true, ui::VKEY_LEFT, ui::EF_COMMAND_DOWN | ui::EF_CONTROL_DOWN,
      DESKS_ACTIVATE_DESK},
@@ -261,15 +273,10 @@ const AcceleratorData kNewAdditionalAcceleratorData[] = {
     // Shortcut Viewer
     {true, ui::VKEY_OEM_2, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN,
      SHOW_SHORTCUT_VIEWER},
-
-    // Minimize / Maximize
-    {true, ui::VKEY_OEM_MINUS, ui::EF_COMMAND_DOWN, WINDOW_MINIMIZE},
-    {true, ui::VKEY_OEM_PLUS, ui::EF_SHIFT_DOWN | ui::EF_COMMAND_DOWN,
-     TOGGLE_MAXIMIZED},
 };
 
-const size_t kNewAdditionalAcceleratorDataLength =
-    base::size(kNewAdditionalAcceleratorData);
+const size_t kEnableWithNewMappingAcceleratorDataLength =
+    base::size(kEnableWithNewMappingAcceleratorData);
 
 // static
 AcceleratorController* AcceleratorController::Get() {

ash/public/cpp/accelerators.h

@@ -155,9 +155,16 @@ ASH_PUBLIC_EXPORT constexpr int kDebugModifier =
 ASH_PUBLIC_EXPORT extern const AcceleratorData kAcceleratorData[];
 ASH_PUBLIC_EXPORT extern const size_t kAcceleratorDataLength;
 
-// Experimental new additional accelerators. crbug.com/1067269
-ASH_PUBLIC_EXPORT extern const AcceleratorData kNewAdditionalAcceleratorData[];
-ASH_PUBLIC_EXPORT extern const size_t kNewAdditionalAcceleratorDataLength;
+// Accelerators that are enabled/disabled with new accelerator mapping.
+// crbug.com/1067269
+ASH_PUBLIC_EXPORT extern const AcceleratorData
+    kEnableWithNewMappingAcceleratorData[];
+ASH_PUBLIC_EXPORT extern const size_t
+    kEnableWithNewMappingAcceleratorDataLength;
+ASH_PUBLIC_EXPORT extern const AcceleratorData
+    kDisableWithNewMappingAcceleratorData[];
+ASH_PUBLIC_EXPORT extern const size_t
+    kDisableWithNewMappingAcceleratorDataLength;
 
 // The public-facing interface for accelerator handling, which is Ash's duty to
 // implement.

chrome/browser/flag-metadata.json

@@ -3098,7 +3098,7 @@
   {
     "name": "new-shortcut-mapping",
     "owners": [ "oshima", "afakhry" ],
-    "expiry_milestone": 85
+    "expiry_milestone": 86
   },
   {
     "name": "new-tabstrip-animation",

chrome/browser/ui/ash/keyboard_shortcut_viewer_metadata_unittest.cc

@@ -232,6 +232,9 @@ TEST_F(KeyboardShortcutViewerMetadataTest,
   std::vector<AcceleratorMapping> chrome_accelerators;
   for (size_t i = 0; i < ash::kAcceleratorDataLength; ++i)
     ash_accelerators.emplace_back(ash::kAcceleratorData[i]);
+  for (size_t i = 0; i < ash::kDisableWithNewMappingAcceleratorDataLength; ++i)
+    ash_accelerators.emplace_back(
+        ash::kDisableWithNewMappingAcceleratorData[i]);
   for (const auto& accel_mapping : GetAcceleratorList())
     chrome_accelerators.emplace_back(accel_mapping);