Remove PlatformWheelEvent and use WebMouseWheelEvent instead

Remove PlatformWheelEvent and use WebMouseWheelEvent instead

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

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

BUG=625684
Review-Url: https://codereview.chromium.org/2586133003
Cr-Commit-Position: refs/heads/master@{#441735}
Committed: https://chromium.googlesource.com/chromium/src/+/ac51325cc9bb568a337f38e300cdaa5c1c47099a

[dtapuska, 2016-12-19 15:49:56]

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

[commit-bot: I haz the power, 2016-12-19 15:50:06]

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, 2016-12-19 17:02:01]

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

[commit-bot: I haz the power, 2016-12-19 17:02:02]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[dtapuska, 2016-12-19 17:50:13]

dtapuska@chromium.org changed reviewers:
+ esprehn@chromium.org, majidvp@chromium.org

[dtapuska, 2016-12-20 15:15:02]

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

[commit-bot: I haz the power, 2016-12-20 15:15:12]

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, 2016-12-20 17:20:18]

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

[commit-bot: I haz the power, 2016-12-20 17:20:19]

Dry run: This issue passed the CQ dry run.

[majidvp, 2016-12-20 17:30:15]

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/WheelEvent.h (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/WheelEvent.h:38: enum WheelEventPhase {
Can we get rid of this now?

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/WheelEvent.h:59: static WheelEvent*
create(const WebMouseWheelEvent& platformEvent,
nit: s/platformEvent/nativeEvent/

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/WebMouseWheelEvent.cpp (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/WebMouseWheelEvent.cpp:20: result.deltaY /=
result.m_frameScale;
I think this should also flatten movement{X,Y}.

Perhaps a better way to do this to have WebMouseEvent have its own
flattenTransform
method which we invoke to handle movement and x modifications.

BTW, are we missing a test here for this flatten functionality?

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/InspectorOverlay.cpp (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/InspectorOverlay.cpp:298: WebMouseWheelEvent
scaledEvent = TransformWebMouseWheelEvent(
nit: s/scaledEvent/transformedEvent/

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/PageWidgetDelegate.cpp (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/PageWidgetDelegate.cpp:253: WebMouseWheelEvent
scaledEvent =
nit: ditto

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebPluginContainerImpl.cpp (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebPluginContainerImpl.cpp:766:
roundedIntPoint(m_element->layoutObject()->absoluteToLocal(
can |m_element->layoutObject()| be nil? Should we DCHECK that?

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebPluginContainerImpl.cpp:770: translatedEvent.y
= localPoint.y();
he fact that we are doing a flatten but then override the coordinate with the
result of
|absoluteToLocal| is a bit confusing.

I suppose the reason it is done this way is that there may be intermmediate
transforms that should
be taken into account when we translate to plugin space. But then, why are not
these apply to
movement and delta as well?

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp (left):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp:1082:
TEST(WebInputEventConversionTest, WebMouseWheelEventBuilder) {
I think we should replace this test with something that covers the 
translation that happens at WebPluginContainer boundary.

IMHO, the old test was not actually testing the most important bit
which is the coordinate translation which as I pointed out elsewhere
I think is buggy.

[dtapuska, 2017-01-03 20:18:58]

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

[dtapuska, 2017-01-03 20:19:14]

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/WheelEvent.h (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/WheelEvent.h:38: enum WheelEventPhase {
On 2016/12/20 17:30:14, majidvp wrote:
> Can we get rid of this now?

Done.

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/WheelEvent.h:59: static WheelEvent*
create(const WebMouseWheelEvent& platformEvent,
On 2016/12/20 17:30:14, majidvp wrote:
> nit: s/platformEvent/nativeEvent/

Done.

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/WebMouseWheelEvent.cpp (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/WebMouseWheelEvent.cpp:20: result.deltaY /=
result.m_frameScale;
On 2016/12/20 17:30:14, majidvp wrote:
> I think this should also flatten movement{X,Y}.
> 
> Perhaps a better way to do this to have WebMouseEvent have its own
> flattenTransform
> method which we invoke to handle movement and x modifications.
> 
> BTW, are we missing a test here for this flatten functionality?

I've added a FlattenTransformSelf on mouse event that flattens the object
itself.

It should be tested in the WebInputEventConversionTest.cpp

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/InspectorOverlay.cpp (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/InspectorOverlay.cpp:298: WebMouseWheelEvent
scaledEvent = TransformWebMouseWheelEvent(
On 2016/12/20 17:30:15, majidvp wrote:
> nit: s/scaledEvent/transformedEvent/

Done.

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/PageWidgetDelegate.cpp (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/PageWidgetDelegate.cpp:253: WebMouseWheelEvent
scaledEvent =
On 2016/12/20 17:30:15, majidvp wrote:
> nit: ditto

Done.

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebPluginContainerImpl.cpp (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebPluginContainerImpl.cpp:766:
roundedIntPoint(m_element->layoutObject()->absoluteToLocal(
On 2016/12/20 17:30:15, majidvp wrote:
> can |m_element->layoutObject()| be nil? Should we DCHECK that?

Other places use it unconditionally. I can add a DCHECK if you feel strongly.

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebPluginContainerImpl.cpp:770: translatedEvent.y
= localPoint.y();
On 2016/12/20 17:30:15, majidvp wrote:
> he fact that we are doing a flatten but then override the coordinate with the
> result of
> |absoluteToLocal| is a bit confusing.
> 
> I suppose the reason it is done this way is that there may be intermmediate
> transforms that should
> be taken into account when we translate to plugin space. But then, why are not
> these apply to
> movement and delta as well?

Ya I don't know what the best answer here is. But this is logically the same as
it was before.

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp (left):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebInputEventConversionTest.cpp:1082:
TEST(WebInputEventConversionTest, WebMouseWheelEventBuilder) {
On 2016/12/20 17:30:15, majidvp wrote:
> I think we should replace this test with something that covers the 
> translation that happens at WebPluginContainer boundary.
> 
> IMHO, the old test was not actually testing the most important bit
> which is the coordinate translation which as I pointed out elsewhere
> I think is buggy.

Done; added to WebPluginContainerTest.cpp

[commit-bot: I haz the power, 2017-01-03 20:19:23]

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-03 22:57:53]

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

[commit-bot: I haz the power, 2017-01-03 22:57:54]

Dry run: This issue passed the CQ dry run.

[majidvp, 2017-01-04 17:32:46]

lgtm.

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebPluginContainerImpl.cpp (right):

https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebPluginContainerImpl.cpp:770: translatedEvent.y
= localPoint.y();

> Ya I don't know what the best answer here is. But this is logically the same
as
> it was before.

Fair enough. I filed https://crbug.com/678291 to track this.

[dtapuska, 2017-01-04 21:58:49]

On 2017/01/04 17:32:46, majidvp wrote:
> lgtm.
> 
>
https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/web/WebPluginContainerImpl.cpp (right):
> 
>
https://codereview.chromium.org/2586133003/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/web/WebPluginContainerImpl.cpp:770:
translatedEvent.y
> = localPoint.y();
> 
> > Ya I don't know what the best answer here is. But this is logically the same
> as
> > it was before.
> 
> Fair enough. I filed https://crbug.com/678291 to track this.

esprehn@ can you take a look?

[esprehn, 2017-01-04 22:00:53]

lgtm

https://codereview.chromium.org/2586133003/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebMouseWheelEvent.h (right):

https://codereview.chromium.org/2586133003/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebMouseWheelEvent.h:97: bool cancelable()
const { return dispatchType == Blocking; }
isCancelable() ?

[dtapuska, 2017-01-05 14:16:18]

https://codereview.chromium.org/2586133003/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebMouseWheelEvent.h (right):

https://codereview.chromium.org/2586133003/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebMouseWheelEvent.h:97: bool cancelable()
const { return dispatchType == Blocking; }
On 2017/01/04 22:00:53, esprehn wrote:
> isCancelable() ?

Done.

[dtapuska, 2017-01-05 14:25:12]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2017-01-05 14:25:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, majidvp@chromium.org
Link to the patchset: https://codereview.chromium.org/2586133003/#ps60001
(title: "Adjust function name")

[commit-bot: I haz the power, 2017-01-05 14:25:19]

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-05 19:39:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1483626312652050,
"parent_rev": "2805edaba1b852b6ce489a8f88bd8e90e51b73d2", "commit_rev":
"ac51325cc9bb568a337f38e300cdaa5c1c47099a"}

[commit-bot: I haz the power, 2017-01-05 19:40:19]

Description was changed from

==========
Remove PlatformWheelEvent and use WebMouseWheelEvent instead

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

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

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

to

==========
Remove PlatformWheelEvent and use WebMouseWheelEvent instead

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

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

BUG=625684

Review-Url: https://codereview.chromium.org/2586133003
Cr-Commit-Position: refs/heads/master@{#441735}
Committed:
https://chromium.googlesource.com/chromium/src/+/ac51325cc9bb568a337f38e300cd...
==========

[commit-bot: I haz the power, 2017-01-05 19:40:20]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/ac51325cc9bb568a337f38e300cd...