Build key map DomCodeToKey() for Windows

ui/events/keycodes/key_map_win.cc

+// Copyright (c) 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 "ui/events/keycodes/key_map_win.h"
+
+#include <utility>
+
+#include "base/logging.h"
+#include "base/macros.h"
+
+#include "ui/events/event_constants.h"
+#include "ui/events/keycodes/dom/dom_code.h"
+
+namespace ui {
+
+namespace {
+
+struct DomCodeEntry {
+  DomCode dom_code;
+  int scan_code;
+};
+#define USB_KEYMAP_DECLARATION const DomCodeEntry supported_dom_code_list[] =
+#define USB_KEYMAP(usb, evdev, xkb, win, mac, code, id) {DomCode::id, win}
+#include "ui/events/keycodes/dom/keycode_converter_data.inc"
+#undef USB_KEYMAP
+#undef USB_KEYMAP_DECLARATION
+
+// List of modifiers mentioned in https://w3c.github.io/uievents/#keys-modifiers
+// Some modifiers are commented out because they usually don't change keys.
+const EventFlags flags_index[] = {

[Wez, 2016/02/09 00:18:57]

nit: This is the list of interesting modifier flags, so call it
|modifier_flags|? or just |modifiers|?  |flags_index| doesn't help the reader
understand what it's for.

+    EF_SHIFT_DOWN,
+    EF_CONTROL_DOWN,
+    EF_ALT_DOWN,
+    // EF_COMMAND_DOWN,
+    EF_ALTGR_DOWN,
+    // EF_NUM_LOCK_ON,
+    EF_CAPS_LOCK_ON,
+    // EF_SCROLL_LOCK_ON
+};
+const int kMaxEventFlagsIndex = (1 << arraysize(flags_index)) - 1;

[Wez, 2016/02/09 00:18:57]

This still isn't the maximum index of the flags_index ;)  It's the total number
of possible flag combinations, so perhaps call it kModifierFlagsCombinations?

+
+int GetEventFlagsIndex(int flags) {

[Wez, 2016/02/09 00:18:57]

I don't see this used anywhere?

+  int index = 0;
+  for (int i = 0; i < arraysize(flags_index); ++i) {
+    if (flags & flags_index[i])
+      index |= 1 << i;
+  }
+  return index;
+}
+
+int EventFlagsFromIndex(int index) {

[Wez, 2016/02/09 00:18:57]

nit: Suggest GetModifierFlags(int combination) if you follow the "combination"
and "modifiers" naming above.

+  int flags = EF_NONE;
+  for (int i = 0; i < arraysize(flags_index); ++i) {
+    if (index & (1 << i))
+      flags |= flags_index[i];
+  }
+  return flags;
+}
+
+void SetModifierState(BYTE* keyboard_state, int flags) {
+  if (flags & EF_SHIFT_DOWN)
+    keyboard_state[VK_SHIFT] |= 0x80;
+
+  if (flags & EF_CONTROL_DOWN)
+    keyboard_state[VK_CONTROL] |= 0x80;
+
+  if (flags & EF_ALT_DOWN)
+    keyboard_state[VK_MENU] |= 0x80;
+
+  if (flags & EF_ALTGR_DOWN) {
+    // AltGr should be RightAlt+LeftControl within Windows, but actually only
+    // the non-located keys will work here.
+    keyboard_state[VK_MENU] |= 0x80;

[chongz, 2016/02/04 21:10:25]

I'm not sure why but only the non-located Alt+Control will produce the right
key.

[Wez, 2016/02/09 00:18:57]

OK, sorry for the misleading comment earlier! Not sure the exact mechanics of
that hack, unfortunately.

+    keyboard_state[VK_CONTROL] |= 0x80;
+  }
+
+  if (flags & EF_COMMAND_DOWN)
+    keyboard_state[VK_LWIN] |= 0x80;
+
+  if (flags & EF_NUM_LOCK_ON)
+    keyboard_state[VK_NUMLOCK] |= 0x01;
+
+  if (flags & EF_CAPS_LOCK_ON)
+    keyboard_state[VK_CAPITAL] |= 0x01;
+
+  if (flags & EF_SCROLL_LOCK_ON)
+    keyboard_state[VK_SCROLL] |= 0x01;
+}
+
+}  // anonymous namespace
+
+WindowsKeyMap::WindowsKeyMap() : keyboard_layout_(0) {}

[Wez, 2016/02/09 00:18:57]

nit: You can set this via inline initializer in the header.

+
+WindowsKeyMap::WindowsKeyMap(HKL layout) : WindowsKeyMap() {

[Wez, 2016/02/09 00:18:57]

See above re inline initializer for |keyboard_layout_|

+  LoadLayout(layout);
+}
+
+DomKey WindowsKeyMap::DomCodeToKey(DomCode code, int flags) const {
+  const int flags_to_try[] = {
+      // Trying to match Firefox's behavior and UIEvents DomKey guidelines.
+      // If the combination doesn't produce a printable character, the key value
+      // should be the key with no modifiers except for Shift and AltGr.
+      // See https://w3c.github.io/uievents/#keys-guidelines
+      flags,
+      flags & (EF_SHIFT_DOWN | EF_ALTGR_DOWN | EF_CAPS_LOCK_ON),
+      flags & (EF_SHIFT_DOWN | EF_CAPS_LOCK_ON),
+      EF_NONE,
+  };
+
+  DomKey key = DomKey::NONE;
+  for (auto try_flags : flags_to_try) {
+    const auto& it = key_map_.find(std::make_pair(code, try_flags));
+    if (it != key_map_.end())
+      key = it->second.key;
+    if (key != DomKey::NONE)

[Wez, 2016/02/09 00:18:57]

nit: You're doing this if() even if you didn't change |key| - make the if (it !=
....) a block if() and put this inside it.

+      break;
+  }
+  return key;
+}
+
+KeyboardCode WindowsKeyMap::DomCodeToKeyboardCode(DomCode code) const {
+  const auto& it = key_map_.find(std::make_pair(code, EF_NONE));
+  if (it != key_map_.end())
+    return it->second.legacy_key_code;
+  return static_cast<KeyboardCode>(0);
+}
+
+DomCode WindowsKeyMap::KeyboardCodeToDomCode(KeyboardCode key_code) const {

[dtapuska, 2016/02/04 20:31:45]

Is this really required? I'm not sure if we need a keyboardCode->DomCode map

If we don't need this map then we can reduce the size of the struct stored no?
because we wouldn't need to store legacy_key_code

[chongz, 2016/02/04 21:10:25]

I guess Wez wants it in case user only gives us KeyboardCode? I'm not sure if I
should do the implementation now or only when it's actually used.

[Wez, 2016/02/09 00:18:57]

Let's add it if/when it's actually used!

I was thinking just of synthetic events from JS, but I'm not sure we will even
hit the WindowsKeyMap in that case.

+  // TODO(chongz): Could create another map to improve efficiency if necessary.
+  for (const auto& dom_code_entry : supported_dom_code_list) {
+    if (DomCodeToKeyboardCode(dom_code_entry.dom_code) == key_code)
+      return dom_code_entry.dom_code;
+  }
+  return DomCode::NONE;
+}
+
+DomKey WindowsKeyMap::DomCodeToSystemLayoutKey(DomCode code, int flags) {

[Wez, 2016/02/09 00:18:57]

static methods should be prefixed with a "// static" comment line.

+  static base::LazyInstance<WindowsKeyMap>::Leaky s_instance =
+      LAZY_INSTANCE_INITIALIZER;

[Wez, 2016/02/09 00:18:57]

Note that the LazyInstance header says it "really only makes sense as a global
variable".

+  HKL current_layout = ::GetKeyboardLayout(0);

[chongz, 2016/02/04 21:10:25]

Still, should I add a lock or only allow it to be used by a certain thread?

[Wez, 2016/02/09 00:18:57]

GetKeyboardLayout(0) gets the current layout for the thread on which it is
called - so just adding a lock could mean that calling code with multiple
threads ends up swapping back and forth between layouts, if the layouts differ.
It'd also required holding the lock while you populate the map, which is a
non-trivially slow operation during which you'd be blocking any other threads
that happened to call this API.

Given that layouts are per-thread, could we require that the caller creates a
WindowsKeyMap instance on the stack before starting the MessageLoop, and having
a static CodeAndFlagsToDomKey() method (replacing DomCodeToSystemLayoutKey) that
fetches it from thread-local-storage, refreshes it if necessary, and calls
DomCode[AndFlags]ToKey() on it?

[dtapuska, 2016/02/09 16:46:39]

Likely this code is probably only going to execute on one thread or so inside
each process. We should create the TLS WindowsKeyMap lazily not at stack
startup. Alternatively we can use a global ref counted WindowsKeyMap with an
instance per each locale encountered; and have TLS point at the global map. We'd
need to lock on the global table to lookup; and make WindowsKeyMap immutable.
But then at least we'd share the ref across multiple threads if that does occur.

[Wez, 2016/02/09 22:40:01]

On-demand thread-local instance SGTM - do we have a way of doing that without
them leaking, though?

If scanning the keymap is really slow then maintaining a cache would be useful;
I expect it to be rare for a user to have a huge number of keymaps that they
cycle through, but switching amongst a small number of layouts is common.

+  if (s_instance.Get().keyboard_layout_ != current_layout)
+    s_instance.Get().LoadLayout(current_layout);
+  return s_instance.Get().DomCodeToKey(code, flags);
+}
+
+void WindowsKeyMap::LoadLayout(HKL layout) {
+  keyboard_layout_ = layout;
+  key_map_.clear();
+
+  BYTE keyboard_state_to_restore[256];
+  ::GetKeyboardState(keyboard_state_to_restore);
+
+  for (int eindex = 0; eindex <= kMaxEventFlagsIndex; ++eindex) {
+    BYTE keyboard_state[256];
+    memset(keyboard_state, 0, sizeof(keyboard_state));
+    int flags = EventFlagsFromIndex(eindex);
+    SetModifierState(keyboard_state, flags);
+    for (const auto& dom_code_entry : supported_dom_code_list) {
+      wchar_t translated_chars[5];
+      int key_code = ::MapVirtualKeyEx(dom_code_entry.scan_code,
+                                       MAPVK_VSC_TO_VK, keyboard_layout_);
+      int rv = ::ToUnicodeEx(key_code, 0, keyboard_state, translated_chars,
+                             arraysize(translated_chars), 0, keyboard_layout_);
+
+      if (rv < 0) {
+        // Dead key, injecting VK_SPACE to get character representation.
+        BYTE empty_state[256];
+        memset(empty_state, 0, sizeof(empty_state));
+        rv = ::ToUnicodeEx(VK_SPACE, 0, empty_state, translated_chars,
+                           arraysize(translated_chars), 0, keyboard_layout_);
+        // Must be a repeat of dead key character.
+        DCHECK(rv == 2);
+        AddKeyMapEntry(
+            dom_code_entry.dom_code,
+            DomKey::DeadKeyFromCombiningCharacter(translated_chars[0]),
+            static_cast<KeyboardCode>(key_code), flags);
+      } else if (rv == 1) {
+        if (translated_chars[0] >= 0x20) {
+          AddKeyMapEntry(dom_code_entry.dom_code,
+                         DomKey::FromCharacter(translated_chars[0]),
+                         static_cast<KeyboardCode>(key_code), flags);
+        } else {
+          // Ignores legacy non-printable control characters.
+        }
+      } else {
+        // TODO(chongz): Handle multiple characters case.

[dtapuska, 2016/02/04 20:31:45]

What are we going to do with non-printable characters?

Should the table support DomCode->DomKey for the non-printable range as well?

[chongz, 2016/02/04 21:10:25]

I will add (copy?) the US non-printable table here later, currently it's relying
on the default fall-back since ToUnicodeEx only works for printable characters.

+      }
+    }
+  }
+  ::SetKeyboardState(keyboard_state_to_restore);
+}
+
+void WindowsKeyMap::AddKeyMapEntry(DomCode code,
+                                   DomKey key,
+                                   KeyboardCode legacy_key_code,
+                                   int flags) {
+  key_map_[std::make_pair(code, flags)] =
+      KeyMapEntry{code, key, legacy_key_code, flags};
+}
+
+}  // namespace ui