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

remoting/host/event_executor_linux.cc

 // Copyright (c) 2010 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 "remoting/host/event_executor_linux.h"
 
 #include <X11/Xlib.h>
 #include <X11/keysym.h>
 #include <X11/extensions/XTest.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;
 using protocol::KeyEvent;
 
 static int MouseButtonToX11ButtonNumber(
     protocol::MouseEvent::MouseButton button) {
   switch (button) {
     case MouseEvent::BUTTON_LEFT:
       return 1;
@@ skipping 169 matching lines @@
 static int ChromotocolKeycodeToX11Keysym(int32_t keycode) {
   if (keycode < 0 || keycode > 255) {
     return -1;
   }
 
   return kPepperToX11Keysym[keycode];
 }
 
 class EventExecutorLinuxPimpl {
  public:
-  explicit EventExecutorLinuxPimpl(EventExecutorLinux* executor);
+  explicit EventExecutorLinuxPimpl(EventExecutorLinux* executor,
+                                   Display* display);
   ~EventExecutorLinuxPimpl();
 
   bool Init();  // TODO(ajwong): Do we really want this to be synchronous?
 
   void HandleMouse(const MouseEvent* message);
   void HandleKey(const KeyEvent* key_event);
 
  private:
   void DeinitXlib();
 
   // Reference to containing class so we can access friend functions.
   // Not owned.
   EventExecutorLinux* executor_;
 
   // X11 graphics context.
   Display* display_;
   Window root_window_;
   int width_;
   int height_;
 
   int test_event_base_;
   int test_error_base_;
 };
 
-EventExecutorLinuxPimpl::EventExecutorLinuxPimpl(EventExecutorLinux* executor)
+EventExecutorLinuxPimpl::EventExecutorLinuxPimpl(EventExecutorLinux* executor,
+                                                 Display* display)
     : executor_(executor),
-      display_(NULL),
+      display_(display),
       root_window_(BadValue),
       width_(0),
       height_(0) {
 }
 
 EventExecutorLinuxPimpl::~EventExecutorLinuxPimpl() {
   DeinitXlib();
 }
 
 bool EventExecutorLinuxPimpl::Init() {
-  // TODO(ajwong): We should specify the display string we are attaching to
-  // in the constructor.
-  display_ = XOpenDisplay(NULL);
   if (!display_) {
     LOG(ERROR) << "Unable to open display";
     return false;
   }
 
   root_window_ = RootWindow(display_, DefaultScreen(display_));
   if (root_window_ == BadValue) {
     LOG(ERROR) << "Unable to get the root window";
     DeinitXlib();
     return false;
@@ skipping 39 matching lines @@
   if (keycode == 0) {
     LOG(WARNING) << "Ignoring undefined keysym: " << keysym
                  << " for key: " << key_event->keycode();
     return;
   }
 
   VLOG(3) << "Got pepper key: " << key_event->keycode()
           << " 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_) {
       // A misbehaving client may send these. Drop events that are out of range.
       // TODO(ajwong): How can we log this sanely? We don't want to DOS the
       // server with a misbehaving client by logging like crazy.
       return;
     }
 
     VLOG(3) << "Moving mouse to " << event->x()
             << "," << event->y();
-    flusher.SignalFlush();
     XTestFakeMotionEvent(display_, DefaultScreen(display_),
                          event->x(), event->y(),
                          CurrentTime);
   }
 
   if (event->has_button() && event->has_button_down()) {
     int button_number = MouseButtonToX11ButtonNumber(event->button());
 
     if (button_number < 0) {
       LOG(WARNING) << "Ignoring unknown button type: "
                    << event->button();
       return;
     }
 
     VLOG(3) << "Button " << event->button()
             << " received, sending down " << button_number;
-    flusher.SignalFlush();
     XTestFakeButtonEvent(display_, button_number, event->button_down(),
                          CurrentTime);
   }
 
   if (event->has_wheel_offset_x() && event->has_wheel_offset_y()) {
     NOTIMPLEMENTED() << "No scroll wheel support yet.";
   }
 }
 
 void EventExecutorLinuxPimpl::DeinitXlib() {
   // TODO(ajwong): We should expose a "close" or "shutdown" method.
   if (display_) {
     if (!XCloseDisplay(display_)) {
       LOG(ERROR) << "Unable to close the Xlib Display.";
     }
     display_ = NULL;
   }
 }
 
 EventExecutorLinux::EventExecutorLinux(
-    MessageLoop* message_loop, Capturer* capturer)
+    MessageLoopForUI* message_loop, Capturer* capturer)
     : message_loop_(message_loop),
       capturer_(capturer),
-      pimpl_(new EventExecutorLinuxPimpl(this)) {
+      pimpl_(new EventExecutorLinuxPimpl(this, message_loop->get_display())) {
   CHECK(pimpl_->Init());
 }
 
 EventExecutorLinux::~EventExecutorLinux() {
 }
 
 void EventExecutorLinux::InjectKeyEvent(const KeyEvent* event, Task* done) {
   if (MessageLoop::current() != message_loop_) {
     message_loop_->PostTask(
         FROM_HERE,
@@ skipping 13 matching lines @@
         FROM_HERE,
         NewRunnableMethod(this, &EventExecutorLinux::InjectMouseEvent,
                           event, done));
     return;
   }
   pimpl_->HandleMouse(event);
   done->Run();
   delete done;
 }
 
-protocol::InputStub* CreateEventExecutor(MessageLoop* message_loop,
+protocol::InputStub* CreateEventExecutor(MessageLoopForUI* message_loop,
                                          Capturer* capturer) {
   return new EventExecutorLinux(message_loop, capturer);
 }
 
 }  // namespace remoting