Run event executor on the ui thread to remove the need to explicitly XFlush() the XTest calls.

remoting/host/event_executor_linux.cc

Index: remoting/host/event_executor_linux.cc
diff --git a/remoting/host/event_executor_linux.cc b/remoting/host/event_executor_linux.cc
index fddcab9011c377b2ff66caa345b03cc9c6ee58a4..7a8c09211c90be0968cbece3167061530a2694c7 100644
--- a/remoting/host/event_executor_linux.cc
+++ b/remoting/host/event_executor_linux.cc
@@ -7,39 +7,13 @@
 #include <X11/Xlib.h>
 #include <X11/keysym.h>
 #include <X11/extensions/XTest.h>
+#include <gdk/gdkx.h>
 
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/task.h"
 #include "remoting/proto/internal.pb.h"
 
-// TODO(jamiewalch): Class to ensure that XFlush is called regardless of what
-// fields are set in the MouseEvent. It's not worth refactoring HandleMouse to
-// avoid the early returns because calling XFlush is not really the correct
-// way of flushing the XTest requests; instead we should dispatch the requests
-// to a suitable UI thread.
-namespace {
-class ScopedXFlusher {
- public:
-  explicit ScopedXFlusher(Display* display)
-      : display_(display), needs_flush_(false) {
-  }
-
-  ~ScopedXFlusher() {
-    if (needs_flush_)
-      XFlush(display_);
-  }
-
-  void SignalFlush() {
-    needs_flush_ = true;
-  }
-
- private:
-  Display* display_;
-  bool needs_flush_;
-};
-}
-
 namespace remoting {
 
 using protocol::MouseEvent;
@@ -267,9 +241,7 @@ EventExecutorLinuxPimpl::~EventExecutorLinuxPimpl() {
 }
 
 bool EventExecutorLinuxPimpl::Init() {
-  // TODO(ajwong): We should specify the display string we are attaching to
-  // in the constructor.
-  display_ = XOpenDisplay(NULL);
+  display_ = GDK_DISPLAY_XDISPLAY(gdk_display_get_default());

[awong, 2011/02/14 17:27:29]

hmm...can we poke a hole into the message loop such that it gives us the
associated display back? I'm leery of leaking an implicit GDK dependency here.

In the contenxt, we'd want to store a MessageLoopUI instead of just MessageLoop.

[Jamie, 2011/02/14 19:12:38]

I agree, but I couldn't see any clean way of doing this. Passing a MessageLoopUI
into the context means also passing one into ServiceProcess::Initialize, which
made me nervous. Even if we do do that, it's not clear how we would get at a
platform-specific Display pointer from a cross-platform interface.

   if (!display_) {
     LOG(ERROR) << "Unable to open display";
     return false;
@@ -329,13 +301,9 @@ void EventExecutorLinuxPimpl::HandleKey(const KeyEvent* key_event) {
           << " sending keysym: " << keysym
           << " to keycode: " << keycode;
   XTestFakeKeyEvent(display_, keycode, key_event->pressed(), CurrentTime);
-
-  // TODO(jamiewalch): Get rid of this once we're dispatching to the UI thread.
-  XFlush(display_);
 }
 
 void EventExecutorLinuxPimpl::HandleMouse(const MouseEvent* event) {
-  ScopedXFlusher flusher(display_);
   if (event->has_x() && event->has_y()) {
     if (event->x() < 0 || event->y() < 0 ||
         event->x() > width_ || event->y() > height_) {
@@ -347,7 +315,6 @@ void EventExecutorLinuxPimpl::HandleMouse(const MouseEvent* event) {
 
     VLOG(3) << "Moving mouse to " << event->x()
             << "," << event->y();
-    flusher.SignalFlush();
     XTestFakeMotionEvent(display_, DefaultScreen(display_),
                          event->x(), event->y(),
                          CurrentTime);
@@ -364,7 +331,6 @@ void EventExecutorLinuxPimpl::HandleMouse(const MouseEvent* event) {
 
     VLOG(3) << "Button " << event->button()
             << " received, sending down " << button_number;
-    flusher.SignalFlush();
     XTestFakeButtonEvent(display_, button_number, event->button_down(),
                          CurrentTime);
   }