Aura: don't synthesize mouse moves while holding pointer.

Aura: update last mouse position while holding pointer.

BUG=343166
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251834

[dgozman, 2014-02-12 16:48:51]

Could you please take a look?

[sadrul, 2014-02-13 04:32:17]

You should add a test for this.

[dgozman, 2014-02-13 10:47:38]

Added a test. I'm also dropping held move events since their location may be
wrong.
Please take another look.

[sky, 2014-02-13 17:49:04]

https://codereview.chromium.org/160563002/diff/100001/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://codereview.chromium.org/160563002/diff/100001/ui/aura/root_window.cc#...
ui/aura/root_window.cc:785: return;
If you do this, don't we need to ensure when mouse moves are reenabled an event
is generated? Could you write test coverage of this?

[dgozman, 2014-02-13 17:55:06]

https://codereview.chromium.org/160563002/diff/100001/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://codereview.chromium.org/160563002/diff/100001/ui/aura/root_window.cc#...
ui/aura/root_window.cc:785: return;
On 2014/02/13 17:49:04, sky wrote:
> If you do this, don't we need to ensure when mouse moves are reenabled an
event
> is generated? Could you write test coverage of this?

There is already a drop of held moves on the line 764 above in the similar case.
Since I'm not that familiar with this stuff, I can't decide whether it's the
right thing to do. Do you have a strong opinion here?
The test I added verifies that nothing is sent. I will change it if behavior
changes.

[sky, 2014-02-13 18:56:12]

On 2014/02/13 17:55:06, dgozman wrote:
> https://codereview.chromium.org/160563002/diff/100001/ui/aura/root_window.cc
> File ui/aura/root_window.cc (right):
> 
>
https://codereview.chromium.org/160563002/diff/100001/ui/aura/root_window.cc#...
> ui/aura/root_window.cc:785: return;
> On 2014/02/13 17:49:04, sky wrote:
> > If you do this, don't we need to ensure when mouse moves are reenabled an
> event
> > is generated? Could you write test coverage of this?
> 
> There is already a drop of held moves on the line 764 above in the similar
case.
> Since I'm not that familiar with this stuff, I can't decide whether it's the
> right thing to do. Do you have a strong opinion here?
> The test I added verifies that nothing is sent. I will change it if behavior
> changes.

I think we need to synthesize a mouse move when reenabled. Otherwise the window
may be missing out on a mouse enter.

[dgozman, 2014-02-14 12:54:27]

On 2014/02/13 18:56:12, sky wrote:
> On 2014/02/13 17:55:06, dgozman wrote:
> > https://codereview.chromium.org/160563002/diff/100001/ui/aura/root_window.cc
> > File ui/aura/root_window.cc (right):
> > 
> >
>
https://codereview.chromium.org/160563002/diff/100001/ui/aura/root_window.cc#...
> > ui/aura/root_window.cc:785: return;
> > On 2014/02/13 17:49:04, sky wrote:
> > > If you do this, don't we need to ensure when mouse moves are reenabled an
> > event
> > > is generated? Could you write test coverage of this?
> > 
> > There is already a drop of held moves on the line 764 above in the similar
> case.
> > Since I'm not that familiar with this stuff, I can't decide whether it's the
> > right thing to do. Do you have a strong opinion here?
> > The test I added verifies that nothing is sent. I will change it if behavior
> > changes.
> 
> I think we need to synthesize a mouse move when reenabled. Otherwise the
window
> may be missing out on a mouse enter.

If we are going to synthesize a mouse move, we have to know the correct mouse
position, which is not stored right now while holding.
So, in this new patch I store the last mouse position. Synthetic events do no
harm now - they are being held as usual events.
New test verifies correct event location.

Take another look please.

[sky, 2014-02-14 16:40:05]

https://codereview.chromium.org/160563002/diff/240001/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://codereview.chromium.org/160563002/diff/240001/ui/aura/root_window.cc#...
ui/aura/root_window.cc:833: if (event->type() != ui::ET_MOUSE_CAPTURE_CHANGED)
I think you should be selective here. Meaning use equality rather than
inequality. The way you have it now you'll include ET_TOUCH_MOVED, which
shouldn't effect the mouse location.

[dgozman, 2014-02-17 14:19:59]

PTAL

https://codereview.chromium.org/160563002/diff/240001/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

https://codereview.chromium.org/160563002/diff/240001/ui/aura/root_window.cc#...
ui/aura/root_window.cc:833: if (event->type() != ui::ET_MOUSE_CAPTURE_CHANGED)
On 2014/02/14 16:40:05, sky wrote:
> I think you should be selective here. Meaning use equality rather than
> inequality. The way you have it now you'll include ET_TOUCH_MOVED, which
> shouldn't effect the mouse location.

It's inside PreDispatchMouseEvent. The only mouse event we shouldn't update
location is ET_MOUSE_CAPTURE_CHANGED.
I've added check for synthetic event though.

[sky, 2014-02-18 17:31:12]

LGTM

[dgozman, 2014-02-18 17:40:02]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2014-02-18 17:40:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/160563002/340001

[commit-bot: I haz the power, 2014-02-18 18:23:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/160563002/340001

[commit-bot: I haz the power, 2014-02-18 18:31:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/160563002/340001

[commit-bot: I haz the power, 2014-02-18 20:53:45]

Change committed as 251834