Suppress 0x0 MotionNotify events between button presses and releases.

Suppress 0x0 MotionNotify events between button presses and releases.

BUG=138075
TEST=Connect to a Linux host, run Gnome-terminal and click in the terminal to the right of a line of text. No part of the terminal's contents should become selected by the action, unless the mouse is actually moved between the press and release.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=147892

[Wez, 2012-07-23 16:33:49]

PTAL

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

lgtm

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
File remoting/host/event_executor_linux.cc (right):

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
remoting/host/event_executor_linux.cc:400: latest_mouse_position_ =
SkIPoint::Make(event.x(), event.y());
Nit: It might be cleaner to declare a variable, new_mouse_position, rather than
calling Make twice.

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

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
File remoting/host/event_executor_linux.cc (right):

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
remoting/host/event_executor_linux.cc:67: SkIPoint latest_mouse_position_;
Drive-by: Do you need to initialise this member?

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
remoting/host/event_executor_linux.cc:395: if (event.has_button() &&
event.has_button_down() && !event.button_down()) {
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]

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
File remoting/host/event_executor_linux.cc (right):

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
remoting/host/event_executor_linux.cc:67: SkIPoint latest_mouse_position_;
On 2012/07/23 16:45:01, simonmorris wrote:
> Drive-by: Do you need to initialise this member?

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

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
remoting/host/event_executor_linux.cc:67: SkIPoint latest_mouse_position_;
On 2012/07/23 16:45:01, simonmorris wrote:
> Drive-by: Do you need to initialise this member?

Done.

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
remoting/host/event_executor_linux.cc:395: if (event.has_button() &&
event.has_button_down() && !event.button_down()) {
On 2012/07/23 16:45:01, simonmorris wrote:
> Drive-by: Is there any need for these button tests? Should we ever inject a
> motion event if the mouse position hasn't changed?

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.

http://codereview.chromium.org/10795065/diff/1/remoting/host/event_executor_l...
remoting/host/event_executor_linux.cc:400: latest_mouse_position_ =
SkIPoint::Make(event.x(), event.y());
On 2012/07/23 16:44:16, Jamie wrote:
> Nit: It might be cleaner to declare a variable, new_mouse_position, rather
than
> calling Make twice.

Done.

[commit-bot: I haz the power, 2012-07-23 17:31:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wez@chromium.org/10795065/6001

[commit-bot: I haz the power, 2012-07-23 18:52:53]

Change committed as 147892