Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

This CL fixes both (1) the renderer crash originally fixed by
https://codereview.chromium.org/2165043002, and (2) reverts the part
of that CL which prevents ChromeOS from having zero-velocity
GestureFlingStarts, which caused a regression in two-finger swipe
navigation for touchpad. It also moves the DCHECK from the original CL
to a new location which would have prevented the renderer crashes from
being introduced in the first place.

This CL doesn't include a test, though the test proposed by
khmel@ in https://codereview.chromium.org/2168693003 presumably will
land shortly after this CL.

BUG=627227
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/d51673024bb1990807d86e8acfbf7759748fbae3
Cr-Commit-Position: refs/heads/master@{#407275}

[wjmaclean, 2016-07-22 00:20:00]

Description was changed from

==========
Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

This CL fixes both (1) the renderer crash originally fixed by
https://codereview.chromium.org/2165043002, and (2) reverts the part
of that CL which prevents ChromeOS from having zero-velocity
GestureFlingStarts, which caused a regression in two-finger swipe
navigation for touchpad. It also moves the DCHECK from the original CL
to a new location which would have prevented the renderer crashes from
being introduced in the first place.

This CL doesn't include a test, though the test proposed by
khmel@ in https://codereview.chromium.org/2168693003 presumably will
land shortly after this CL.

BUG=627227
==========

to

==========
Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

This CL fixes both (1) the renderer crash originally fixed by
https://codereview.chromium.org/2165043002, and (2) reverts the part
of that CL which prevents ChromeOS from having zero-velocity
GestureFlingStarts, which caused a regression in two-finger swipe
navigation for touchpad. It also moves the DCHECK from the original CL
to a new location which would have prevented the renderer crashes from
being introduced in the first place.

This CL doesn't include a test, though the test proposed by
khmel@ in https://codereview.chromium.org/2168693003 presumably will
land shortly after this CL.

BUG=627227
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[wjmaclean, 2016-07-22 00:21:18]

Description was changed from

==========
Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

This CL fixes both (1) the renderer crash originally fixed by
https://codereview.chromium.org/2165043002, and (2) reverts the part
of that CL which prevents ChromeOS from having zero-velocity
GestureFlingStarts, which caused a regression in two-finger swipe
navigation for touchpad. It also moves the DCHECK from the original CL
to a new location which would have prevented the renderer crashes from
being introduced in the first place.

This CL doesn't include a test, though the test proposed by
khmel@ in https://codereview.chromium.org/2168693003 presumably will
land shortly after this CL.

BUG=627227
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

This CL fixes both (1) the renderer crash originally fixed by
https://codereview.chromium.org/2165043002, and (2) reverts the part
of that CL which prevents ChromeOS from having zero-velocity
GestureFlingStarts, which caused a regression in two-finger swipe
navigation for touchpad. It also moves the DCHECK from the original CL
to a new location which would have prevented the renderer crashes from
being introduced in the first place.

This CL doesn't include a test, though the test proposed by
khmel@ in https://codereview.chromium.org/2168693003 presumably will
land shortly after this CL.

BUG=627227
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[wjmaclean, 2016-07-22 00:21:18]

wjmaclean@chromium.org changed reviewers:
+ creis@chromium.org, spang@chromium.org

[wjmaclean, 2016-07-22 00:21:26]

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

[commit-bot: I haz the power, 2016-07-22 00:21:49]

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

[wjmaclean, 2016-07-22 00:25:07]

Revised approach to fixing renderer crash from Issue 627227; the original
approach regressed an animation for touchpad two-finger swipe-navigation.

This approach still keeps zero-velocity GestureFlingStarts out of the renderer,
by adopting the approach used in RenderWidgetHostViewAura::FilterInputEvent().

khmel@ - can you try this CL with your proposed test, and if it works, we can
land your test after this CL to provide coverage.

[wjmaclean, 2016-07-22 00:42:01]

https://codereview.chromium.org/2173603002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_widget_host_view_child_frame.cc (right):

https://codereview.chromium.org/2173603002/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_widget_host_view_child_frame.cc:677: //
However, the here we've just indicated touchpad scroll has ended, and
I'll remove the redundant 'the' before landing.

https://codereview.chromium.org/2173603002/diff/1/ui/events/ozone/evdev/libge...
File ui/events/ozone/evdev/libgestures_glue/gesture_interpreter_libevdev_cros.cc
(right):

https://codereview.chromium.org/2173603002/diff/1/ui/events/ozone/evdev/libge...
ui/events/ozone/evdev/libgestures_glue/gesture_interpreter_libevdev_cros.cc:341:
// velocity instead of displacement.
I'll undo this unintended re-formatting before landing too.

[commit-bot: I haz the power, 2016-07-22 01:47:07]

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

[commit-bot: I haz the power, 2016-07-22 01:47:07]

Dry run: This issue passed the CQ dry run.

[wjmaclean, 2016-07-22 13:46:34]

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

[commit-bot: I haz the power, 2016-07-22 13:46:49]

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-07-22 14:40:27]

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

[commit-bot: I haz the power, 2016-07-22 14:40:28]

Dry run: This issue passed the CQ dry run.

[khmel, 2016-07-22 15:02:50]

