[KeyboardEvent] Add embedder APIs to translate between Dom |key| enum and strings

components/test_runner/event_sender.cc

Index: components/test_runner/event_sender.cc
diff --git a/components/test_runner/event_sender.cc b/components/test_runner/event_sender.cc
index c78d03972ca4f494195eeb964a3f7bf3a21008dc..f642034c8b1e3d0f325c28e9e2130f96a718961d 100644
--- a/components/test_runner/event_sender.cc
+++ b/components/test_runner/event_sender.cc
@@ -22,7 +22,10 @@
 #include "third_party/WebKit/public/web/WebKit.h"
 #include "third_party/WebKit/public/web/WebPagePopup.h"
 #include "third_party/WebKit/public/web/WebView.h"
+#include "ui/events/event_constants.h"
+#include "ui/events/keycodes/dom/dom_key.h"
 #include "ui/events/keycodes/dom/keycode_converter.h"
+#include "ui/events/keycodes/keyboard_code_conversion.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "v8/include/v8.h"
 
@@ -1252,78 +1255,55 @@ void EventSender::KeyDown(const std::string& code_str,
 
   // Convert \n -> VK_RETURN. Some layout tests use \n to mean "Enter", when
   // Windows uses \r for "Enter".
-  int code = 0;
+  int keyCode = 0;

[Wez, 2015/06/11 00:09:05]

keyCode -> key_code

Chromium style-guide requires unix_style names for variables, rather than
camelCaseStyle.

[Habib Virji, 2015/06/24 14:32:12]

Done.

   int text = 0;
   bool needs_shift_key_modifier = false;
-  std::string domString;
 
   if ("\n" == code_str) {
     generate_char = true;
-    text = code = ui::VKEY_RETURN;
-    domString.assign("Enter");
+    text = keyCode = ui::VKEY_RETURN;
   } else if ("rightArrow" == code_str) {
-    code = ui::VKEY_RIGHT;
-    domString.assign("ArrowRight");
+    keyCode = ui::VKEY_RIGHT;
   } else if ("downArrow" == code_str) {
-    code = ui::VKEY_DOWN;
-    domString.assign("ArrowDown");
+    keyCode = ui::VKEY_DOWN;
   } else if ("leftArrow" == code_str) {
-    code = ui::VKEY_LEFT;
-    domString.assign("ArrowLeft");
+    keyCode = ui::VKEY_LEFT;
   } else if ("upArrow" == code_str) {
-    code = ui::VKEY_UP;
-    domString.assign("ArrowUp");
+    keyCode = ui::VKEY_UP;
   } else if ("insert" == code_str) {
-    code = ui::VKEY_INSERT;
-    domString.assign("Insert");
+    keyCode = ui::VKEY_INSERT;
   } else if ("delete" == code_str) {
-    code = ui::VKEY_DELETE;
-    domString.assign("Delete");
+    keyCode = ui::VKEY_DELETE;
   } else if ("pageUp" == code_str) {
-    code = ui::VKEY_PRIOR;
-    domString.assign("PageUp");
+    keyCode = ui::VKEY_PRIOR;
   } else if ("pageDown" == code_str) {
-    code = ui::VKEY_NEXT;
-    domString.assign("PageDown");
+    keyCode = ui::VKEY_NEXT;
   } else if ("home" == code_str) {
-    code = ui::VKEY_HOME;
-    domString.assign("Home");
+    keyCode = ui::VKEY_HOME;
   } else if ("end" == code_str) {
-    code = ui::VKEY_END;
-    domString.assign("End");
+    keyCode = ui::VKEY_END;
   } else if ("printScreen" == code_str) {
-    code = ui::VKEY_SNAPSHOT;
-    domString.assign("PrintScreen");
+    keyCode = ui::VKEY_SNAPSHOT;
   } else if ("menu" == code_str) {
-    code = ui::VKEY_APPS;
-    domString.assign("ContextMenu");
+    keyCode = ui::VKEY_APPS;
   } else if ("leftControl" == code_str) {
-    code = ui::VKEY_LCONTROL;
-    domString.assign("ControlLeft");
+    keyCode = ui::VKEY_LCONTROL;
   } else if ("rightControl" == code_str) {
-    code = ui::VKEY_RCONTROL;
-    domString.assign("ControlRight");
+    keyCode = ui::VKEY_RCONTROL;
   } else if ("leftShift" == code_str) {
-    code = ui::VKEY_LSHIFT;
-    domString.assign("ShiftLeft");
+    keyCode = ui::VKEY_LSHIFT;
   } else if ("rightShift" == code_str) {
-    code = ui::VKEY_RSHIFT;
-    domString.assign("ShiftRight");
+    keyCode = ui::VKEY_RSHIFT;
   } else if ("leftAlt" == code_str) {
-    code = ui::VKEY_LMENU;
-    domString.assign("AltLeft");
+    keyCode = ui::VKEY_LMENU;
   } else if ("rightAlt" == code_str) {
-    code = ui::VKEY_RMENU;
-    domString.assign("AltRight");
+    keyCode = ui::VKEY_RMENU;
   } else if ("numLock" == code_str) {
-    code = ui::VKEY_NUMLOCK;
-    domString.assign("NumLock");
+    keyCode = ui::VKEY_NUMLOCK;
   } else if ("backspace" == code_str) {
-    code = ui::VKEY_BACK;
-    domString.assign("Backspace");
+    keyCode = ui::VKEY_BACK;
   } else if ("escape" == code_str) {
-    code = ui::VKEY_ESCAPE;
-    domString.assign("Escape");
+    keyCode = ui::VKEY_ESCAPE;
   } else {
     // Compare the input string with the function-key names defined by the
     // DOM spec (i.e. "F1",...,"F24"). If the input string is a function-key
@@ -1331,12 +1311,11 @@ void EventSender::KeyDown(const std::string& code_str,
     for (int i = 1; i <= 24; ++i) {
       std::string function_key_name = base::StringPrintf("F%d", i);
       if (function_key_name == code_str) {
-        code = ui::VKEY_F1 + (i - 1);
-        domString = function_key_name;
+        keyCode = ui::VKEY_F1 + (i - 1);
         break;
       }
     }
-    if (!code) {
+    if (!keyCode) {
       WebString web_code_str =
           WebString::fromUTF8(code_str.data(), code_str.size());

[Wez, 2015/06/11 00:09:05]

Why do we even convert this to a WebString? We never seem to use it as a
WebString anywhere here?

[Habib Virji, 2015/06/24 14:32:12]

I did try just using code_str, but apparently it is for line below
web_code_str(0 i.e. to get the ascii value for the string. BTW it was not my
addition, but it was added prior. 

[Wez, 2015/06/25 10:40:21]

All that the conversion is actually doing is taking |key_code_str| in UTF-8,
converting it to UTF-16 and looking at the first code-point (i.e. character)
that it contains.  You can do the same thing using the conversion UTF8toUTF16
function from
https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/utf_s...
and then pulling out the first character (if any), without needing to involve
WebString.

[Habib Virji, 2015/06/26 17:35:38]

Thanks this is useful information.

       if (web_code_str.length() != 1u) {
@@ -1345,30 +1324,26 @@ void EventSender::KeyDown(const std::string& code_str,
             gin::StringToV8(isolate, "Invalid web code.")));
         return;
       }
-      text = code = web_code_str.at(0);
-      needs_shift_key_modifier = NeedsShiftModifier(code);
-      if ((code & 0xFF) >= 'a' && (code & 0xFF) <= 'z')
-        code -= 'a' - 'A';
-      if ((code >= 'A' && code <= 'Z') || (code >= 'a' && code <= 'z')) {
-        domString.assign("Key");
-        domString.push_back(base::ToUpperASCII(code));
-      } else if (code >= '0' && code <= '9') {
-        domString.assign("Digit");
-        domString.push_back(code);
-      } else if (code == ' ') {
-        domString.assign("Space");
-      } else if (code == 9) {
-        domString.assign("Tab");
-      }
+      text = keyCode = web_code_str.at(0);

[Wez, 2015/06/11 00:09:05]

DOM |code| values for e.g. keys A-Z are of the form "KeyA", for example, not
"A", so this won't do what you intend - you'll always generate VKEY_K for those
|code| values, surely?

nit: This implementation also seems to be assuming a US English layout - is that
valid for EventSender?

[Habib Virji, 2015/06/24 14:32:12]

Sorry did not followed about the DOM |code| comment. 

Not sure about how test will behave in non-US english layout.

[Wez, 2015/06/25 10:40:21]

Sorry - I mis-read that |web_code_str| contained the DOM |code|, which would
contain e.g. "KeyA", but actually it contains the |key_code_str|, which would
just be 'a' or 'A'.
Essentially this function completely bypasses the layout, injecting events with
specific |keyCode| values - now that we have |key| and |code|, though, the
distinction becomes important.

Basically we should re-implement this function to work out what to generate
based on the DOM |code| of the key, using the US English layout routines since
this is just for unit-tests. We can support the existing interface, based on
|keyCode| by converting the |keyCode| down to DOM |code| at the start of the
method, again using the US English layout routines, i.e:

... KeyDown(key_code_str) {
  if (key_code_str == "rightArrow") {
    event.dom_code = ...
  } else if (...) {
    ...
  } else {
    event.dom_code = UsEnglishKeyboardCodeToDomCode(...)
  }
  event.dom_key = UsEnglishDomCodeToDomKeyAndCharacter(..., dom_key_character)
  if (dom_key == CHARACTER)
    event.text = event.unmodifiedText = dom_key_character
...
}

WDYT?

[Habib Virji, 2015/06/26 17:35:38]

I agree with your suggestion. Since unittest has very small values to check it
is feasible and does not have dead keys as kevin has pointed. I have updated
code based on the above suggestion.

+      needs_shift_key_modifier = NeedsShiftModifier(keyCode);
+      if ((keyCode & 0xFF) >= 'a' && (keyCode & 0xFF) <= 'z')
+        keyCode -= 'a' - 'A';
       generate_char = true;
     }
 
     if ("(" == code_str) {
-      code = '9';
+      keyCode = '9';
       needs_shift_key_modifier = true;
     }
   }
 
+  ui::DomKey domkey = ui::DomKey::NONE;

[Wez, 2015/06/11 00:09:05]

nit: domkey -> dom_key, domcode -> dom_code, domkey_char -> dom_key_char
nit: Declare domKey immediately before domKeyChar, below.

[Habib Virji, 2015/06/24 14:32:12]

Done.

+  ui::DomCode domcode = ui::UsLayoutKeyboardCodeToDomCode(
+      static_cast<ui::KeyboardCode>(keyCode));
+  base::char16 domkey_char;
+  GetMeaningFromKeyCode(static_cast<ui::KeyboardCode>(keyCode),
+                        ui::EF_NONE, &domkey, &domkey_char);

[Wez, 2015/06/11 00:09:05]

We're trying to get rid of GetMeaningFromKeyCode(), since it's a hack.

To migrate EventSender::KeyDown() to the new scheme it should:

1. Process the supplied |code_str| (which should be renamed |key_code_str| to
match |key_code| and |keyCode| naming within this function) to get the VKEY
value.

2. Call UsLayoutKeyboardCodeToDomCode to get the DomCode for the VKEY under a US
English layout.

3. Call DomCodeToUsLayoutMeaning to get the DomKey+character corresponding to
the meaning of that key under a US English layout.

Ideally we'd migrate EventSender to use DOM |code| as the parameter in place of
the old KeyboardCode parameter, at which point we would be able to dispense with
step #1, and instead set the KeyboardCode based on the result of #3.

[Habib Virji, 2015/06/24 14:32:12]

Implemented as suggested. It also include now the way to differentiate DOM |Key|
printable and non-printable characters, since this is the code that's called
when running layout tests.

+
   // For one generated keyboard event, we need to generate a keyDown/keyUp
   // pair;
   // On Windows, we might also need to generate a char event to mimic the
@@ -1377,9 +1352,10 @@ void EventSender::KeyDown(const std::string& code_str,
   WebKeyboardEvent event_down;
   event_down.type = WebInputEvent::RawKeyDown;
   event_down.modifiers = modifiers;
-  event_down.windowsKeyCode = code;
-  event_down.domCode = static_cast<int>(
-      ui::KeycodeConverter::CodeStringToDomCode(domString.c_str()));
+  event_down.windowsKeyCode = keyCode;
+  event_down.domCode = static_cast<int>(domcode);
+  event_down.domKey = static_cast<int>(domkey);
+  event_down.domKeyChar = static_cast<int>(domkey_char);
 
   if (generate_char) {
     event_down.text[0] = text;
@@ -1417,7 +1393,7 @@ void EventSender::KeyDown(const std::string& code_str,
 
   HandleInputEventOnViewOrPopup(event_down);
 
-  if (code == ui::VKEY_ESCAPE && !current_drag_data_.isNull()) {
+  if (keyCode == ui::VKEY_ESCAPE && !current_drag_data_.isNull()) {
     WebMouseEvent event;
     InitMouseEvent(WebInputEvent::MouseDown,
                    pressed_button_,
@@ -1437,7 +1413,7 @@ void EventSender::KeyDown(const std::string& code_str,
     // keyIdentifier is an empty string, unless the Enter key was pressed.
     // This behavior is not standard (keyIdentifier itself is not even a
     // standard any more), but it matches the actual behavior in Blink.
-    if (code != ui::VKEY_RETURN)
+    if (keyCode != ui::VKEY_RETURN)
       event_char.keyIdentifier[0] = '\0';
     HandleInputEventOnViewOrPopup(event_char);
   }