Refactor the glib message-pump, and use it as the base for a gtk message pump and an X message pump.

base/message_pump_x.h

 // 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.
 
-#ifndef BASE_MESSAGE_PUMP_GLIB_X_H
-#define BASE_MESSAGE_PUMP_GLIB_X_H
+#ifndef BASE_MESSAGE_PUMP_X_H
+#define BASE_MESSAGE_PUMP_X_H
 
 #include "base/message_pump.h"
 #include "base/message_pump_glib.h"
 
 #include <bitset>
 
 #include <glib.h>
 #include <gtk/gtk.h>
-#include <X11/X.h>
 
 typedef union _XEvent XEvent;
 
 namespace base {
 
-class MessagePumpGlibX : public MessagePumpForUI {
+// The documentation for this class is in message_pump_glib.h
+class MessagePumpDispatcher {
  public:
-  MessagePumpGlibX();
-  virtual ~MessagePumpGlibX();
+  virtual ~MessagePumpDispatcher() {}

[sky, 2011/06/24 15:27:12]

If the pump doesn't own the dispatcher, make this protected. Same comment for
the Observer down below.

[sadrul, 2011/06/24 17:14:01]

Done.

+
+  enum DispatchStatus {
+    EVENT_IGNORED,    // The event was not processed.
+    EVENT_PROCESSED,  // The event has been processed.
+    EVENT_QUIT        // The event was processed and the message-loop should
+                      // terminate.
+  };
+
+  // Dispatches the event. EVENT_IGNORED is returned if the event was ignored
+  // (i.e. not processed). EVENT_PROCESSED is returned if the event was
+  // processed. The nested loop exits immediately if EVENT_QUIT is returned.
+  virtual DispatchStatus DispatchX(XEvent* xevent) = 0;

[sky, 2011/06/24 15:27:12]

nit: DispatchX sounds pretty vague. How about DispatchXEvent? Or even just
Dispatch

[sadrul, 2011/06/24 17:14:01]

I have changed it to Dispatch. (it used to be Dispatch, but then clang was upset
because the parent class had a 'Dispatch(GdkEvent*)')

+};
+
+// The documentation for this class is in message_pump_glib.h
+class BASE_API MessagePumpObserver {
+ public:
+  virtual ~MessagePumpObserver() {}
+
+  // This method is called before processing an XEvent. If the method returns
+  // true, it indicates the event has already been handled, so the event is not
+  // processed any farther. If the method returns false, the event dispatching
+  // proceeds as normal.
+  virtual bool WillProcessXEvent(XEvent* xevent);

[sky, 2011/06/24 15:27:12]

For clarity consider making this return an enum. In fact its probably worth
changing all the various observers to return an enum instead of bool. If you
agree, don't feel you need to change all observers now.

[sadrul, 2011/06/24 17:14:01]

Sounds like a good idea. I have done it here. The GTK handlers don't have a
return value, so I have kept them as they are.

+};
+
+// This class implements a message-pump for dispatching X events.
+class MessagePumpX : public MessagePumpGlib {
+ public:
+  MessagePumpX();
+  virtual ~MessagePumpX();
 
   // Indicates whether a GDK event was injected by chrome (when |true|) or if it
   // was captured and being processed by GDK (when |false|).
   bool IsDispatchingEvent(void) { return dispatching_event_; }
 
-  // Overridden from MessagePumpForUI:
-  virtual bool RunOnce(GMainContext* context, bool block);
+  // Overridden from MessagePumpGlib:
+  virtual bool RunOnce(GMainContext* context, bool block) OVERRIDE;
 
  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);
 
@@ skipping 34 matching lines @@
   // was captured and being processed by GDK (when |false|).
   bool dispatching_event_;
 
 #if ! GTK_CHECK_VERSION(2,18,0)
 // GDK_EVENT_LAST was introduced in GTK+ 2.18.0. For earlier versions, we pick a
 // large enough value (the value of GDK_EVENT_LAST in 2.18.0) so that it works
 // for all versions.
 #define GDK_EVENT_LAST 37
 #endif
 
+// Ideally we would #include X.h for LASTEvent, but it brings in a lot of stupid
+// stuff (like Time, CurrentTime etc.) that messes up a lot of things. So define
+// XLASTEvent here to a large enough value so that it works.
+#define XLASTEvent 40
+
   // We do not want to process all the events ourselves. So we use a lookup
   // table to quickly check if a particular event should be handled by us or if
   // it should be passed on to the default GDK handler.
-  std::bitset<LASTEvent> capture_x_events_;
+  std::bitset<XLASTEvent> capture_x_events_;
   std::bitset<GDK_EVENT_LAST> capture_gdk_events_;
 
-  DISALLOW_COPY_AND_ASSIGN(MessagePumpGlibX);
+  DISALLOW_COPY_AND_ASSIGN(MessagePumpX);
 };
 
+typedef MessagePumpX MessagePumpForUI;
+
 }  // namespace base
 
-#endif  // BASE_MESSAGE_PUMP_GLIB_X_H
+#endif  // BASE_MESSAGE_PUMP_X_H