Removing X11 native_event uses for key events.

ui/events/event.h

Index: ui/events/event.h
diff --git a/ui/events/event.h b/ui/events/event.h
index 7257f625f40299b32510fb80850b4259db5f00c0..b950125cf85c5b7c7fdfea68b7f532a6c06974da 100644
--- a/ui/events/event.h
+++ b/ui/events/event.h
@@ -587,7 +587,7 @@ class EVENTS_EXPORT ExtendedKeyEventData {
 // -- character_ is a UTF-16 character value.
 // -- key_code_ is conflated with character_ by some code, because both
 //    arrive in the wParam field of a Windows event.
-// -- code_ is "".
+// -- code_ is the empty string.
 //
 class EVENTS_EXPORT KeyEvent : public Event {
  public:
@@ -636,9 +636,24 @@ class EVENTS_EXPORT KeyEvent : public Event {
   // BMP characters.
   base::char16 GetCharacter() const;
 
+  // If this is a keystroke event with key_code_ VKEY_RETURN, returns '\r';
+  // otherwise returns the same as GetCharacter().
+  base::char16 GetUnmodifiedText() const;

[sky, 2014/09/02 18:23:15]

My initial reaction is that all of this is a mess. Do we really need all these
variants that are similar yet differ in some subtle way? Can't the subtleties be
moved to the call sites so that the code in event.h is clean and without
idiosyncrasies?

[kpschoedel, 2014/09/02 19:27:26]

That can be done, but these correspond to repeated patterns. Historically there
have been ad-hoc transformations of keystroke and/or character events with
duplicate code, or worse, *near* duplicate code with no explanation, and I've
encountered at least one bug due to someone making a change that was not carried
through everywhere. On top of that, callers of ui::KeyEvent::key_code() have not
generally been clear about which subset of ui::KeyboardCode (e.g. VKEY_SHIFT vs
VKEY_[LR]SHIFT) they expect (or indeed superset ui::KeyboardCode union UCS-2),
with the meaning of key_code_ not being documented, and defined only by implied
Windows emulation.

So the attempt to clarify and unify this is the motivation for the additional
accessors here. My own preference is still to have them as ui::KeyEvent members.
As an alternate proposal, I'd suggest the common , repeated operations be
provided as functions under ui/events/keycodes, or at least some designated
single place, with other current duplication (e.g.
content::GetWindowsKeyCodeWithoutLocation() and
DevToolsEventForwarder::VirtualKeyCodeWithoutLocation() are the same function)
eventually migrated.

[Elliot Glaysher, 2014/09/02 19:31:07]

One thing to watch out for (which you might already know) is that the vkey
constants aren't actually the same on all platforms. (I found this out the hard
way.) I would assume that in the long term, we probably want to standardize on
the windows constants. (Which is what I did in
mojo/services/public/interfaces/input_events/input_key_codes.mojom.)

[kpschoedel, 2014/09/02 19:47:16]

I suspect that's an accidental drift, and looking into that and merging
ui/events/keycodes/keyboard_codes_* is on the list of things I want to do, but
really I'd like it better if they could be defined in just one place… blink and
pdfium have their own copies, too.

+
+  // If the Control key is down in the event, returns a layout-independent
+  // character (corresponding to US layout); otherwise returns the same
+  // as GetUnmodifiedText().
+  base::char16 GetText() const;
+
   // Gets the platform key code. For XKB, this is the xksym value.
   void set_platform_keycode(uint32 keycode) { platform_keycode_ = keycode; }
   uint32 platform_keycode() const { return platform_keycode_; }
+
+  // Gets the associated (Windows-based) KeyboardCode for this key event.
+  // Historically, this has also been used to obtain the character associated
+  // with a character event, because both use the Window message 'wParam' field.
+  // This should be avoided; if necessary for backwards compatibility, use
+  // GetConflatedWindowsKeyCode().
   KeyboardCode key_code() const { return key_code_; }
 
   // True if this is a character event, false if this is a keystroke event.
@@ -648,6 +663,17 @@ class EVENTS_EXPORT KeyEvent : public Event {
   // events in an EventRewriter.
   void set_key_code(KeyboardCode key_code) { key_code_ = key_code; }
 
+  // Returns the same value as key_code(), except that located codes are
+  // returned in place of non-located ones (e.g. VKEY_LSHIFT or VKEY_RSHIFT
+  // instead of VKEY_SHIFT). This is a hybrid of semantic and physical
+  // for legacy DOM reasons.
+  KeyboardCode GetLocatedWindowsKeyboardCode() const;
+
+  // For a keystroke event, returns the same value as key_code().
+  // For a character event, returns the same value as GetCharacter().
+  // This exists for backwards compatibility with Windows key events.
+  uint16 GetConflatedWindowsKeyCode() const;
+
   // Returns true for [Alt]+<num-pad digit> Unicode alt key codes used by Win.
   // TODO(msw): Additional work may be needed for analogues on other platforms.
   bool IsUnicodeKeyCode() const;
@@ -671,6 +697,9 @@ class EVENTS_EXPORT KeyEvent : public Event {
   void set_is_char(bool is_char) { is_char_ = is_char; }
 
  private:
+  // True if the key press originated from a 'right' key (VKEY_RSHIFT, etc.).
+  bool IsRightSideKey() const;
+
   KeyboardCode key_code_;
 
   // String of 'code' defined in DOM KeyboardEvent (e.g. 'KeyA', 'Space')
@@ -693,7 +722,7 @@ class EVENTS_EXPORT KeyEvent : public Event {
   // This value represents the text that the key event will insert to input
   // field. For key with modifier key, it may have specifial text.
   // e.g. CTRL+A has '\x01'.
-  base::char16 character_;
+  mutable base::char16 character_;
 
   // Parts of our event handling require raw native events (see both the
   // windows and linux implementations of web_input_event in content/). Because