Remove PlatformMouseEvent and use WebMouseEvent instead

Remove PlatformMouseEvent and use WebMouseEvent instead

Perform similar cleanup as GestureEvents used in change:
https://codereview.chromium.org/2539283002/
https://codereview.chromium.org/2586133003/
https://codereview.chromium.org/2646163002/

Cleanup proposal: https://docs.google.com/document/d/1s4Lfy22CNU1OZ5Rec6Oano_5BvIhdK6uFVsVe7FphKI/edit

BUG=625684
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2650403006
Cr-Commit-Position: refs/heads/master@{#446853}
Committed: https://chromium.googlesource.com/chromium/src/+/6a0ddfbef3bfb61f2de91026ac1e749172917210

[dtapuska, 2017-01-26 20:58:19]

Description was changed from

==========
Remove PlatformMouseEvent and use WebMouseEvent instead

Perform similar cleanup as GestureEvents used in change:
https://codereview.chromium.org/2539283002/
https://codereview.chromium.org/2586133003/
https://codereview.chromium.org/2646163002/

Cleanup proposal:
https://docs.google.com/document/d/1s4Lfy22CNU1OZ5Rec6Oano_5BvIhdK6uFVsVe7Fph...

BUG=625684
==========

to

==========
Remove PlatformMouseEvent and use WebMouseEvent instead

Perform similar cleanup as GestureEvents used in change:
https://codereview.chromium.org/2539283002/
https://codereview.chromium.org/2586133003/
https://codereview.chromium.org/2646163002/

Cleanup proposal:
https://docs.google.com/document/d/1s4Lfy22CNU1OZ5Rec6Oano_5BvIhdK6uFVsVe7Fph...

BUG=625684
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[dtapuska, 2017-01-26 20:58:27]

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

[commit-bot: I haz the power, 2017-01-26 20:58:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 23:26:46]

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

[commit-bot: I haz the power, 2017-01-26 23:26:46]

Dry run: This issue passed the CQ dry run.

[dtapuska, 2017-01-27 14:31:01]

dtapuska@chromium.org changed reviewers:
+ bokan@chromium.org, mustaq@chromium.org

[mustaq, 2017-01-27 16:53:45]

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Node.cpp:2154: IntPoint locationInContents =
flooredIntPoint(
Let's floor the coord first before transforming, to preserve old behavior for
corner cases where flooring before/after gives different results.

Moreover, mouseEvent positions are truncated to ints in many other places here.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/events/MouseEvent.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/MouseEvent.cpp:195:
PlatformEvent::Modifiers modifiers,
Please remove platform Modifiers from MouseEvent interface completely.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/MouseEvent.cpp:290: unsigned short
MouseEvent::platformModifiersToButtons(unsigned modifiers) {
Also s/platformModifiersToButtons()/webModifiersToButtons()/.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/events/MouseEvent.h (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/MouseEvent.h:105: void setRegion(const
String& region) { m_region = region; }
Does anybody use setRegion? I didn't find any. If this is the case, please nuke
it.

[bokan, 2017-01-27 16:57:18]

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1901: // coordinates
instead of root frame coordinates.
Why? The sendContextMenuEvent expects it in root frame coordinates and it's
generated from within Blink. Is there a reason this is a problem?

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/MouseEventManager.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/MouseEventManager.cpp:310: //
TODO(dtapuska): Update m_lastKnowMousePosition to be viewport coordinates.
Ditto here

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/PopupMenuImpl.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/PopupMenuImpl.cpp:449: // TODO(dtapuska): Why is
this event positionless?
It looks like this is called from popup HTML content, e.g. when a DateTime
picker selects an item using the enter key. It seems like we dispatch these
mouse events for compatibility but they don't actually come from a mouse click
so there's no position to set. I suppose we could pass the "last known mouse
position" but that doesn't seem easily accessible from here.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp:163: if
(mouseEvent->nativeEvent()) {
So we'll only forward non-synthetic events to remote frames right? Was this the
behaviour before?

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp:166: WebFloatPoint
absoluteRootFrameLocation =
You can just replace this with transformedEvent.positionInRootFrame()

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:152: //
TODO(dtapuska): Perhaps the event should be constructed correctly?
Please file bugs for these and put the bug # in the TODO.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:202: // This is the
slimplified code path once we OOPIF ships
nit: "we OOPIF" -> "OOPIF"

I think this comment should be reworded - I'm not sure what it's trying to say.
Do we have to fix this once OOPIF ships or is this a hack to keep OOPIF working?

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/WebInputEvent.h (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebInputEvent.h:192: // events will this.
nit: "will set this"

[dtapuska, 2017-01-27 20:59:28]

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

[dtapuska, 2017-01-27 21:00:00]

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Node.cpp:2154: IntPoint locationInContents =
flooredIntPoint(
On 2017/01/27 16:53:44, mustaq wrote:
> Let's floor the coord first before transforming, to preserve old behavior for
> corner cases where flooring before/after gives different results.
> 
> Moreover, mouseEvent positions are truncated to ints in many other places
here.

Done.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/events/MouseEvent.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/MouseEvent.cpp:195:
PlatformEvent::Modifiers modifiers,
On 2017/01/27 16:53:44, mustaq wrote:
> Please remove platform Modifiers from MouseEvent interface completely.

I will do this in here: https://codereview.chromium.org/2655873003/

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/MouseEvent.cpp:290: unsigned short
MouseEvent::platformModifiersToButtons(unsigned modifiers) {
On 2017/01/27 16:53:44, mustaq wrote:
> Also s/platformModifiersToButtons()/webModifiersToButtons()/.

See dependent change as well.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/events/MouseEvent.h (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/MouseEvent.h:105: void setRegion(const
String& region) { m_region = region; }
On 2017/01/27 16:53:44, mustaq wrote:
> Does anybody use setRegion? I didn't find any. If this is the case, please
nuke
> it.

Done.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1901: // coordinates
instead of root frame coordinates.
On 2017/01/27 16:57:17, bokan wrote:
> Why? The sendContextMenuEvent expects it in root frame coordinates and it's
> generated from within Blink. Is there a reason this is a problem?

So that it is consistent for all events. Reading the position should really be
the true viewport position.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/MouseEventManager.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/MouseEventManager.cpp:310: //
TODO(dtapuska): Update m_lastKnowMousePosition to be viewport coordinates.
On 2017/01/27 16:57:17, bokan wrote:
> Ditto here

same reasoning.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/PopupMenuImpl.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/PopupMenuImpl.cpp:449: // TODO(dtapuska): Why is
this event positionless?
On 2017/01/27 16:57:17, bokan wrote:
> It looks like this is called from popup HTML content, e.g. when a DateTime
> picker selects an item using the enter key. It seems like we dispatch these
> mouse events for compatibility but they don't actually come from a mouse click
> so there's no position to set. I suppose we could pass the "last known mouse
> position" but that doesn't seem easily accessible from here.

This should probably be generated from the element's position no? kind of like
how the context menu is generated...

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp:163: if
(mouseEvent->nativeEvent()) {
On 2017/01/27 16:57:17, bokan wrote:
> So we'll only forward non-synthetic events to remote frames right? Was this
the
> behaviour before?

Reverted back to original code.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/RemoteFrameClientImpl.cpp:166: WebFloatPoint
absoluteRootFrameLocation =
On 2017/01/27 16:57:17, bokan wrote:
> You can just replace this with transformedEvent.positionInRootFrame()

Removed.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebInputEventConversion.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:152: //
TODO(dtapuska): Perhaps the event should be constructed correctly?
On 2017/01/27 16:57:17, bokan wrote:
> Please file bugs for these and put the bug # in the TODO.

Done.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebInputEventConversion.cpp:202: // This is the
slimplified code path once we OOPIF ships
On 2017/01/27 16:57:17, bokan wrote:
> nit: "we OOPIF" -> "OOPIF"
> 
> I think this comment should be reworded - I'm not sure what it's trying to
say.
> Do we have to fix this once OOPIF ships or is this a hack to keep OOPIF
working?

Done.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/WebInputEvent.h (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebInputEvent.h:192: // events will this.
On 2017/01/27 16:57:17, bokan wrote:
> nit: "will set this"

Done.

[commit-bot: I haz the power, 2017-01-27 21:00:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bokan, 2017-01-27 21:08:47]

Thanks, lgtm from me

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandler.cpp:1901: // coordinates
instead of root frame coordinates.
On 2017/01/27 21:00:00, dtapuska wrote:
> On 2017/01/27 16:57:17, bokan wrote:
> > Why? The sendContextMenuEvent expects it in root frame coordinates and it's
> > generated from within Blink. Is there a reason this is a problem?
> 
> So that it is consistent for all events. Reading the position should really be
> the true viewport position.

Right, forgot that the |position| is really in viewport coords.

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/PopupMenuImpl.cpp (right):

https://codereview.chromium.org/2650403006/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/PopupMenuImpl.cpp:449: // TODO(dtapuska): Why is
this event positionless?
On 2017/01/27 21:00:00, dtapuska wrote:
> On 2017/01/27 16:57:17, bokan wrote:
> > It looks like this is called from popup HTML content, e.g. when a DateTime
> > picker selects an item using the enter key. It seems like we dispatch these
> > mouse events for compatibility but they don't actually come from a mouse
click
> > so there's no position to set. I suppose we could pass the "last known mouse
> > position" but that doesn't seem easily accessible from here.
> 
> This should probably be generated from the element's position no? kind of like
> how the context menu is generated...

Hmm, yeah, that makes sense.

[mustaq, 2017-01-27 21:10:47]

lgtm

[dtapuska, 2017-01-27 21:14:19]

dtapuska@chromium.org changed reviewers:
+ rbyers@chromium.org

[dtapuska, 2017-01-27 21:14:20]

rbyers@chromium.org: Please review changes in 

*/platform/*

[Rick Byers, 2017-01-27 21:22:50]

platform/ and public/platform LGTM

[commit-bot: I haz the power, 2017-01-28 00:23:08]

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

[commit-bot: I haz the power, 2017-01-28 00:23:09]

Dry run: This issue passed the CQ dry run.

[dtapuska, 2017-01-28 00:28:46]

The CQ bit was checked by dtapuska@chromium.org

[commit-bot: I haz the power, 2017-01-28 00:29:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-28 00:39:37]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485563326936880,
"parent_rev": "14bb3dc50a132c22dae437c7ee7cccc18948f718", "commit_rev":
"6a0ddfbef3bfb61f2de91026ac1e749172917210"}

[commit-bot: I haz the power, 2017-01-28 00:40:07]

Description was changed from

==========
Remove PlatformMouseEvent and use WebMouseEvent instead

Perform similar cleanup as GestureEvents used in change:
https://codereview.chromium.org/2539283002/
https://codereview.chromium.org/2586133003/
https://codereview.chromium.org/2646163002/

Cleanup proposal:
https://docs.google.com/document/d/1s4Lfy22CNU1OZ5Rec6Oano_5BvIhdK6uFVsVe7Fph...

BUG=625684
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Remove PlatformMouseEvent and use WebMouseEvent instead

Perform similar cleanup as GestureEvents used in change:
https://codereview.chromium.org/2539283002/
https://codereview.chromium.org/2586133003/
https://codereview.chromium.org/2646163002/

Cleanup proposal:
https://docs.google.com/document/d/1s4Lfy22CNU1OZ5Rec6Oano_5BvIhdK6uFVsVe7Fph...

BUG=625684
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2650403006
Cr-Commit-Position: refs/heads/master@{#446853}
Committed:
https://chromium.googlesource.com/chromium/src/+/6a0ddfbef3bfb61f2de91026ac1e...
==========

[commit-bot: I haz the power, 2017-01-28 00:40:09]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/6a0ddfbef3bfb61f2de91026ac1e...