On 2016/07/22 00:42:01, wjmaclean wrote:
>
https://codereview.chromium.org/2173603002/diff/1/content/browser/frame_host/...
> File content/browser/frame_host/render_widget_host_view_child_frame.cc
(right):
> 
>
https://codereview.chromium.org/2173603002/diff/1/content/browser/frame_host/...
> content/browser/frame_host/render_widget_host_view_child_frame.cc:677: //
> However, the here we've just indicated touchpad scroll has ended, and
> I'll remove the redundant 'the' before landing.
> 
>
https://codereview.chromium.org/2173603002/diff/1/ui/events/ozone/evdev/libge...
> File
ui/events/ozone/evdev/libgestures_glue/gesture_interpreter_libevdev_cros.cc
> (right):
> 
>
https://codereview.chromium.org/2173603002/diff/1/ui/events/ozone/evdev/libge...
>
ui/events/ozone/evdev/libgestures_glue/gesture_interpreter_libevdev_cros.cc:341:
> // velocity instead of displacement.
> I'll undo this unintended re-formatting before landing too.

Hi James,

Thanks for providing this fix. I tested manually on minnie all crashing test
cases and chrome navigation + passed browser_tests for this. Everything looks
good. lgtm

[Charlie Reis, 2016-07-22 17:25:55]

content/ LGTM with some comment clarifications.

https://codereview.chromium.org/2173603002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_widget_host_view_child_frame.cc (right):

https://codereview.chromium.org/2173603002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_widget_host_view_child_frame.cc:669: //
Reporting consumed for a fling suggests that there's now an *active*
nit: Can you rephrase?  I'm not sure what "Reporting consumed for a fling
suggests" means.

https://codereview.chromium.org/2173603002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_widget_host_view_child_frame.cc:672: // we're
not going to actually tick a fling animation. Report no consumer
tick?

[spang, 2016-07-22 18:24:16]

ozone lgtm

[wjmaclean, 2016-07-22 20:42:38]

The CQ bit was checked by wjmaclean@chromium.org

[wjmaclean, 2016-07-22 20:42:38]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org,
khmel@chromium.org, spang@chromium.org
Link to the patchset: https://codereview.chromium.org/2173603002/#ps40001
(title: "Clarify comments, and include reference to RWHVA::FilterInputEvent().")

[commit-bot: I haz the power, 2016-07-22 20:43:12]

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-07-22 21:53:01]

Description was changed from

==========
Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

This CL fixes both (1) the renderer crash originally fixed by
https://codereview.chromium.org/2165043002, and (2) reverts the part
of that CL which prevents ChromeOS from having zero-velocity
GestureFlingStarts, which caused a regression in two-finger swipe
navigation for touchpad. It also moves the DCHECK from the original CL
to a new location which would have prevented the renderer crashes from
being introduced in the first place.

This CL doesn't include a test, though the test proposed by
khmel@ in https://codereview.chromium.org/2168693003 presumably will
land shortly after this CL.

BUG=627227
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

This CL fixes both (1) the renderer crash originally fixed by
https://codereview.chromium.org/2165043002, and (2) reverts the part
of that CL which prevents ChromeOS from having zero-velocity
GestureFlingStarts, which caused a regression in two-finger swipe
navigation for touchpad. It also moves the DCHECK from the original CL
to a new location which would have prevented the renderer crashes from
being introduced in the first place.

This CL doesn't include a test, though the test proposed by
khmel@ in https://codereview.chromium.org/2168693003 presumably will
land shortly after this CL.

BUG=627227
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-07-22 21:53:03]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-22 21:54:30]

Description was changed from

==========
Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

This CL fixes both (1) the renderer crash originally fixed by
https://codereview.chromium.org/2165043002, and (2) reverts the part
of that CL which prevents ChromeOS from having zero-velocity
GestureFlingStarts, which caused a regression in two-finger swipe
navigation for touchpad. It also moves the DCHECK from the original CL
to a new location which would have prevented the renderer crashes from
being introduced in the first place.

This CL doesn't include a test, though the test proposed by
khmel@ in https://codereview.chromium.org/2168693003 presumably will
land shortly after this CL.

BUG=627227
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Filter GestureFlingStarts with Vx=Vy=0 in RenderWidgetHostViewChildFrame

This CL fixes both (1) the renderer crash originally fixed by
https://codereview.chromium.org/2165043002, and (2) reverts the part
of that CL which prevents ChromeOS from having zero-velocity
GestureFlingStarts, which caused a regression in two-finger swipe
navigation for touchpad. It also moves the DCHECK from the original CL
to a new location which would have prevented the renderer crashes from
being introduced in the first place.

This CL doesn't include a test, though the test proposed by
khmel@ in https://codereview.chromium.org/2168693003 presumably will
land shortly after this CL.

BUG=627227
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/d51673024bb1990807d86e8acfbf7759748fbae3
Cr-Commit-Position: refs/heads/master@{#407275}
==========

[commit-bot: I haz the power, 2016-07-22 21:54:32]

Patchset 3 (id:??) landed as
https://crrev.com/d51673024bb1990807d86e8acfbf7759748fbae3
Cr-Commit-Position: refs/heads/master@{#407275}