Keep non-key event handling in-house for NativeWidgetMus

Keep non-key event handling in-house for NativeWidgetMus

R=sadrul@chromium.org
BUG=609555

[Mark Dittmer, 2016-05-24 19:10:24]

Keep non-key event handling in-house for NativeWidgetMus

[sadrul, 2016-05-24 21:54:49]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-24 21:55:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2013503002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2013503002/20001

[sadrul, 2016-05-24 22:40:58]

Can you add a test (or update NativeWidgetMusTest.WidgetReceivesEvent) to verify
that the Widget/View receives the mouse/gesture events in the right coordinate
space?

[commit-bot: I haz the power, 2016-05-24 23:15:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-24 23:15:47]

Dry run: This issue passed the CQ dry run.

[Mark Dittmer, 2016-05-26 14:10:54]

On 2016/05/24 22:40:58, sadrul wrote:
> Can you add a test (or update NativeWidgetMusTest.WidgetReceivesEvent) to
verify
> that the Widget/View receives the mouse/gesture events in the right coordinate
> space?

Sure. We'll need to quickly review a precise definition of "the right coordinate
space".

[Mark Dittmer, 2016-05-26 19:14:28]

Add test for mouse input info agreement between widget and view

[Mark Dittmer, 2016-05-26 19:16:02]

sadrul@chromium.org: PTAL. Is this what you meant as a view/widget coordinate
space test?

[sadrul, 2016-05-26 22:00:50]

https://codereview.chromium.org/2013503002/diff/40001/ui/views/mus/native_wid...
File ui/views/mus/native_widget_mus_unittest.cc (right):

https://codereview.chromium.org/2013503002/diff/40001/ui/views/mus/native_wid...
ui/views/mus/native_widget_mus_unittest.cc:512: EXPECT_EQ(ui_pointer_details,
mus_pointer_details);
PointerDetails do not contain event-location, which is the more interesting
thing to test here.

Since |content| is positioned at (10, 20), and the event is at (50, 50) on the
Widget, therefore |content| should see the event at (40, 30).

[Mark Dittmer, 2016-05-27 12:37:32]

Q'n regarding coordinates. sadrul@chromium.org: PTAL.

https://codereview.chromium.org/2013503002/diff/40001/ui/views/mus/native_wid...
File ui/views/mus/native_widget_mus_unittest.cc (right):

https://codereview.chromium.org/2013503002/diff/40001/ui/views/mus/native_wid...
ui/views/mus/native_widget_mus_unittest.cc:512: EXPECT_EQ(ui_pointer_details,
mus_pointer_details);
On 2016/05/26 22:00:50, sadrul wrote:
> PointerDetails do not contain event-location, which is the more interesting
> thing to test here.
> 
> Since |content| is positioned at (10, 20), and the event is at (50, 50) on the
> Widget, therefore |content| should see the event at (40, 30).

Wait. So does that mean that the view and widget _don't_ see the same
coordinates?

[Mark Dittmer, 2016-05-27 13:45:47]

Mouse event unit test: Check event location, not pointer details

[Mark Dittmer, 2016-05-27 13:46:19]

Candidate location-based unit test. sadrul@chromium.org: PTAL.

https://codereview.chromium.org/2013503002/diff/40001/ui/views/mus/native_wid...
File ui/views/mus/native_widget_mus_unittest.cc (right):

https://codereview.chromium.org/2013503002/diff/40001/ui/views/mus/native_wid...
ui/views/mus/native_widget_mus_unittest.cc:512: EXPECT_EQ(ui_pointer_details,
mus_pointer_details);
On 2016/05/27 12:37:32, Mark Dittmer wrote:
> On 2016/05/26 22:00:50, sadrul wrote:
> > PointerDetails do not contain event-location, which is the more interesting
> > thing to test here.
> > 
> > Since |content| is positioned at (10, 20), and the event is at (50, 50) on
the
> > Widget, therefore |content| should see the event at (40, 30).
> 
> Wait. So does that mean that the view and widget _don't_ see the same
> coordinates?

Done.

[Mark Dittmer, 2016-05-27 15:00:29]

rebase

[sadrul, 2016-05-27 18:11:16]

https://codereview.chromium.org/2013503002/diff/80001/ui/views/mus/native_wid...
File ui/views/mus/native_widget_mus_unittest.cc (right):

https://codereview.chromium.org/2013503002/diff/80001/ui/views/mus/native_wid...
ui/views/mus/native_widget_mus_unittest.cc:520:
EXPECT_EQ(gfx::Point(mouse_location()), mus_event_location);
I think this test could be simpler:

  . Define an EventView that keeps track of the events it received and their
locations.
  . Add that EventView to the Widget.
  . Use WindowTreeClientImplPrivate to drop the event on the mus::Window.
  . Get the event-location from EventView, verify that it is correct.

I don't think we need either UIMouseEventHandler, or MusMouseEventHandler.

[Mark Dittmer, 2016-05-30 15:07:23]

Simplify unit test; just test listening view coordinates

[Mark Dittmer, 2016-05-30 16:01:21]

rebase

[Mark Dittmer, 2016-05-30 17:03:24]

Revert mus::Window API change; no longer necessary

[Mark Dittmer, 2016-05-30 17:03:59]

https://codereview.chromium.org/2013503002/diff/80001/ui/views/mus/native_wid...
File ui/views/mus/native_widget_mus_unittest.cc (right):

https://codereview.chromium.org/2013503002/diff/80001/ui/views/mus/native_wid...
ui/views/mus/native_widget_mus_unittest.cc:520:
EXPECT_EQ(gfx::Point(mouse_location()), mus_event_location);
On 2016/05/27 18:11:16, sadrul wrote:
> I think this test could be simpler:
> 
>   . Define an EventView that keeps track of the events it received and their
> locations.
>   . Add that EventView to the Widget.
>   . Use WindowTreeClientImplPrivate to drop the event on the mus::Window.
>   . Get the event-location from EventView, verify that it is correct.
> 
> I don't think we need either UIMouseEventHandler, or MusMouseEventHandler.

Done.

[Mark Dittmer, 2016-05-31 17:19:43]

Use aura::EnvInputStateController for mouse events