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

Index: base/message_pump_x.h
diff --git a/base/message_pump_glib_x.h b/base/message_pump_x.h
similarity index 60%
rename from base/message_pump_glib_x.h
rename to base/message_pump_x.h
index ed29e6a777e2c1cb5643b13df08aed741e249cd8..c38a90b21952ae8124f4433558e198fef7c93286 100644
--- a/base/message_pump_glib_x.h
+++ b/base/message_pump_x.h
@@ -2,8 +2,8 @@
 // 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"
@@ -12,23 +12,53 @@
 
 #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
@@ -83,15 +113,22 @@ class MessagePumpGlibX : public MessagePumpForUI {
 #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