Fix handling of rewritten-Alt + Button1 -> Button3.

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 c2518897d248cb405734c8f3163bfee56f547dd6..52b6e0cd1c910ebeb3b4b25fb6b3527104a251c3 100644
--- a/chrome/browser/chromeos/events/event_rewriter.cc
+++ b/chrome/browser/chromeos/events/event_rewriter.cc
@@ -183,9 +183,7 @@ EventRewriter::DeviceType EventRewriter::DeviceAddedForTesting(
 
 void EventRewriter::RewriteLocatedEventForTesting(const ui::Event& event,
                                                   int* flags) {
-  MutableKeyState state = {*flags, ui::VKEY_UNKNOWN};
-  RewriteLocatedEvent(event, &state);
-  *flags = state.flags;
+  RewriteLocatedEvent(event, flags);
 }
 
 ui::EventRewriteStatus EventRewriter::RewriteEvent(
@@ -200,56 +198,17 @@ ui::EventRewriteStatus EventRewriter::RewriteEvent(
 #endif
   switch (event.type()) {
     case ui::ET_KEY_PRESSED:
-    case ui::ET_KEY_RELEASED: {
-      const ui::KeyEvent& key_event = static_cast<const ui::KeyEvent&>(event);
-      MutableKeyState state = {key_event.flags(), key_event.key_code()};
-      RewriteModifierKeys(key_event, &state);
-      RewriteNumPadKeys(key_event, &state);
-      RewriteExtendedKeys(key_event, &state);
-      RewriteFunctionKeys(key_event, &state);
-      if ((key_event.flags() != state.flags) ||
-          (key_event.key_code() != state.key_code)) {
-        ui::KeyEvent* rewritten_key_event = new ui::KeyEvent(key_event);
-        rewritten_event->reset(rewritten_key_event);
-        rewritten_key_event->set_flags(state.flags);
-        rewritten_key_event->set_key_code(state.key_code);
-        rewritten_key_event->set_character(
-            ui::GetCharacterFromKeyCode(state.key_code, state.flags));
-        rewritten_key_event->NormalizeFlags();
-#if defined(USE_X11)
-        xev = rewritten_key_event->native_event();
-        if (xev) {
-          XKeyEvent* xkey = &(xev->xkey);
-          UpdateX11EventMask(state.flags, &xkey->state);
-          xkey->keycode = XKeysymToKeycode(
-              gfx::GetXDisplay(),
-              ui::XKeysymForWindowsKeyCode(state.key_code,
-                                           state.flags & ui::EF_SHIFT_DOWN));
-        }
-#endif
-        return ui::EVENT_REWRITE_REWRITTEN;
-      }
-      return ui::EVENT_REWRITE_CONTINUE;
-    }
+    case ui::ET_KEY_RELEASED:
+      return RewriteKeyEvent(static_cast<const ui::KeyEvent&>(event),
+                             rewritten_event);
     case ui::ET_MOUSE_PRESSED:
     case ui::ET_MOUSE_RELEASED:
+      return RewriteMouseEvent(static_cast<const ui::MouseEvent&>(event),
+                               rewritten_event);
     case ui::ET_TOUCH_PRESSED:
-    case ui::ET_TOUCH_RELEASED: {
-      MutableKeyState state = {event.flags(), ui::VKEY_UNKNOWN};
-      RewriteLocatedEvent(event, &state);
-      if (event.flags() != state.flags) {
-        if (event.IsMouseEvent()) {
-          rewritten_event->reset(
-              new ui::MouseEvent(static_cast<const ui::MouseEvent&>(event)));
-        } else {
-          rewritten_event->reset(
-              new ui::TouchEvent(static_cast<const ui::TouchEvent&>(event)));
-        }
-        rewritten_event->get()->set_flags(state.flags);
-        return ui::EVENT_REWRITE_REWRITTEN;
-      }
-      return ui::EVENT_REWRITE_CONTINUE;
-    }
+    case ui::ET_TOUCH_RELEASED:
+      return RewriteTouchEvent(static_cast<const ui::TouchEvent&>(event),
+                               rewritten_event);
     default:
       return ui::EVENT_REWRITE_CONTINUE;
   }
@@ -364,6 +323,108 @@ bool EventRewriter::RewriteWithKeyboardRemappingsByKeyCode(
   return false;
 }
 
+ui::EventRewriteStatus EventRewriter::RewriteKeyEvent(
+    const ui::KeyEvent& key_event,
+    scoped_ptr<ui::Event>* rewritten_event) {
+  ui::EventRewriteStatus status = ui::EVENT_REWRITE_CONTINUE;
+  MutableKeyState state = {key_event.flags(), key_event.key_code()};
+  RewriteModifierKeys(key_event, &state);
+  RewriteNumPadKeys(key_event, &state);
+  RewriteExtendedKeys(key_event, &state);
+  RewriteFunctionKeys(key_event, &state);
+  if ((key_event.flags() != state.flags) ||
+      (key_event.key_code() != state.key_code) ||
+      (status != ui::EVENT_REWRITE_CONTINUE)) {

[sadrul, 2014/05/14 03:06:01]

status here is always _CONTINUE, right?

[kpschoedel, 2014/05/14 14:46:53]

Yes, I forgot to remove that when I copied this code back from the sticky keys
CL.

+    if (status == ui::EVENT_REWRITE_CONTINUE)

[sadrul, 2014/05/14 03:06:01]

ditto

+      status = ui::EVENT_REWRITE_REWRITTEN;
+    ui::KeyEvent* rewritten_key_event = new ui::KeyEvent(key_event);
+    rewritten_event->reset(rewritten_key_event);
+    rewritten_key_event->set_flags(state.flags);
+    rewritten_key_event->set_key_code(state.key_code);
+    rewritten_key_event->set_character(
+        ui::GetCharacterFromKeyCode(state.key_code, state.flags));
+    rewritten_key_event->NormalizeFlags();
+#if defined(USE_X11)
+    XEvent* xev = rewritten_key_event->native_event();
+    if (xev) {
+      XKeyEvent* xkey = &(xev->xkey);
+      UpdateX11EventMask(rewritten_key_event->flags(), &xkey->state);
+      xkey->keycode = XKeysymToKeycode(
+          gfx::GetXDisplay(),
+          ui::XKeysymForWindowsKeyCode(state.key_code,
+                                       state.flags & ui::EF_SHIFT_DOWN));
+    }
+#endif
+  }
+  return status;
+}
+
+ui::EventRewriteStatus EventRewriter::RewriteMouseEvent(
+    const ui::MouseEvent& mouse_event,
+    scoped_ptr<ui::Event>* rewritten_event) {
+  ui::EventRewriteStatus status = ui::EVENT_REWRITE_CONTINUE;
+  int flags = mouse_event.flags();
+  RewriteLocatedEvent(mouse_event, &flags);
+  if (mouse_event.flags() != flags) {
+    status = ui::EVENT_REWRITE_REWRITTEN;
+    ui::MouseEvent* rewritten_mouse_event = new ui::MouseEvent(mouse_event);
+    rewritten_event->reset(rewritten_mouse_event);
+    rewritten_mouse_event->set_flags(flags);
+#if defined(USE_X11)

[kpschoedel, 2014/05/12 21:58:03]

adopted from ash::StickKeysHandler::AppendModifier()

+    XEvent* xev = rewritten_mouse_event->native_event();
+    if (xev) {
+      switch (xev->type) {
+        case ButtonPress:
+        case ButtonRelease: {
+          XButtonEvent* xbutton = &(xev->xbutton);
+          UpdateX11EventMask(rewritten_mouse_event->flags(), &xbutton->state);
+          break;
+        }
+        case GenericEvent: {
+          XIDeviceEvent* xievent =
+              static_cast<XIDeviceEvent*>(xev->xcookie.data);
+          CHECK(xievent->evtype == XI_ButtonPress ||
+                xievent->evtype == XI_ButtonRelease);
+          UpdateX11EventMask(
+              rewritten_mouse_event->flags(),
+              reinterpret_cast<unsigned int*>(&xievent->mods.effective));
+          break;
+        }
+        default:
+          NOTREACHED();
+      }
+    }
+#endif
+  }
+  return status;
+}
+
+ui::EventRewriteStatus EventRewriter::RewriteTouchEvent(
+    const ui::TouchEvent& touch_event,
+    scoped_ptr<ui::Event>* rewritten_event) {
+  ui::EventRewriteStatus status = ui::EVENT_REWRITE_CONTINUE;
+  int flags = touch_event.flags();
+  RewriteLocatedEvent(touch_event, &flags);
+  if (touch_event.flags() != flags) {
+    status = ui::EVENT_REWRITE_REWRITTEN;
+    ui::TouchEvent* rewritten_touch_event = new ui::TouchEvent(touch_event);
+    rewritten_event->reset(rewritten_touch_event);
+    rewritten_touch_event->set_flags(flags);
+#if defined(USE_X11)
+    XEvent* xev = rewritten_touch_event->native_event();
+    if (xev) {
+      XIDeviceEvent* xievent = static_cast<XIDeviceEvent*>(xev->xcookie.data);
+      if (xievent) {
+        UpdateX11EventMask(
+            rewritten_touch_event->flags(),
+            reinterpret_cast<unsigned int*>(&xievent->mods.effective));
+      }
+    }
+#endif
+  }
+  return status;
+}
+
 void EventRewriter::RewriteModifierKeys(const ui::KeyEvent& key_event,
                                         MutableKeyState* state) {
   DCHECK(key_event.type() == ui::ET_KEY_PRESSED ||
@@ -653,13 +714,13 @@ void EventRewriter::RewriteFunctionKeys(const ui::KeyEvent& key_event,
 }
 
 void EventRewriter::RewriteLocatedEvent(const ui::Event& event,
-                                        MutableKeyState* state) {
+                                        int* flags) {
   const PrefService* pref_service = GetPrefService();
   if (!pref_service)
     return;
 
   // First, remap modifier masks.
-  state->flags = GetRemappedModifierMasks(*pref_service, event, state->flags);
+  *flags = GetRemappedModifierMasks(*pref_service, event, *flags);
 
 #if defined(USE_X11)
   // TODO(kpschoedel): de-X11 with unified device ids from crbug.com/360377
@@ -669,13 +730,15 @@ void EventRewriter::RewriteLocatedEvent(const ui::Event& event,
   XIDeviceEvent* xievent = static_cast<XIDeviceEvent*>(xevent->xcookie.data);
   if (xievent->evtype != XI_ButtonPress && xievent->evtype != XI_ButtonRelease)
     return;
+  UpdateX11EventMask(*flags,

[kpschoedel, 2014/05/12 21:58:03]

Essential to get the native flags for the remapped ui::EventFlags before testing
them below; this was the proximate cause for the Alt+Button1 symptom.

+                     reinterpret_cast<unsigned int*>(&xievent->mods.effective));
 
   // Then, remap Alt+Button1 to Button3.
   if ((xievent->evtype == XI_ButtonPress ||
        pressed_device_ids_.count(xievent->sourceid)) &&
       (xievent->mods.effective & Mod1Mask) && xievent->detail == Button1) {
-    state->flags &= ~(ui::EF_ALT_DOWN | ui::EF_LEFT_MOUSE_BUTTON);
-    state->flags |= ui::EF_RIGHT_MOUSE_BUTTON;
+    *flags &= ~(ui::EF_ALT_DOWN | ui::EF_LEFT_MOUSE_BUTTON);
+    *flags |= ui::EF_RIGHT_MOUSE_BUTTON;
     xievent->mods.effective &= ~Mod1Mask;
     xievent->detail = Button3;
     if (xievent->evtype == XI_ButtonRelease) {