Fix caret browsing UI on Chrome OS

This adds the toggle button for caret browsing to
Chrome OS accessibility settings; previously it was
only in browser accessibility settings which isn't
used on Chrome OS.

Also fixes the syntax for the accelerator on Chrome OS.
The actual user-facing shortcut is Ctrl+Search+7, but
the only way to achieve that in accelerator_table.cc is
to use VKEY_F7.

Manually tested on-device and confirmed that the correct
shortcut Ctrl+Search+7 does work and that no permutation
of modifier keys with F7 has any effect.

Bug: 1120826

Change-Id: I89b4948a874f232e2c893f00611a927b3f45e44d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2390876Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Taylor Bergquist <tbergquist@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Auto-Submit: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806365}

chrome/browser/resources/settings/chromeos/os_a11y_page/manage_a11y_page.html

@@ -169,6 +169,13 @@
         label="$i18n{caretHighlightLabel}"
         deep-link-focus-id$="[[Setting.kHighlightTextCaret]]">
     </settings-toggle-button>
+    <settings-toggle-button
+        class="hr"
+        pref="{{prefs.settings.a11y.caretbrowsing.enabled}}"
+        on-change="onA11yCaretBrowsingChange_"
+        label="$i18n{caretBrowsingTitle}"
+        sub-label="$i18n{caretBrowsingSubtitle}">
+    </settings-toggle-button>
     <template is="dom-if" if="[[!isKioskModeActive_]]">
       <settings-toggle-button
           id="enableSwitchAccess"

chrome/browser/resources/settings/chromeos/os_a11y_page/manage_a11y_page.js

@@ -416,6 +416,20 @@ Polymer({
         /* dynamicParams */ null, /* removeSearch */ true);
   },
 
