Remove EF_FUNCTION_KEY and EF_NUMPAD_KEY.

chrome/browser/chromeos/events/event_rewriter.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/events/event_rewriter.h"
 
 #include <vector>
 
 #include "ash/sticky_keys/sticky_keys_controller.h"
 #include "ash/wm/window_state.h"
 #include "ash/wm/window_util.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
 #include "base/sys_info.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host_impl.h"
 #include "chrome/browser/extensions/extension_commands_global_registry.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/chromeos_switches.h"
 #include "components/user_manager/user_manager.h"
 #include "ui/base/ime/chromeos/ime_keyboard.h"
 #include "ui/base/ime/chromeos/input_method_manager.h"
 #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"
 
 #if defined(USE_X11)
 #include <X11/extensions/XInput2.h>
 #include <X11/Xlib.h>
 
 // Get rid of macros from Xlib.h that conflicts with other parts of the code.
 #undef RootWindow
 #undef Status
@@ skipping 241 matching lines @@
       numpad_xevent.xkey = xkeyevent.xkey;
       // Remove the shift state before getting key char.
       // Because X11/XKB sometimes returns unexpected key char for
       // Shift+NumpadKey. e.g. Shift+Numpad_4 returns 'D', etc.
       numpad_xevent.xkey.state &= ~ShiftMask;
       numpad_xevent.xkey.state |= Mod2Mask;  // Always set NumLock mask.
       if (!(flags & ui::EF_SHIFT_DOWN))
         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
   if (!rewritten_key_event)
     rewritten_key_event = new ui::KeyEvent(key_event);
   rewritten_key_event->set_flags(flags);
   rewritten_key_event->set_key_code(key_code);
 #if defined(USE_X11)
   ui::UpdateX11EventForFlags(rewritten_key_event);
   rewritten_key_event->NormalizeFlags();
@@ skipping 400 matching lines @@
             : chromeos::input_method::InputMethodManager::Get()
                   ->GetImeKeyboard();
     ime_keyboard->SetCapsLockEnabled(!ime_keyboard->CapsLockIsEnabled());
   }
 }
 
 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,
                                         MutableKeyState* state) {
   DCHECK(key_event.type() == ui::ET_KEY_PRESSED ||
          key_event.type() == ui::ET_KEY_RELEASED);
 
   MutableKeyState incoming = *state;
   bool rewritten = false;
 
@@ skipping 200 matching lines @@
   for (const auto& keyboard : keyboards) {
     if (keyboard.id == device_id) {
       return KeyboardDeviceAddedInternal(
           keyboard.id, keyboard.name, keyboard.vendor_id, keyboard.product_id);
     }
   }
   return kDeviceUnknown;
 }
 
 }  // namespace chromeos