Suppress 0x0 MotionNotify events between button presses and releases.

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 4e50f9fd8693b60c1cc20f5112577279e8734046..b6175a9c3b04be3226fe63e0e7367bb34b439664 100644
--- a/remoting/host/event_executor_linux.cc
+++ b/remoting/host/event_executor_linux.cc
@@ -18,6 +18,7 @@
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
 #include "remoting/proto/internal.pb.h"
+#include "third_party/skia/include/core/SkPoint.h"
 
 namespace remoting {
 
@@ -63,6 +64,7 @@ class EventExecutorLinux : public EventExecutor {
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
   std::set<int> pressed_keys_;
+  SkIPoint latest_mouse_position_;

[simonmorris, 2012/07/23 16:45:01]

Drive-by: Do you need to initialise this member?

[Wez, 2012/07/23 17:30:16]

Yes! Good spot. I wonder how the Skia folks would feel about having these types
initialized to something...

[Wez, 2012/07/23 17:30:16]

Done.

 
   // X11 graphics context.
   Display* display_;
@@ -386,11 +388,24 @@ void EventExecutorLinux::InjectMouseEvent(const MouseEvent& event) {
   }
 
   if (event.has_x() && event.has_y()) {
-    VLOG(3) << "Moving mouse to " << event.x()
-            << "," << event.y();
-    XTestFakeMotionEvent(display_, DefaultScreen(display_),
-                         event.x(), event.y(),
-                         CurrentTime);
+    // Injecting a motion event immediately before a button release results in
+    // a MotionNotify even if the mouse position hasn't changed, which confuses
+    // apps which assume MotionNotify implies movement. See crbug.com/138075.
+    bool inject_motion = true;
+    if (event.has_button() && event.has_button_down() && !event.button_down()) {

[simonmorris, 2012/07/23 16:45:01]

Drive-by: Is there any need for these button tests? Should we ever inject a
motion event if the mouse position hasn't changed?

[Wez, 2012/07/23 17:30:16]

Yes; if a local user moves the cursor and after waiting for the block on remote
input to expire, the remote user clicks, then the position in
last_mouse_position_ will cause us not to move the mouse to the right position
before clicking.

+      if (SkIPoint::Make(event.x(), event.y()) == latest_mouse_position_)
+        inject_motion = false;
+    }
+
+    latest_mouse_position_ = SkIPoint::Make(event.x(), event.y());

[Jamie, 2012/07/23 16:44:16]

Nit: It might be cleaner to declare a variable, new_mouse_position, rather than
calling Make twice.

[Wez, 2012/07/23 17:30:16]

Done.

+
+    if (inject_motion) {
+      VLOG(3) << "Moving mouse to " << event.x()
+              << "," << event.y();
+      XTestFakeMotionEvent(display_, DefaultScreen(display_),
+                           event.x(), event.y(),
+                           CurrentTime);
+    }
   }
 
   if (event.has_button() && event.has_button_down()) {