+  /**
+   * @param {!Event} event
+   * @private
+   */
+  onA11yCaretBrowsingChange_(event) {
+    if (event.target.checked) {
+      chrome.metricsPrivate.recordUserAction(
+          'Accessibility.CaretBrowsing.EnableWithSettings');
+    } else {
+      chrome.metricsPrivate.recordUserAction(
+          'Accessibility.CaretBrowsing.DisableWithSettings');
+    }
+  },
+
   /**
    * @return {boolean} Whether shelf navigation buttons should implicitly be
    *     enabled in tablet mode (due to accessibility settings different than

chrome/browser/ui/ash/keyboard_shortcut_viewer_metadata_unittest.cc

@@ -31,12 +31,12 @@ constexpr char kAshAcceleratorsHash[] = "8c10cc51d4e84e4b94310bc91d15a0a8";
 // The total number of Chrome accelerators (available on Chrome OS).
 constexpr int kChromeAcceleratorsTotalNum = 97;
 // The hash of Chrome accelerators (available on Chrome OS).
-constexpr char kChromeAcceleratorsHash[] = "a5d5a245352c94b17618c6d9425a0a9b";
+constexpr char kChromeAcceleratorsHash[] = "efe4c88a2b35238274820158dd46fc0a";
 #else
 // The total number of Chrome accelerators (available on Chrome OS).
 constexpr int kChromeAcceleratorsTotalNum = 96;
 // The hash of Chrome accelerators (available on Chrome OS).
-constexpr char kChromeAcceleratorsHash[] = "1c6a632c57e6e1033e482fc6ffc2184e";
+constexpr char kChromeAcceleratorsHash[] = "9a9ca7beaf63ecdbfdb94aad12ed2c68";
 #endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
 
 const char* BooleanToString(bool value) {
@@ -162,11 +162,13 @@ TEST_F(KeyboardShortcutViewerMetadataTest, CheckAcceleratorIdHasAccelerator) {
     if (shortcut_item.description_message_id ==
             IDS_KSV_DESCRIPTION_DESKS_NEW_DESK ||
         shortcut_item.description_message_id ==
-            IDS_KSV_DESCRIPTION_DESKS_REMOVE_CURRENT_DESK) {
+            IDS_KSV_DESCRIPTION_DESKS_REMOVE_CURRENT_DESK ||
+        shortcut_item.description_message_id ==
+            IDS_KSV_DESCRIPTION_CARET_BROWSING_TOGGLE) {
       // Ignore these for now until https://crbug.com/976487 is fixed.
       // These two accelerators have to be listed differently in the keyboard
       // shortcut viewer than how they are actually defined in the accelerator
-      // table due to the SEARCH + "=" and "-" remapping to F11 and F12
+      // table due to the SEARCH + "1","2"..."0","=","-" remapping to F1...F12
       // respectively.
       continue;
     }

chrome/browser/ui/views/accelerator_table.cc

@@ -14,6 +14,7 @@
 #include "chrome/app/chrome_command_ids.h"
 #include "printing/buildflags/buildflags.h"
 #include "ui/base/accelerators/accelerator.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/events/event_constants.h"
 
 namespace {
@@ -151,8 +152,6 @@ const AcceleratorMapping kAcceleratorMap[] = {
     {ui::VKEY_BROWSER_STOP, ui::EF_NONE, IDC_STOP},
     // On Chrome OS, Search + Esc is used to call out task manager.
     {ui::VKEY_ESCAPE, ui::EF_COMMAND_DOWN, IDC_TASK_MANAGER},
-    {ui::VKEY_7, ui::EF_CONTROL_DOWN | ui::EF_COMMAND_DOWN,
-     IDC_CARET_BROWSING_TOGGLE},
 #else  // !OS_CHROMEOS
     {ui::VKEY_ESCAPE, ui::EF_SHIFT_DOWN, IDC_TASK_MANAGER},
     {ui::VKEY_LMENU, ui::EF_NONE, IDC_FOCUS_MENU_BAR},
@@ -217,6 +216,22 @@ const AcceleratorMapping kAcceleratorMap[] = {
 #endif  // !OS_MAC
 };
 
+#if defined(OS_CHROMEOS)
+// Accelerators to enable if features::IsNewShortcutMappingEnabled is false.
+const AcceleratorMapping kDisableWithNewMappingAcceleratorMap[] = {
+    // On Chrome OS, Control + Search + 7 toggles caret browsing.
+    // Note that VKEY_F7 is not a typo; Search + 7 maps to F7 for accelerators.
+    {ui::VKEY_F7, ui::EF_CONTROL_DOWN, IDC_CARET_BROWSING_TOGGLE},
+};
+
+// Accelerators to enable if features::IsNewShortcutMappingEnabled is true.
+const AcceleratorMapping kEnableWithNewMappingAcceleratorMap[] = {
+    // On Chrome OS, Control + Search + 7 toggles caret browsing.
+    {ui::VKEY_7, ui::EF_CONTROL_DOWN | ui::EF_COMMAND_DOWN,
+     IDC_CARET_BROWSING_TOGGLE},
+};
+#endif
+
 const int kRepeatableCommandIds[] = {
   IDC_FIND_NEXT,
   IDC_FIND_PREVIOUS,
@@ -232,8 +247,25 @@ const size_t kRepeatableCommandIdsLength = base::size(kRepeatableCommandIds);
 } // namespace
 
 std::vector<AcceleratorMapping> GetAcceleratorList() {
-  static base::NoDestructor<std::vector<AcceleratorMapping>> accelerators(
-      std::begin(kAcceleratorMap), std::end(kAcceleratorMap));
+  static base::NoDestructor<std::vector<AcceleratorMapping>> accelerators;
+
+  if (accelerators->empty()) {
+    accelerators->insert(accelerators->begin(), std::begin(kAcceleratorMap),
+                         std::end(kAcceleratorMap));
+
+#if defined(OS_CHROMEOS)
+    if (::features::IsNewShortcutMappingEnabled()) {
+      accelerators->insert(accelerators->begin(),
+                           std::begin(kEnableWithNewMappingAcceleratorMap),
+                           std::end(kEnableWithNewMappingAcceleratorMap));
+    } else {
+      accelerators->insert(accelerators->begin(),
+                           std::begin(kDisableWithNewMappingAcceleratorMap),
+                           std::end(kDisableWithNewMappingAcceleratorMap));
+    }
+#endif
+  }
+
   return *accelerators;
 }
 

chrome/browser/ui/webui/settings/chromeos/accessibility_section.cc

@@ -513,6 +513,8 @@ void AccessibilitySection::AddLoadTimeData(
        IDS_SETTINGS_CAPTIONS_ENABLE_LIVE_CAPTION_TITLE},
       {"captionsEnableLiveCaptionSubtitle",
        IDS_SETTINGS_CAPTIONS_ENABLE_LIVE_CAPTION_SUBTITLE},
+      {"caretBrowsingTitle", IDS_SETTINGS_ENABLE_CARET_BROWSING_TITLE},
+      {"caretBrowsingSubtitle", IDS_SETTINGS_ENABLE_CARET_BROWSING_SUBTITLE},
   };
   AddLocalizedStringsBulk(html_source, kLocalizedStrings);