Do not set the last mouse location from synthesized event

Do not set the last mouse location from synthesized event.
 The last mouse location should be the updated only by real event because it should point to the real mouse point and synthesized enter/leave may not contain the valid point.

minor: remove unnecessary cast.

BUG=None
TEST=TBD.

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

[oshima, 2014-02-11 19:43:38]

I'll add test if you're ok with this change.

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

https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc#n...
ui/aura/root_window.cc:405: }
This was necessary because EnterOrExit may cause early exit in 
PreDispatchMouseEvent, which means PreDispatchLocatedEvent is not called. Let me
know if there is a better way/place to fix this.

[sadrul, 2014-02-11 21:26:20]

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

https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc#n...
ui/aura/root_window.cc:405: }
On 2014/02/11 19:43:38, oshima wrote:
> This was necessary because EnterOrExit may cause early exit in 
> PreDispatchMouseEvent, which means PreDispatchLocatedEvent is not called. Let
me
> know if there is a better way/place to fix this.

Can you explain this a bit more? The early returns in PreDispatchMouseEvent for
EnterOrExit: are these only for the cases where the |target| is destroyed
(because I think the rest of the early returns are related to the root-window
itself being destroyed)? If so, can we do this in there instead?

[oshima, 2014-02-11 22:01:12]

On 2014/02/11 21:26:20, sadrul wrote:
> https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc
> File ui/aura/root_window.cc (right):
> 
>
https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc#n...
> ui/aura/root_window.cc:405: }
> On 2014/02/11 19:43:38, oshima wrote:
> > This was necessary because EnterOrExit may cause early exit in 
> > PreDispatchMouseEvent, which means PreDispatchLocatedEvent is not called.
Let
> me
> > know if there is a better way/place to fix this.
> 
> Can you explain this a bit more? The early returns in PreDispatchMouseEvent
for
> EnterOrExit: are these only for the cases where the |target| is destroyed
> (because I think the rest of the early returns are related to the root-window
> itself being destroyed)? If so, can we do this in there instead?

I looked into a bit more and here are a couple of things I found:

1) DispatchMouseEnterOrExit is dispatching the event without updating
last_mouse_location, which means
 the code that handles enter/exit may see stale location.

2) Even if the target is destroyed, the last mouse location has to be updated
because the next task (such as key event, events generated by app) should see
the correct mouse location.

And for the reason 1), I think we should update the location in this method, may
be after return DispatchDetails() ?

[oshima, 2014-02-11 22:07:08]

On 2014/02/11 22:01:12, oshima wrote:
> On 2014/02/11 21:26:20, sadrul wrote:
> > https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc
> > File ui/aura/root_window.cc (right):
> > 
> >
>
https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc#n...
> > ui/aura/root_window.cc:405: }
> > On 2014/02/11 19:43:38, oshima wrote:
> > > This was necessary because EnterOrExit may cause early exit in 
> > > PreDispatchMouseEvent, which means PreDispatchLocatedEvent is not called.
> Let
> > me
> > > know if there is a better way/place to fix this.
> > 
> > Can you explain this a bit more? The early returns in PreDispatchMouseEvent
> for
> > EnterOrExit: are these only for the cases where the |target| is destroyed
> > (because I think the rest of the early returns are related to the
root-window
> > itself being destroyed)? If so, can we do this in there instead?
> 
> I looked into a bit more and here are a couple of things I found:
> 
> 1) DispatchMouseEnterOrExit is dispatching the event without updating
> last_mouse_location, which means
>  the code that handles enter/exit may see stale location.

Forgot to mention. This behavior is with the change not to update the location
with SYNTHESIZED event.
In other words, this was working because we're updating the location using this
fake enter/exit event.

> 2) Even if the target is destroyed, the last mouse location has to be updated
> because the next task (such as key event, events generated by app) should see
> the correct mouse location.
> 
> And for the reason 1), I think we should update the location in this method,
may
> be after return DispatchDetails() ?

[sadrul, 2014-02-13 04:38:51]

On 2014/02/11 22:01:12, oshima wrote:
> On 2014/02/11 21:26:20, sadrul wrote:
> > https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc
> > File ui/aura/root_window.cc (right):
> > 
> >
>
https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc#n...
> > ui/aura/root_window.cc:405: }
> > On 2014/02/11 19:43:38, oshima wrote:
> > > This was necessary because EnterOrExit may cause early exit in 
> > > PreDispatchMouseEvent, which means PreDispatchLocatedEvent is not called.
> Let
> > me
> > > know if there is a better way/place to fix this.
> > 
> > Can you explain this a bit more? The early returns in PreDispatchMouseEvent
> for
> > EnterOrExit: are these only for the cases where the |target| is destroyed
> > (because I think the rest of the early returns are related to the
root-window
> > itself being destroyed)? If so, can we do this in there instead?
> 
> I looked into a bit more and here are a couple of things I found:
> 
> 1) DispatchMouseEnterOrExit is dispatching the event without updating
> last_mouse_location, which means
>  the code that handles enter/exit may see stale location.
> 
> 2) Even if the target is destroyed, the last mouse location has to be updated
> because the next task (such as key event, events generated by app) should see
> the correct mouse location.
> 
> And for the reason 1), I think we should update the location in this method,
may
> be after return DispatchDetails() ?

Sounds reasonable.

[sadrul, 2014-02-13 04:38:59]

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

https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc#n...
ui/aura/root_window.cc:819: if (!(event->flags() & ui::EF_IS_SYNTHESIZED)) {
Merge the two if's?

[oshima, 2014-02-13 15:25:31]

added test. PTAL.

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

https://codereview.chromium.org/143303006/diff/20001/ui/aura/root_window.cc#n...
ui/aura/root_window.cc:819: if (!(event->flags() & ui::EF_IS_SYNTHESIZED)) {
On 2014/02/13 04:38:59, sadrul wrote:
> Merge the two if's?

Done.

[sadrul, 2014-02-13 15:30:26]

lgtm

https://codereview.chromium.org/143303006/diff/140001/ui/aura/root_window_uni...
File ui/aura/root_window_unittest.cc (right):

https://codereview.chromium.org/143303006/diff/140001/ui/aura/root_window_uni...
ui/aura/root_window_unittest.cc:1759: // Synthesized event should not update the
moust location.
*mouse

[oshima, 2014-02-13 16:48:17]

https://codereview.chromium.org/143303006/diff/140001/ui/aura/root_window_uni...
File ui/aura/root_window_unittest.cc (right):

https://codereview.chromium.org/143303006/diff/140001/ui/aura/root_window_uni...
ui/aura/root_window_unittest.cc:1759: // Synthesized event should not update the
moust location.
On 2014/02/13 15:30:27, sadrul wrote:
> *mouse

Done.

[oshima, 2014-02-13 16:48:22]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-02-13 16:48:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/143303006/180001

[commit-bot: I haz the power, 2014-02-13 19:48:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/143303006/180001

[commit-bot: I haz the power, 2014-02-14 13:44:49]

Change committed as 251291