cros: Fix Shortcut Viewer labels for non-Qwerty layout

Currently for non-Qwerty keyboard layout, Shortcut Viewer returns
incorrect keys. This cl fixes the bug by creating a reverse mapping from
dom_code to {dom_key, key_code}.

Bug: 841670
Test: manually.
Change-Id: Ib75bf84b34c0b03bf5501fab0586db8e750478bc
Reviewed-on: https://chromium-review.googlesource.com/1058080
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Kevin Schoedel <kpschoedel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559200}

ash/components/shortcut_viewer/keyboard_shortcut_viewer_metadata.cc

@@ -26,6 +26,20 @@ namespace keyboard_shortcut_viewer {
 
 namespace {
 
+// Get all DomCodes to construct the reverse mapping from DomCode to VKEY
+// and DomKey. We want to map a VKEY value to a text label, i.e. VKEY -> DomKey.
+// But VKEY and DomKey are both outputs of layout, so we only have
+// DomCode -> (VKEY, DomKey) by KeyboardLayoutEngine::Lookup(). We need to
+// iterate over the full list of DomCodes in order to look up a corresponding
+// DomKey for a KeyboardCode we want to get the string representation for.
+// keycode_converter.cc does not expose this list, so we need to generate it
+// here.
+#define USB_KEYMAP(usb, evdev, xkb, win, mac, code, id) usb
+#define USB_KEYMAP_DECLARATION constexpr uint32_t kDomCodes[] =
+#include "ui/events/keycodes/dom/keycode_converter_data.inc"
+#undef USB_KEYMAP
+#undef USB_KEYMAP_DECLARATION
+
 // Gets the keyboard codes for modifiers.
 ui::KeyboardCode GetKeyCodeForModifier(ui::EventFlags modifier) {
   switch (modifier) {
@@ -125,23 +139,21 @@ base::string16 GetStringForKeyboardCode(ui::KeyboardCode key_code) {
   if (key_label)
     return key_label.value();
 
-  ui::DomCode dom_code = ui::UsLayoutKeyboardCodeToDomCode(key_code);
   ui::DomKey dom_key;
-  ui::KeyboardCode keycode_ignored;
-  if (ui::KeyboardLayoutEngineManager::GetKeyboardLayoutEngine()->Lookup(
-          dom_code, 0 /* flags */, &dom_key, &keycode_ignored)) {
-    if (dom_key.IsValid() && dom_key.IsDeadKey())
-      return base::string16();
+  ui::KeyboardCode key_code_to_compare = ui::VKEY_UNKNOWN;
+  for (const auto& code : kDomCodes) {
+    const ui::DomCode dom_code = static_cast<ui::DomCode>(code);
+    if (!ui::KeyboardLayoutEngineManager::GetKeyboardLayoutEngine()->Lookup(
+            dom_code, /*flags=*/ui::EF_NONE, &dom_key, &key_code_to_compare)) {
+      continue;
+    }
+    if (key_code_to_compare != key_code || !dom_key.IsValid() ||
+        dom_key.IsDeadKey()) {
+      continue;
+    }
     return base::UTF8ToUTF16(ui::KeycodeConverter::DomKeyToKeyString(dom_key));
   }
-
-  // Fall back to US keyboard layout.
-  const bool has_mapping = ui::DomCodeToUsLayoutDomKey(
-      dom_code, 0 /* flags */, &dom_key, &keycode_ignored);
-  DCHECK(has_mapping);
-  if (dom_key.IsValid() && dom_key.IsDeadKey())
-    return base::string16();
-  return base::UTF8ToUTF16(ui::KeycodeConverter::DomKeyToKeyString(dom_key));
+  return base::string16();
 }
 
 const gfx::VectorIcon* GetVectorIconForKeyboardCode(ui::KeyboardCode key_code) {

ash/components/shortcut_viewer/views/keyboard_shortcut_item_view.cc

@@ -13,6 +13,7 @@
 #include "ash/components/shortcut_viewer/views/bubble_view.h"
 #include "ash/components/strings/grit/ash_components_strings.h"
 #include "base/i18n/rtl.h"
+#include "base/no_destructor.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -21,6 +22,7 @@
 #include "ui/views/border.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/styled_label.h"
+
 namespace keyboard_shortcut_viewer {
 
 namespace {
@@ -74,7 +76,13 @@ KeyboardShortcutItemView::KeyboardShortcutItemView(
   replacement_strings.reserve(shortcut_key_codes_size);
   bool has_invalid_dom_key = false;
   for (ui::KeyboardCode key_code : item.shortcut_key_codes) {
-    const base::string16& dom_key_string = GetStringForKeyboardCode(key_code);
+    auto iter = GetKeycodeToString16Cache()->find(key_code);
+    if (iter == GetKeycodeToString16Cache()->end()) {
+      iter = GetKeycodeToString16Cache()
+                 ->emplace(key_code, GetStringForKeyboardCode(key_code))
+                 .first;
+    }
+    const base::string16& dom_key_string = iter->second;
     // If the |key_code| has no mapped |dom_key_string|, we use alternative
     // string to indicate that the shortcut is not supported by current keyboard
     // layout.
@@ -148,6 +156,19 @@ void KeyboardShortcutItemView::Layout() {
   MaybeCalculateAndDoLayout(GetLocalBounds().width());
 }
 
+// static
+void KeyboardShortcutItemView::ClearKeycodeToString16Cache() {
+  GetKeycodeToString16Cache()->clear();
+}
+
+// static
+std::map<ui::KeyboardCode, base::string16>*
+KeyboardShortcutItemView::GetKeycodeToString16Cache() {
+  static base::NoDestructor<std::map<ui::KeyboardCode, base::string16>>
+      key_code_to_string16_cache;
+  return key_code_to_string16_cache.get();
+}
+
 void KeyboardShortcutItemView::MaybeCalculateAndDoLayout(int width) const {
   if (width == calculated_size_.width())
     return;

ash/components/shortcut_viewer/views/keyboard_shortcut_item_view.h

@@ -39,7 +39,19 @@ class KeyboardShortcutItemView : public views::View {
 
   const KeyboardShortcutItem* shortcut_item() const { return shortcut_item_; }
 
+  // Clear the cache.
+  static void ClearKeycodeToString16Cache();
+
  private:
+  // A cache to avoid repeatly looking up base::string16 from ui::KeyboardCode.
+  // Currently the Keyboard Shortcut Viewer (KSV) will not refresh its contents
+  // when keyboard layout changes. The users must restart KSV again to get new
+  // keys for the new layout. Also since GetStringForKeyboardCode is only called
+  // for KSV to create the strings in the initialization process, clearing the
+  // cache is not necessary when keyboard layout changes.
+  static std::map<ui::KeyboardCode, base::string16>*
+  GetKeycodeToString16Cache();
+
   // Calculates how to layout child views, sets their size and position. |width|
   // is the horizontal space, in pixels, that the view has to work with.
   // Returns the needed size and caches the result in |calculated_size_|.

ash/components/shortcut_viewer/views/keyboard_shortcut_view.cc

@@ -255,6 +255,9 @@ void KeyboardShortcutView::InitViews() {
   AddChildView(search_results_container_);
 
   // Init views of KeyboardShortcutItemView.
+  // TODO(https://crbug.com/843394): Observe changes in keyboard layout and
+  // clear the cache.
+  KeyboardShortcutItemView::ClearKeycodeToString16Cache();
   for (const auto& item : GetKeyboardShortcutItemList()) {
     for (auto category : item.categories) {
       shortcut_views_.emplace_back(