Fixing the issue of GDK discarding unsupported XInput events.

base/message_pump_glib_x.cc

 // Copyright (c) 2011 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 "base/message_pump_glib_x.h"
 
 #include <gdk/gdkx.h>
 #if defined(HAVE_XINPUT2)
 #include <X11/extensions/XInput2.h>
 #else
@@ skipping 15 matching lines @@
 namespace base {
 
 MessagePumpGlibX::MessagePumpGlibX() : base::MessagePumpForUI(),
 #if defined(HAVE_XINPUT2)
     xiopcode_(-1),
 #endif
     gdksource_(NULL),
     dispatching_event_(false),
     capture_x_events_(0),
     capture_gdk_events_(0) {
+  gdk_window_add_filter(NULL, GdkEventFilter, this);

[sadrul, 2011/05/24 15:34:18]

To keep with the existing code, '&GdkEventFilter'

[Yufeng Shen (Slow to review), 2011/05/24 18:30:50]

Done.

   gdk_event_handler_set(&EventDispatcherX, this, NULL);
 
 #if defined(HAVE_XINPUT2)
   InitializeXInput2();
 #endif
   InitializeEventsToCapture();
 }
 
 MessagePumpGlibX::~MessagePumpGlibX() {
 }
 
+bool MessagePumpGlibX::ShouldCaptureXEvent(XEvent* xev) {
+  return capture_x_events_[xev->type]
+#if defined(HAVE_XINPUT2)
+        && (xev->type != GenericEvent || xev->xcookie.extension == xiopcode_)
+#endif
+    ;
+}
+
+
+bool MessagePumpGlibX::ProcessXEvent(XEvent* xev) {
+  bool should_quit = false;
+
+#if defined(HAVE_XINPUT2)
+  bool have_cookie = false;
+  if (xev->type == GenericEvent &&
+      XGetEventData(xev->xgeneric.display, &xev->xcookie)) {
+    have_cookie = true;
+  }
+#endif
+
+  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)
+  if (have_cookie) {
+    XFreeEventData(xev->xgeneric.display, &xev->xcookie);
+  }
+#endif
+
+  return should_quit;
+}
+
 bool MessagePumpGlibX::RunOnce(GMainContext* context, bool block) {
   GdkDisplay* gdisp = gdk_display_get_default();
   if (!gdisp || !GetDispatcher())
     return MessagePumpForUI::RunOnce(context, block);
 
   Display* display = GDK_DISPLAY_XDISPLAY(gdisp);
   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_)
-#endif
-        ) {
+
+    if (ShouldCaptureXEvent(&xev)) {
       XNextEvent(display, &xev);
-
-#if defined(HAVE_XINPUT2)
-      bool have_cookie = false;
-      if (xev.type == GenericEvent &&
-          XGetEventData(xev.xgeneric.display, &xev.xcookie)) {
-        have_cookie = true;
-      }
-#endif
-
-      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)
-      if (have_cookie) {
-        XFreeEventData(xev.xgeneric.display, &xev.xcookie);
-      }
-#endif
+      should_quit = ProcessXEvent(&xev);

[sadrul, 2011/05/24 15:34:18]

Using should_quit isn't necessary anymore: 
  if (ProcessXEvent(&xev))
    return true;

[Yufeng Shen (Slow to review), 2011/05/24 18:30:50]

Done.

     } 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
       // EventDispatcherX.
       if (gdksource_)
         gdksource_->source_funcs->dispatch = gdkdispatcher_;
       g_main_context_iteration(context, FALSE);
     }
   }
 
@@ skipping 13 matching lines @@
     dispatching_event_ = false;
 
     gdksource_->source_funcs->dispatch = cb;
   } else {
     retvalue = g_main_context_iteration(context, 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);

[sadrul, 2011/05/24 15:34:18]

This looks good as is. But should this do anything with the return value
(probably not)?

[Yufeng Shen (Slow to review), 2011/05/24 18:30:50]

Since it is in the GDK's message processing loop, I don't know if there is a way
to break the loop right here if the ProcessXEvent(xev) return true (meaning
'should quit'). I change it to call pump->Quit() if ProcessXEvent() return true
so that the quit signal will be handled soon after the GDK's message loop is
finished.

+    return GDK_FILTER_REMOVE;
+  }
+
+  return GDK_FILTER_CONTINUE;
+}
+
+

[sadrul, 2011/05/24 15:34:18]

Too many new lines!

[Yufeng Shen (Slow to review), 2011/05/24 18:30:50]

Done.

+
 bool MessagePumpGlibX::WillProcessXEvent(XEvent* xevent) {
   ObserverListBase<Observer>::Iterator it(observers());
   Observer* obs;
   while ((obs = it.GetNext()) != NULL) {
     MessagePumpXObserver* xobs =
         static_cast<MessagePumpXObserver*>(obs);
     if (xobs->WillProcessXEvent(xevent))
       return true;
   }
   return false;
 }
 
 void MessagePumpGlibX::EventDispatcherX(GdkEvent* event, gpointer data) {
   MessagePumpGlibX* pump_x = reinterpret_cast<MessagePumpGlibX*>(data);
 
   if (!pump_x->gdksource_) {
     pump_x->gdksource_ = g_main_current_source();
     if (pump_x->gdksource_)
       pump_x->gdkdispatcher_ = pump_x->gdksource_->source_funcs->dispatch;
   } 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";

[sadrul, 2011/05/24 15:34:18]

Can you please replace the DLOG with a NOTREACHED here, and see if that causes
any problems? If not, get rid of the TODO too. From what I understand, such an
event should never reach here anymore?

[Yufeng Shen (Slow to review), 2011/05/24 18:30:50]

Done.

     }
   }
 
   pump_x->DispatchEvents(event);
 }
 
 void MessagePumpGlibX::InitializeEventsToCapture(void) {
   // TODO(sad): Decide which events we want to capture and update the tables
   // accordingly.
   capture_x_events_[KeyPress] = true;
@@ skipping 38 matching lines @@
     return;
   }
 }
 #endif  // HAVE_XINPUT2
 
 bool MessagePumpXObserver::WillProcessXEvent(XEvent* xev) {
   return false;
 }
 
 }  // namespace base