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

content/child/blink_platform_impl.cc

Index: content/child/blink_platform_impl.cc
diff --git a/content/child/blink_platform_impl.cc b/content/child/blink_platform_impl.cc
index 78939bb0167d693444707233ea5ba300d1cc7eb3..9c87777d0758caeca30573143c6c77fc881c2384 100644
--- a/content/child/blink_platform_impl.cc
+++ b/content/child/blink_platform_impl.cc
@@ -71,6 +71,7 @@
 #include "third_party/WebKit/public/web/WebSecurityOrigin.h"
 #include "ui/base/layout.h"
 #include "ui/events/gestures/blink/web_gesture_curve_impl.h"
+#include "ui/events/keycodes/dom/dom_key.h"
 #include "ui/events/keycodes/dom/keycode_converter.h"
 
 using blink::WebData;
@@ -1392,4 +1393,23 @@ int BlinkPlatformImpl::domEnumFromCodeString(const WebString& code) {
       code.utf8().data()));
 }
 
+WebString BlinkPlatformImpl::domKeyStringFromEnum(int dom_key) {
+  if (dom_key < DOM_KEY_PRINT_NON_DIFF) {

[Wez, 2015/07/25 23:12:26]

Discussing this with kpschoedel@, he prefers that the DomKey enum values come
first, so that the "enum" values used here are either equal to a DomKey enum, or
are out of range, in which case they represent a character.

My suggestion would be to:
- Make sure that DomKey::CHARACTER is the last/max DomKey enum value.
<  DomKey::CHARACTER -> DomKey enum
This way DomKey enums can be compared to these Blink DOM key enums directly, and
DomKey::CHARACTER acts as a natural sentinel value.

e.g. to encode a DomKey + character:

int domKeyAndCharToBlinkDomKeyEnum(DomKey dom_key, int code_point) {
  dom_key_int = dom_key;
  if (dom_key == DomKey::CHARACTER)
    dom_key_int += code_point;
  return dom_key_int;
}

e.g. to decode a Blink DOM key to DomKey + character:

DomKey blinkDomKeyEnumToDomKeyAndChar(int dom_key_int, int* code_point) {
  if (dom_key_int >= DomKey::CHARACTER) {
    *code_point = dom_key_int - DomKey::CHARACTER;
    return DomKey::CHARACTER;
  } else {
    return static_cast<int>(dom_key_int);
  }
}

[kpschoedel, 2015/07/29 14:43:14]

Apologies for being away from this for a while.

Given Gary's clarification of the UI Events spec it's probably easier that I had
previously thought to migrate |ui::DomKey| to a combined Unicode/non-printable
code. I'm going to look at that over the next few days and see whether there are
any obstacles elsewhere in Chrome.

[Wez, 2015/07/29 17:59:36]

SGTM - thanks Kevin!

[kpschoedel, 2015/07/29 19:16:11]

I'm now thinking the opposite for a unified ui::DomKey implementation — I'd like
to preserve the existing usage of ui::DomKey as the type and ui::DomKey::NAME as
enumerated value, to match C++ 'enum class' types including ui::DomCode, but to
allow non-enumerated values (i.e. Unicode characters) means giving up the type
safety of 'enum class', and so I think it's safer to have the non-Unicode key
constants outside the Unicode range.

[Wez, 2015/07/29 19:59:47]

I'm not sure that type-safety is really an issue here, since the current DomKey
enum doesn't cover the Unicode code-points at all.

It may be better to swap DomKey out for a plain old int, and provide inline
static functions to generate the opaque integer values corresponding to a
Unicode code point, or a non-printable DOM key identifier, i.e. promote Habib's
Blink DOM key enum APIs to be a core part of ui/, and perhaps just provide
constants for the non-printable DOM key for convenience, and a
domkeyFromUnicode() helper in case of printable-character DOM key values.

WDYT?

[Habib Virji, 2015/07/31 16:17:29]

It is a good suggestion. In the code snippet though we receive only one value.
So codepoint cannot be used. Have updated code based on one value.

+    std::string dom_key_char = base::StringPrintf("%c", dom_key);
+    return WebString::fromUTF8(dom_key_char);
+  } else {
+    return WebString::fromUTF8(ui::KeycodeConverter::DomKeyToKeyString(
+        static_cast<ui::DomKey>(dom_key - DOM_KEY_PRINT_NON_DIFF)));
+  }
+}
+
+int BlinkPlatformImpl::domKeyEnumFromString(const WebString& key) {
+  ui::DomKey dom_key =
+      ui::KeycodeConverter::KeyStringToDomKey(key.utf8().data());
+  if (ui::DomKey::NONE == dom_key) {

[Wez, 2015/07/25 23:12:26]

As noted previously, NONE means something different from CHARACTER.

[Habib Virji, 2015/07/31 16:17:29]

Done.

+    return key.at(0);

[Wez, 2015/07/25 23:12:26]

My understanding is that WebString's are expressed in UTF16, so that at(0)
returns the first 16-bit code-point - if that's true then this will break if
|key| represents a character outside the Unicode Basic Multilingual Plane, such
as a Chinese GB18030 character.

[Habib Virji, 2015/07/31 16:17:29]

Have changed to using StringToInt as it appears to be better alternate to at..

+  }
+  return static_cast<int>(dom_key);

[Wez, 2015/07/25 23:12:26]

Note that the way this function is written currently, you have the character
values and DomKey values using the same space, from zero upwards, so you'll find
that in this impl, your DomKeys won't decode properly - they'll be treated as
characters.

[Habib Virji, 2015/07/31 16:17:29]

With the update checking DomKey::CHARACTER it should fix this issue.

+}
+
 }  // namespace content