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

ui/events/keycodes/platform_key_map_win_unittest.cc

Index: ui/events/keycodes/platform_key_map_win_unittest.cc
diff --git a/ui/events/keycodes/platform_key_map_win_unittest.cc b/ui/events/keycodes/platform_key_map_win_unittest.cc
index 934ab00a8bbbbbf1722ff1b355d47808297b0bfd..4a1c4888d3ab8ba99ecf919e6469dcf716de824e 100644
--- a/ui/events/keycodes/platform_key_map_win_unittest.cc
+++ b/ui/events/keycodes/platform_key_map_win_unittest.cc
@@ -31,44 +31,67 @@ struct TestKey {
   const char* altgr_capslock;
 };
 
-void CheckDomCodeToKeyString(const char* label,
-                             const PlatformKeyMap& keymap,
-                             const TestKey& t,
-                             HKL layout) {
-  int scan_code = ::MapVirtualKeyEx(t.vk, MAPVK_VK_TO_VSC, layout);
-  DomCode dom_code = KeycodeConverter::NativeKeycodeToDomCode(scan_code);
-  EXPECT_STREQ(t.normal, KeycodeConverter::DomKeyToKeyString(
-    keymap.DomCodeAndFlagsToDomKey(dom_code, EF_NONE)).c_str()
-  ) << label;
-  EXPECT_STREQ(t.shift, KeycodeConverter::DomKeyToKeyString(
-    keymap.DomCodeAndFlagsToDomKey(dom_code, EF_SHIFT_DOWN)).c_str()
-  ) << label;
-  EXPECT_STREQ(t.capslock, KeycodeConverter::DomKeyToKeyString(
-    keymap.DomCodeAndFlagsToDomKey(dom_code, EF_CAPS_LOCK_ON)).c_str()
-  ) << label;
-  EXPECT_STREQ(t.altgr, KeycodeConverter::DomKeyToKeyString(
-    keymap.DomCodeAndFlagsToDomKey(dom_code, EF_ALTGR_DOWN)).c_str()
-  ) << label;
-  EXPECT_STREQ(t.shift_capslock, KeycodeConverter::DomKeyToKeyString(
-    keymap.DomCodeAndFlagsToDomKey(dom_code,
-                                EF_SHIFT_DOWN | EF_CAPS_LOCK_ON)).c_str()
-  ) << label;
-  EXPECT_STREQ(t.shift_altgr, KeycodeConverter::DomKeyToKeyString(
-    keymap.DomCodeAndFlagsToDomKey(dom_code,
-                                EF_SHIFT_DOWN | EF_ALTGR_DOWN)).c_str()
-  ) << label;
-  EXPECT_STREQ(t.altgr_capslock, KeycodeConverter::DomKeyToKeyString(
-    keymap.DomCodeAndFlagsToDomKey(dom_code,
-                                EF_ALTGR_DOWN | EF_CAPS_LOCK_ON)).c_str()
-  ) << label;
-}
-
 }  // anonymous namespace
 
 class PlatformKeyMapTest : public testing::Test {
  public:
   PlatformKeyMapTest() {}
   ~PlatformKeyMapTest() override {}
+
+  void CheckDomCodeToKeyString(const char* label,
+                               const PlatformKeyMap& keymap,
+                               const TestKey& t,
+                               HKL layout) {

[Wez, 2016/03/18 22:50:39]

nit: Why do this and DomCodeAndFlagsToDomKey need to be members of the test
base, rather than in an anonymous namespace in this test .cc?

nit: Please choose more descriptive names than 't' for parameters, variables,
etc!

[chongz, 2016/03/21 20:07:29]

Because only |PlatformKeyMapTest| is a friend class of |PlatformKeyMap| and
anonymous namespace cannot access |keymap.DomKeyFromNativeImpl|...

+    KeyboardCode vk = t.vk;

[Wez, 2016/03/18 22:50:39]

nit: vk->key_code

[chongz, 2016/03/21 20:07:29]

Done.

+    int scan_code = ::MapVirtualKeyEx(vk, MAPVK_VK_TO_VSC, layout);
+    DomCode dom_code = KeycodeConverter::NativeKeycodeToDomCode(scan_code);
+    EXPECT_STREQ(t.normal,
+                 KeycodeConverter::DomKeyToKeyString(
+                     keymap.DomCodeAndFlagsToDomKey(dom_code, vk, EF_NONE))
+                     .c_str())
+        << label;
+    EXPECT_STREQ(t.shift, KeycodeConverter::DomKeyToKeyString(
+                              keymap.DomCodeAndFlagsToDomKey(dom_code, vk,
+                                                             EF_SHIFT_DOWN))
+                              .c_str())
+        << label;
+    EXPECT_STREQ(t.capslock, KeycodeConverter::DomKeyToKeyString(
+                                 keymap.DomCodeAndFlagsToDomKey(
+                                     dom_code, vk, EF_CAPS_LOCK_ON))
+                                 .c_str())
+        << label;
+    EXPECT_STREQ(t.altgr, KeycodeConverter::DomKeyToKeyString(
+                              keymap.DomCodeAndFlagsToDomKey(dom_code, vk,
+                                                             EF_ALTGR_DOWN))
+                              .c_str())
+        << label;
+    EXPECT_STREQ(t.shift_capslock,
+                 KeycodeConverter::DomKeyToKeyString(
+                     keymap.DomCodeAndFlagsToDomKey(
+                         dom_code, vk, EF_SHIFT_DOWN | EF_CAPS_LOCK_ON))
+                     .c_str())
+        << label;
+    EXPECT_STREQ(t.shift_altgr,
+                 KeycodeConverter::DomKeyToKeyString(
+                     keymap.DomCodeAndFlagsToDomKey(
+                         dom_code, vk, EF_SHIFT_DOWN | EF_ALTGR_DOWN))
+                     .c_str())
+        << label;
+    EXPECT_STREQ(t.altgr_capslock,
+                 KeycodeConverter::DomKeyToKeyString(
+                     keymap.DomCodeAndFlagsToDomKey(
+                         dom_code, vk, EF_ALTGR_DOWN | EF_CAPS_LOCK_ON))
+                     .c_str())
+        << label;
+  }
+
+  DomKey DomCodeAndFlagsToDomKey(const PlatformKeyMap& keymap,
+                                 DomCode dom_code,
+                                 KeyboardCode vk,
+                                 int flags) {
+    return keymap.DomCodeAndFlagsToDomKey(dom_code, vk, flags);
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(PlatformKeyMapTest);
 };
@@ -171,4 +194,54 @@ TEST_F(PlatformKeyMapTest, FRLayout) {
   }
 }
 
