Build key map DomCodeToKey() for Windows

ui/events/keycodes/keyboard_lookup_win.h

Index: ui/events/keycodes/keyboard_lookup_win.h
diff --git a/ui/events/keycodes/keyboard_lookup_win.h b/ui/events/keycodes/keyboard_lookup_win.h
new file mode 100644
index 0000000000000000000000000000000000000000..7e689fe809995c5883ca840f9c0b033f6f592e22
--- /dev/null
+++ b/ui/events/keycodes/keyboard_lookup_win.h
@@ -0,0 +1,41 @@
+// 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.
+
+#ifndef UI_EVENTS_KEYCODES_KEYBOARD_LOOKUP_WIN_H_
+#define UI_EVENTS_KEYCODES_KEYBOARD_LOOKUP_WIN_H_
+
+#include <windows.h>
+
+#include "ui/events/event_constants.h"
+#include "ui/events/events_base_export.h"
+#include "ui/events/keycodes/dom/dom_key.h"
+#include "ui/events/keycodes/keyboard_codes_win.h"
+
+namespace ui {
+
+class EVENTS_BASE_EXPORT WindowsKeyboardLookup {

[Wez, 2016/01/29 00:22:24]

This class encapsulates the key-map, in effect - could it just be WindowsKeymap?

[chongz, 2016/01/29 20:25:49]

Acknowledged.

+ public:
+  WindowsKeyboardLookup(HKL alayout);

[Wez, 2016/01/29 00:22:24]

explicit

[chongz, 2016/01/29 20:25:49]

Acknowledged.

+
+  static DomKey VirtualKeyToCurrentLayoutDomKey(KeyboardCode vkcode,
+                                                int event_flags);
+  DomKey VirtualKeyToDomKey(KeyboardCode vkcode, int event_flags) const;

[Wez, 2016/01/29 00:22:24]

nit: Please add brief comments to explain why we have both of these.

Within the src/ui/ code we refer to the virtual key codes as KeyboardCode (as
opposed to DomKey or DomCode) so perhaps this should be KeyboardCodeToDomKey()?

It's the case that there is not a 1:1 mapping from virtual key codes to DOM key
values, so I'd recommend changing this to provide DomCodeToKey() and
DomCodeToKeyboardCode() instead, and then have a separate OemScancodeToDomCode()
to fetch the DOM |code| value based on the actual key scancode, as well as a
"legacy fallback" KeyboardCodeToDomCode() helper akin to what you have here.

That said, perhaps Dave has an alternative layout in mind?

Also note that in general it's not possible to map back and forth between
KeyboardCode, DomCode, DomKey and OEM scancode without being stateful.

[dtapuska, 2016/01/29 17:05:48]

Ok I do see how we can make this as an API that maps DomCodeToDomKey that would
be similar to the DomCodeToUsLayoutDomKey API.

To do this we could enumerate all possible DOMCodes we support in:
 
https://code.google.com/p/chromium/codesearch#chromium/src/ui/events/keycodes...

Grab the scan code and call ToUnicode on that.

We'd then have some sort of map of [DOMCode + modifiers] -> DOMKey

Can you explain the 1:1 mapping? I know it is true that multiple VKEYs will map
to one DomKey; for example Numpad4 and 4 will both produce DomKey4. But I do
think that one VKEY won't ever map to multiple DomKeys is always true.

It is true that multiple scan codes do map to the same DOMCode.

If we base this on the DomCode then we can handle non-printable characters as
well; for example calling in to handle the non-printable map here:
https://code.google.com/p/chromium/codesearch#chromium/src/ui/events/keycodes...

Chong can you investigate if ToUnicode works well when we enumerate the
scancodes? Or do we have to map the scancodes to vkeys (via MapVirtualKey) and
then call ToUnicode on that result.

[chongz, 2016/01/29 20:25:50]

The static function VirtualKeyToCurrentLayoutDomKey() was expected to be used in
Chrome since we always use current layout, and the other one
VirtualKeyToDomKey() was to provide a way for testing different keyboard layout
without changing system layout.

Sure I will try changing the API and check if ToUnicode (and MapVirtualKey)
works well, especially for custom key remapping. 

[chongz, 2016/02/02 21:14:04]

Changed it to DomCodeToKey(), also added DomCodeToKeyboardCode() and
KeyboardCodeToDomCode(), not sure if we still need OemScancodeToDomCode()?

+
+  static WindowsKeyboardLookup s_instance;

[dtapuska, 2016/01/28 15:27:55]

I'd define this static inside the VirtualKeyToCurrentLayoutDomKey itself.

[chongz, 2016/01/29 20:25:49]

Acknowledged.

+
+ private:
+  WindowsKeyboardLookup();
+
+  void CheckOrLoadLayout(HKL alayout);

[Wez, 2016/01/29 00:22:24]

nit: Why |alayout| rather than |layout|?

Why is this "CheckOrLoadLayout"? What does it mean to "check" the layout?

[chongz, 2016/01/29 20:25:50]

It will load layout only when it's different from the current one... I will move
the check out and change it to LoadLayout()

+
+  static const size_t kMaxVirtualKeyCode = 256;

[Wez, 2016/01/29 00:22:24]

nit: This isn't the maximum virtual key code value, but the _count_ of possible
virtual key code values - the _maximum_ key code value is 255.

Can you get away with using int rather than size_t here, since these aren't
sizes of in-memory structures? We general prefer to use signed types unless we
really have to use unsigned for some specific reason, to reduce scope for issues
arising from signed/unsigned mismatches.

[chongz, 2016/01/29 20:25:50]

Acknowledged.

+  static const size_t kMaxKeyboardModifier = 1 << 8;

[Wez, 2016/01/29 00:22:24]

nit: This isn't the maximum modifier, it's the number of different modifier
combinations, given eight discrete modifier flags that we take account of.

[chongz, 2016/01/29 20:25:50]

Acknowledged.

+
+  HKL keyboard_layout_;
+  DomKey lookup_table_[kMaxVirtualKeyCode][kMaxKeyboardModifier];

[Wez, 2016/01/29 00:22:24]

This means we're creating a table with 256*256 entries, each of which is 4 bytes
in size, i.e. a 256KB table, which seems excessive - are all of the modifiers
you're considering necessary to take into account?

[chongz, 2016/01/29 20:25:50]

Yes it's too large. I guess for each DomCode I can only store valid modifiers
entries in a dynamic allocated array.

[chongz, 2016/02/02 21:14:04]

Switched to the sparse map and ignored EF_COMMAND_DOWN, EF_NUM_LOCK_ON and
EF_NUM_LOCK_ON to save memory.

Now the map has 856 entries for English layout and 12 byte per entry, so it will
be around 13KB plus std::map overhead.

Next step is to handle EF_NUM_LOCK_ON and maybe add more fall-back logic to save
more memory if necessary.

(FYI 900 entries for French and 868 for Persian, LoadLayout() takes 20ms on my
Win8 VM)

+};

[Wez, 2016/01/29 00:22:24]

DISALLOW_COPY_AND_ASSIGN() on this class.

[chongz, 2016/01/29 20:25:50]

Acknowledged.

+
+}  // namespace ui
+
+#endif  // UI_EVENTS_KEYCODES_KEYBOARD_LOOKUP_WIN_H_