[Windows] Produce correct DomKey for NumPad when combined with Shift/NumLock

ui/events/keycodes/platform_key_map_win.cc

Index: ui/events/keycodes/platform_key_map_win.cc
diff --git a/ui/events/keycodes/platform_key_map_win.cc b/ui/events/keycodes/platform_key_map_win.cc
index 6a95fd7b2f3cdb6a2f3a89047bec7690ba372ec2..3ba499aa9985384e7fbb38adfd3bee5f6275f81c 100644
--- a/ui/events/keycodes/platform_key_map_win.cc
+++ b/ui/events/keycodes/platform_key_map_win.cc
@@ -13,6 +13,7 @@
 
 #include "ui/events/event_constants.h"
 #include "ui/events/keycodes/dom/dom_code.h"
+#include "ui/events/keycodes/keyboard_code_conversion.h"
 
 namespace ui {
 
@@ -87,6 +88,35 @@ void SetModifierState(BYTE* keyboard_state, int flags) {
     keyboard_state[VK_SCROLL] |= 0x01;
 }
 
+DomKey NonPrintableNumPadKeyCodeToDomKey(KeyboardCode key_code) {
+  switch (key_code) {
+    case VKEY_CLEAR:
+      return DomKey::CLEAR;
+    case VKEY_PRIOR:
+      return DomKey::PAGE_UP;
+    case VKEY_NEXT:
+      return DomKey::PAGE_DOWN;
+    case VKEY_END:
+      return DomKey::END;
+    case VKEY_HOME:
+      return DomKey::HOME;
+    case VKEY_LEFT:
+      return DomKey::ARROW_LEFT;
+    case VKEY_UP:
+      return DomKey::ARROW_UP;
+    case VKEY_RIGHT:
+      return DomKey::ARROW_RIGHT;
+    case VKEY_DOWN:
+      return DomKey::ARROW_DOWN;
+    case VKEY_INSERT:
+      return DomKey::INSERT;
+    case VKEY_DELETE:
+      return DomKey::DEL;
+    default:
+      return DomKey::NONE;
+  }
+}
+
 void CleanupKeyMapTls(void* data) {
   PlatformKeyMap* key_map = reinterpret_cast<PlatformKeyMap*>(data);
   delete key_map;
@@ -115,7 +145,10 @@ PlatformKeyMap::PlatformKeyMap(HKL layout) {
 
 PlatformKeyMap::~PlatformKeyMap() {}
 
-DomKey PlatformKeyMap::DomCodeAndFlagsToDomKey(DomCode code, int flags) const {
+DomKey PlatformKeyMap::DomCodeAndFlagsToDomKey(DomCode code,
+                                               KeyboardCode key_code,
+                                               int flags) const {
+  // Try printable key map first.
   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
@@ -137,11 +170,28 @@ DomKey PlatformKeyMap::DomCodeAndFlagsToDomKey(DomCode code, int flags) const {
         break;
     }
   }
+
+  // Handle non-printable key produced by NumPad.
+  if (key == DomKey::NONE) {

[Wez, 2016/03/16 21:03:06]

IMO it would be cleaner & easier to understand this code if just had a
special-case for Numpad keys (e.g. detected based on the Numpad DomCodes) that
causes them to be mapped based on KeyboardCode instead of DomCode+flags.

+    // Have to use KeyboardCode instead of DomCode because NumLockOn+Shift+4
+    // will generate keydown event with EF_SHIFT_DOWN modifier set to false, so
+    // we have no way to know whether Shift is down.
+    key = NonPrintableNumPadKeyCodeToDomKey(key_code);
+  }
+
+  // Use US layout for other non-printable key.

[dtapuska, 2016/03/10 20:51:44]

This is a bit odd that this is done here; because it is done in ApplyLayout as
well; but from your explanation it sounds like the US code was added to simplify
the tests?

[chongz, 2016/03/10 21:03:32]

Yes it will simplify the tests, otherwise I will need to simulate how
ApplyLayout() works. 

Anyways if it's preferred to rely on ApplyLayout() I'm OK to remove it here.  

[Wez, 2016/03/16 21:03:06]

Sorry, I didn't get the logic here; are you suggesting to fall-back to the
hard-wired US layout mapping in tests?  That surely means we're basically not
testing the PlatformKeyMap impl at all?

Also, note that DomCodeToUsLayoutDomKey doesn't have any special handling of
this Windows-specific behaviour either.

+  if (key == DomKey::NONE) {
+    KeyboardCode dummy_key_code;
+    DomCodeToUsLayoutDomKey(code, flags, &key, &dummy_key_code);
+  }
+
   return key;
 }
 
 // static
-DomKey PlatformKeyMap::DomCodeAndFlagsToDomKeyStatic(DomCode code, int flags) {
+DomKey PlatformKeyMap::DomCodeAndFlagsToDomKeyStatic(DomCode code,
+                                                     KeyboardCode key_code,
+                                                     int flags) {
   // Use TLS because KeyboardLayout is per thread.
   // However currently PlatformKeyMap will only be used by the host application,
   // which is just one process and one thread.
@@ -156,7 +206,7 @@ DomKey PlatformKeyMap::DomCodeAndFlagsToDomKeyStatic(DomCode code, int flags) {
 
   HKL current_layout = ::GetKeyboardLayout(0);
   platform_key_map->UpdateLayout(current_layout);
-  return platform_key_map->DomCodeAndFlagsToDomKey(code, flags);
+  return platform_key_map->DomCodeAndFlagsToDomKey(code, key_code, flags);
 }
 
 void PlatformKeyMap::UpdateLayout(HKL layout) {