Fixing the issue of GDK discarding unsupported XInput events.

Current implementation of message_pump_glib_x uses both X event queue and GDK's g_main_context_iteration to read event messages.

If the first X event in the X event queue can be dispatched directly, it will be extracted and dispatched.
If the first X event in the X event queue can't be dispatched directly, g_main_context_iteration will be called to translate X events into GDK evnets.
If the first X event can't be dispatched directly and is followed by X events that can be dispatched directly in the event queue,
those events that can be dispatched directly will also go through GDK and may get lost if GDK does not recognize them (e.g. XInput2 events).

To fix this, a GDK event filter is setup to intercept all the X events before they are translated into GDK events.
In the filter if the X events are found to be wanted, they will get dispatched directly instead of translated into GDK events.

BUG=
TEST=

Review URL: http://codereview.chromium.org/7050030
Patch from Yufeng Shen <miletus@chromium.org>.

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86610 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 77 insertions, 40 deletions

diff --git a/base/message_pump_glib_x.cc b/base/message_pump_glib_x.cc
index b4d677e..c694114 100644
--- a/base/message_pump_glib_x.cc
+++ b/base/message_pump_glib_x.cc
@@ -33,6 +33,7 @@ MessagePumpGlibX::MessagePumpGlibX() : base::MessagePumpForUI(),
     dispatching_event_(false),
     capture_x_events_(0),
     capture_gdk_events_(0) {
+  gdk_window_add_filter(NULL, &GdkEventFilter, this);
   gdk_event_handler_set(&EventDispatcherX, this, NULL);
 
 #if defined(HAVE_XINPUT2)
@@ -44,50 +45,63 @@ MessagePumpGlibX::MessagePumpGlibX() : base::MessagePumpForUI(),
 MessagePumpGlibX::~MessagePumpGlibX() {
 }
 
-bool MessagePumpGlibX::RunOnce(GMainContext* context, bool block) {
-  GdkDisplay* gdisp = gdk_display_get_default();
-  if (!gdisp || !GetDispatcher())
-    return MessagePumpForUI::RunOnce(context, block);
+bool MessagePumpGlibX::ShouldCaptureXEvent(XEvent* xev) {
+  return capture_x_events_[xev->type]
+#if defined(HAVE_XINPUT2)
+        && (xev->type != GenericEvent || xev->xcookie.extension == xiopcode_)
+#endif
+    ;
+}
 
-  Display* display = GDK_DISPLAY_XDISPLAY(gdisp);
+
+bool MessagePumpGlibX::ProcessXEvent(XEvent* xev) {
   bool should_quit = false;
 
-  if (XPending(display)) {
-    XEvent xev;
-    XPeekEvent(display, &xev);
-    if (capture_x_events_[xev.type]
 #if defined(HAVE_XINPUT2)
-        && (xev.type != GenericEvent || xev.xcookie.extension == xiopcode_)
+  bool have_cookie = false;
+  if (xev->type == GenericEvent &&
+      XGetEventData(xev->xgeneric.display, &xev->xcookie)) {
+    have_cookie = true;
+  }
 #endif
-        ) {
-      XNextEvent(display, &xev);
+
+  if (!WillProcessXEvent(xev)) {
+    MessagePumpGlibXDispatcher::DispatchStatus status =
+      static_cast<MessagePumpGlibXDispatcher*>
+      (GetDispatcher())->DispatchX(xev);
+
+    if (status == MessagePumpGlibXDispatcher::EVENT_QUIT) {
+      should_quit = true;
+      Quit();
+    } else if (status == MessagePumpGlibXDispatcher::EVENT_IGNORED) {
+      DLOG(WARNING) << "Event (" << xev->type << ") not handled.";
+    }
+  }
 
 #if defined(HAVE_XINPUT2)
-      bool have_cookie = false;
-      if (xev.type == GenericEvent &&
-          XGetEventData(xev.xgeneric.display, &xev.xcookie)) {
-        have_cookie = true;
-      }
+  if (have_cookie) {
+    XFreeEventData(xev->xgeneric.display, &xev->xcookie);
+  }
 #endif
 
-      if (!WillProcessXEvent(&xev)) {
-        MessagePumpGlibXDispatcher::DispatchStatus status =
-            static_cast<MessagePumpGlibXDispatcher*>
-            (GetDispatcher())->DispatchX(&xev);
+  return should_quit;
+}
 
-        if (status == MessagePumpGlibXDispatcher::EVENT_QUIT) {
-          should_quit = true;
-          Quit();
-        } else if (status == MessagePumpGlibXDispatcher::EVENT_IGNORED) {
-          DLOG(WARNING) << "Event (" << xev.type << ") not handled.";
-        }
-      }
+bool MessagePumpGlibX::RunOnce(GMainContext* context, bool block) {
+  GdkDisplay* gdisp = gdk_display_get_default();
+  if (!gdisp || !GetDispatcher())
+    return MessagePumpForUI::RunOnce(context, block);
 
-#if defined(HAVE_XINPUT2)
-      if (have_cookie) {
-        XFreeEventData(xev.xgeneric.display, &xev.xcookie);
-      }
-#endif
+  Display* display = GDK_DISPLAY_XDISPLAY(gdisp);
+
+  if (XPending(display)) {
+    XEvent xev;
+    XPeekEvent(display, &xev);
+
+    if (ShouldCaptureXEvent(&xev)) {
+      XNextEvent(display, &xev);
+      if (ProcessXEvent(&xev))
+        return true;
     } else {
       // TODO(sad): A couple of extra events can still sneak in during this.
       // Those should be sent back to the X queue from the dispatcher
@@ -98,9 +112,6 @@ bool MessagePumpGlibX::RunOnce(GMainContext* context, bool block) {
     }
   }
 
-  if (should_quit)
-    return true;
-
   bool retvalue;
   if (gdksource_) {
     // Replace the dispatch callback of the GDK event source temporarily so that
@@ -121,6 +132,20 @@ bool MessagePumpGlibX::RunOnce(GMainContext* context, bool block) {
   return retvalue;
 }
 
+GdkFilterReturn MessagePumpGlibX::GdkEventFilter(GdkXEvent* gxevent,
+                                                 GdkEvent* gevent,
+                                                 gpointer data) {
+  MessagePumpGlibX* pump = static_cast<MessagePumpGlibX*>(data);
+  XEvent* xev = static_cast<XEvent*>(gxevent);
+
+  if (pump->ShouldCaptureXEvent(xev) && pump->GetDispatcher()) {
+    pump->ProcessXEvent(xev);
+    return GDK_FILTER_REMOVE;
+  }
+
+  return GDK_FILTER_CONTINUE;
+}
+
 bool MessagePumpGlibX::WillProcessXEvent(XEvent* xevent) {
   ObserverListBase<Observer>::Iterator it(observers());
   Observer* obs;
@@ -143,10 +168,7 @@ void MessagePumpGlibX::EventDispatcherX(GdkEvent* event, gpointer data) {
   } else if (!pump_x->IsDispatchingEvent()) {
     if (event->type != GDK_NOTHING &&
         pump_x->capture_gdk_events_[event->type]) {
-      // TODO(sad): An X event is caught by the GDK handler. Put it back in the
-      // X queue so that we catch it in the next iteration. When done, the
-      // following DLOG statement will be removed.
-      DLOG(WARNING) << "GDK received an event it shouldn't have";
+      NOTREACHED() << "GDK received an event it shouldn't have";
     }
   }
 
diff --git a/base/message_pump_glib_x.h b/base/message_pump_glib_x.h
index 6bd29c9..ed29e6a 100644
--- a/base/message_pump_glib_x.h
+++ b/base/message_pump_glib_x.h
@@ -31,8 +31,23 @@ class MessagePumpGlibX : public MessagePumpForUI {
   virtual bool RunOnce(GMainContext* context, bool block);
 
  private:
+  // Some XEvent's can't be directly read from X event queue and will go
+  // through GDK's dispatching process and may get discarded. This function
+  // sets up a filter to intercept those XEvent's we are interested in
+  // and dispatches them so that they won't get lost.
+  static GdkFilterReturn GdkEventFilter(GdkXEvent* gxevent,
+                                        GdkEvent* gevent,
+                                        gpointer data);
+
   static void EventDispatcherX(GdkEvent* event, gpointer data);
 
+  // Decides whether we are interested in processing this XEvent.
+  bool ShouldCaptureXEvent(XEvent* event);
+
+  // Dispatches the XEvent and returns true if we should exit the current loop
+  // of message processing.
+  bool ProcessXEvent(XEvent* event);
+
   // Sends the event to the observers. If an observer returns true, then it does
   // not send the event to any other observers and returns true. Returns false
   // if no observer returns true.