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 2603e0519400b43345afd263b049287000255c77..4f7a99bed1b8a7befcdcdcc506ea405ab756b6d9 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;
   }
@@ -369,6 +328,100 @@ bool EventRewriter::RewriteWithKeyboardRemappingsByKeyCode(
   return false;
 }
 
+ui::EventRewriteStatus EventRewriter::RewriteKeyEvent(
+    const ui::KeyEvent& key_event,
+    scoped_ptr<ui::Event>* rewritten_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)) {
+    return ui::EVENT_REWRITE_CONTINUE;
+  }
+  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);

[sadrul, 2014/06/02 00:19:52]

Add a CHECK here that xev->type == KeyPress or KeyRelease

[kpschoedel, 2014/06/02 00:52:36]

Done.

+    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 ui::EVENT_REWRITE_REWRITTEN;
+}
+
+ui::EventRewriteStatus EventRewriter::RewriteMouseEvent(
+    const ui::MouseEvent& mouse_event,
+    scoped_ptr<ui::Event>* rewritten_event) {
+  int flags = mouse_event.flags();
+  RewriteLocatedEvent(mouse_event, &flags);
+  if (mouse_event.flags() == flags)
+    return ui::EVENT_REWRITE_CONTINUE;
+  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)
+  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 ui::EVENT_REWRITE_REWRITTEN;
+}
+
+ui::EventRewriteStatus EventRewriter::RewriteTouchEvent(
+    const ui::TouchEvent& touch_event,
+    scoped_ptr<ui::Event>* rewritten_event) {
+  int flags = touch_event.flags();
+  RewriteLocatedEvent(touch_event, &flags);
+  if (touch_event.flags() == flags)
+    return ui::EVENT_REWRITE_CONTINUE;
+  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 ui::EVENT_REWRITE_REWRITTEN;
+}
+
 void EventRewriter::RewriteModifierKeys(const ui::KeyEvent& key_event,
                                         MutableKeyState* state) {
   DCHECK(key_event.type() == ui::ET_KEY_PRESSED ||
@@ -658,13 +711,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
@@ -674,13 +727,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,
+                     reinterpret_cast<unsigned int*>(&xievent->mods.effective));

[sadrul, 2014/06/02 00:19:52]

Should this happen after |flags| is changed in lines 737-738 below (so, after
line 751)?

[kpschoedel, 2014/06/02 00:52:36]

No, this is updating the native (X11) flags so that the test at 736 below uses
the results of key rewriting, if there was any.

One of the upcoming CLs (post support for XI2 key events) replaces this whole
section with generic ui::Event tests. 

[sadrul, 2014/06/02 00:58:47]

In that case, do we need to UpdateX11EventMask() again after |flags| is changed
in lines 737-738?

 
   // 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) {