Remove EF_FUNCTION_KEY and EF_NUMPAD_KEY.

chrome/browser/chromeos/events/event_rewriter.cc

Index: chrome/browser/chromeos/events/event_rewriter.cc
diff --git a/chrome/browser/chromeos/events/event_rewriter.cc b/chrome/browser/chromeos/events/event_rewriter.cc
index 015502d240ca2cde5d1b5d685c8ceb4fbe9f7b3c..d21842f94e324748661a69833dfd1abd93d09437 100644
--- a/chrome/browser/chromeos/events/event_rewriter.cc
+++ b/chrome/browser/chromeos/events/event_rewriter.cc
@@ -26,6 +26,8 @@
 #include "ui/events/devices/device_data_manager.h"
 #include "ui/events/event.h"
 #include "ui/events/event_utils.h"
+#include "ui/events/keycodes/dom3/dom_code.h"
+#include "ui/events/keycodes/dom4/keycode_converter.h"
 #include "ui/events/keycodes/keyboard_code_conversion.h"
 #include "ui/wm/core/window_util.h"
 
@@ -287,6 +289,7 @@ void EventRewriter::BuildRewrittenKeyEvent(
         numpad_xevent.xkey.keycode = original_x11_keycode;
       rewritten_key_event->set_character(
           ui::GetCharacterFromXEvent(&numpad_xevent));
+      rewritten_key_event->native_event()->xkey.state |= Mod2Mask;

[Wez, 2015/05/05 21:23:44]

This code is super confusing; does native_event() refer to |numpad_event|, or to
|xkeyevent|, or to some separate internal XEvent...?

[kpschoedel, 2015/05/05 22:19:07]

It is (and I don't actually understand the intent of this block). Fortunately
the whole USE_X11 section should be removable as soon as EventRewriter starts
rewriting DomCode & DomKey correctly. (That's next on my list.)
It's a copy of xkeyevent owned by rewritten_key_event; i.e. the modified event
that EventRewriter returns. Mod2Mask needs to be set because ChromeOS always
interprets numpad keys as numbers. Really the lookup should handle this and we
shouldn't have to set it explicitly on the event, but that X11 lookup code is a
bit fragmented (and further down my list).

     }
   }
 #endif
@@ -707,26 +710,31 @@ void EventRewriter::RewriteNumPadKeys(const ui::KeyEvent& key_event,
                                       MutableKeyState* state) {
   DCHECK(key_event.type() == ui::ET_KEY_PRESSED ||
          key_event.type() == ui::ET_KEY_RELEASED);
-  if (!(state->flags & ui::EF_NUMPAD_KEY))
-    return;
   MutableKeyState incoming = *state;
-
-  static const KeyboardRemapping kNumPadRemappings[] = {
-      {ui::VKEY_INSERT, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD0, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_DELETE, ui::EF_NUMPAD_KEY, ui::VKEY_DECIMAL, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_END, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD1, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_DOWN, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD2, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_NEXT, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD3, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_LEFT, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD4, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_CLEAR, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD5, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_RIGHT, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD6, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_HOME, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD7, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_UP, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD8, ui::EF_NUMPAD_KEY},
-      {ui::VKEY_PRIOR, ui::EF_NUMPAD_KEY, ui::VKEY_NUMPAD9, ui::EF_NUMPAD_KEY},
+  static const struct NumPadRemapping {
+    ui::DomCode input_dom_code;
+    ui::KeyboardCode input_key_code;
+    ui::KeyboardCode output_key_code;
+  } kNumPadRemappings[] = {
+      {ui::DomCode::NUMPAD_DECIMAL, ui::VKEY_DELETE, ui::VKEY_DECIMAL},
+      {ui::DomCode::NUMPAD0, ui::VKEY_INSERT, ui::VKEY_NUMPAD0},
+      {ui::DomCode::NUMPAD1, ui::VKEY_END, ui::VKEY_NUMPAD1},
+      {ui::DomCode::NUMPAD2, ui::VKEY_DOWN, ui::VKEY_NUMPAD2},
+      {ui::DomCode::NUMPAD3, ui::VKEY_NEXT, ui::VKEY_NUMPAD3},
+      {ui::DomCode::NUMPAD4, ui::VKEY_LEFT, ui::VKEY_NUMPAD4},
+      {ui::DomCode::NUMPAD5, ui::VKEY_CLEAR, ui::VKEY_NUMPAD5},
+      {ui::DomCode::NUMPAD6, ui::VKEY_RIGHT, ui::VKEY_NUMPAD6},
+      {ui::DomCode::NUMPAD7, ui::VKEY_HOME, ui::VKEY_NUMPAD7},
+      {ui::DomCode::NUMPAD8, ui::VKEY_UP, ui::VKEY_NUMPAD8},
+      {ui::DomCode::NUMPAD9, ui::VKEY_PRIOR, ui::VKEY_NUMPAD9},

[Wez, 2015/05/05 21:23:44]

Remind me - we always rewrite num-pad keys to behave as numeric keys, even if
the user has an external keyboard with a NumLock key?

[kpschoedel, 2015/05/05 22:19:07]

Yes. In the future, either when/if I get around to refactoring X11 lookup, or
when no ChromeOS still use X11, this function will go away, because we'll
generate the correct event to start with. 

   };
-
-  RewriteWithKeyboardRemappingsByKeyCode(
-      kNumPadRemappings, arraysize(kNumPadRemappings), incoming, state);
+  for (const auto& map : kNumPadRemappings) {

[Wez, 2015/05/05 21:23:44]

We end up with a lot of these remapping loops; if possible it'd be preferable to
merge the remappings into a single map so we can e.g. do a single binary search
in it to find remappings - in this case mapping
pair<DomCode,KeyboardCode>->KeyboardCode, although in future we're going to need
to handle DomKey as well.

[kpschoedel, 2015/05/05 22:19:08]

Yes, I've been considering this as I do the work to handle DomCode & DomKey,
though I'm currently leaning toward doing that expediently in hope of getting to
the point where we can remove this entirely.

+    if ((incoming.key_code == map.input_key_code) &&
+        (key_event.code() == map.input_dom_code)) {
+      state->key_code = map.output_key_code;
+      break;
+    }
+  }
 }
 
 void EventRewriter::RewriteExtendedKeys(const ui::KeyEvent& key_event,