+TEST_F(PlatformKeyMapTest, NumPad) {
+  HKL layout = ::LoadKeyboardLayout(LAYOUT_US, 0);
+  PlatformKeyMap keymap(layout);
+
+  const struct TestCase {
+    KeyboardCode vk;

[Wez, 2016/03/18 22:50:39]

nit: key_code

[chongz, 2016/03/21 20:07:29]

Done.

+    DomKey key;
+  } NumPadKeys[] = {

[Wez, 2016/03/18 22:50:39]

Suggest kNumPadTestCases

(The other const test-case data in these tests should also use kFoo constant
naming style)

[chongz, 2016/03/21 20:07:29]

Done.

+      {VKEY_NUMPAD0, DomKey::FromCharacter('0')},
+      {VKEY_NUMPAD1, DomKey::FromCharacter('1')},
+      {VKEY_NUMPAD2, DomKey::FromCharacter('2')},
+      {VKEY_NUMPAD3, DomKey::FromCharacter('3')},
+      {VKEY_NUMPAD4, DomKey::FromCharacter('4')},
+      {VKEY_NUMPAD5, DomKey::FromCharacter('5')},
+      {VKEY_NUMPAD6, DomKey::FromCharacter('6')},
+      {VKEY_NUMPAD7, DomKey::FromCharacter('7')},
+      {VKEY_NUMPAD8, DomKey::FromCharacter('8')},
+      {VKEY_NUMPAD9, DomKey::FromCharacter('9')},
+      {VKEY_CLEAR, DomKey::CLEAR},
+      {VKEY_PRIOR, DomKey::PAGE_UP},
+      {VKEY_NEXT, DomKey::PAGE_DOWN},
+      {VKEY_END, DomKey::END},
+      {VKEY_HOME, DomKey::HOME},
+      {VKEY_LEFT, DomKey::ARROW_LEFT},
+      {VKEY_UP, DomKey::ARROW_UP},
+      {VKEY_RIGHT, DomKey::ARROW_RIGHT},
+      {VKEY_DOWN, DomKey::ARROW_DOWN},
+      {VKEY_INSERT, DomKey::INSERT},
+      {VKEY_DELETE, DomKey::DEL},
+  };
+
+  for (const auto& k : NumPadKeys) {

[Wez, 2016/03/18 22:50:39]

nit: suggest k->test_case

[chongz, 2016/03/21 20:07:29]

Done.

+    KeyboardCode vk = k.vk;
+    int scan_code = ::MapVirtualKeyEx(vk, MAPVK_VK_TO_VSC, layout);
+    DomCode dom_code = KeycodeConverter::NativeKeycodeToDomCode(scan_code);
+
+    EXPECT_EQ(k.key, DomCodeAndFlagsToDomKey(keymap, dom_code, vk, EF_NONE))
+        << vk;
+    EXPECT_EQ(k.key,
+              DomCodeAndFlagsToDomKey(keymap, dom_code, vk, EF_ALTGR_DOWN))
+        << vk;
+    EXPECT_EQ(k.key,
+              DomCodeAndFlagsToDomKey(keymap, dom_code, vk, EF_CONTROL_DOWN))
+        << vk;
+    EXPECT_EQ(k.key, DomCodeAndFlagsToDomKey(keymap, dom_code, vk,
+                                             EF_ALTGR_DOWN | EF_CONTROL_DOWN))
+        << vk;
+  }
+}
+
 }  // namespace ui