Commit d5167302 authored by wjmaclean's avatar wjmaclean Committed by Commit bot

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

Review-Url: https://codereview.chromium.org/2173603002
Cr-Commit-Position: refs/heads/master@{#407275}
parent 524692f9
...@@ -33,6 +33,7 @@ ...@@ -33,6 +33,7 @@
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "content/public/common/browser_plugin_guest_mode.h" #include "content/public/common/browser_plugin_guest_mode.h"
#include "gpu/ipc/common/gpu_messages.h" #include "gpu/ipc/common/gpu_messages.h"
#include "third_party/WebKit/public/web/WebInputEvent.h"
#include "ui/gfx/geometry/size_conversions.h" #include "ui/gfx/geometry/size_conversions.h"
#include "ui/gfx/geometry/size_f.h" #include "ui/gfx/geometry/size_f.h"
...@@ -659,6 +660,31 @@ void RenderWidgetHostViewChildFrame::OnSetNeedsBeginFrames( ...@@ -659,6 +660,31 @@ void RenderWidgetHostViewChildFrame::OnSetNeedsBeginFrames(
} }
} }
InputEventAckState RenderWidgetHostViewChildFrame::FilterInputEvent(
const blink::WebInputEvent& input_event) {
if (input_event.type == blink::WebInputEvent::GestureFlingStart) {
const blink::WebGestureEvent& gesture_event =
static_cast<const blink::WebGestureEvent&>(input_event);
// Zero-velocity touchpad flings are an Aura-specific signal that the
// touchpad scroll has ended, and should not be forwarded to the renderer.
if (gesture_event.sourceDevice == blink::WebGestureDeviceTouchpad &&
!gesture_event.data.flingStart.velocityX &&
!gesture_event.data.flingStart.velocityY) {
// Here we indicate that there was no consumer for this event, as
// otherwise the fling animation system will try to run an animation
// and will also expect a notification when the fling ends. Since
// CrOS just uses the GestureFlingStart with zero-velocity as a means
// of indicating that touchpad scroll has ended, we don't actually want
// a fling animation.
// Note: this event handling is modeled on similar code in
// TenderWidgetHostViewAura::FilterInputEvent().
return INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS;
}
}
return INPUT_EVENT_ACK_STATE_NOT_CONSUMED;
}
BrowserAccessibilityManager* BrowserAccessibilityManager*
RenderWidgetHostViewChildFrame::CreateBrowserAccessibilityManager( RenderWidgetHostViewChildFrame::CreateBrowserAccessibilityManager(
BrowserAccessibilityDelegate* delegate, bool for_root_frame) { BrowserAccessibilityDelegate* delegate, bool for_root_frame) {
......
...@@ -150,6 +150,8 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame ...@@ -150,6 +150,8 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
void LockCompositingSurface() override; void LockCompositingSurface() override;
void UnlockCompositingSurface() override; void UnlockCompositingSurface() override;
InputEventAckState FilterInputEvent(
const blink::WebInputEvent& input_event) override;
BrowserAccessibilityManager* CreateBrowserAccessibilityManager( BrowserAccessibilityManager* CreateBrowserAccessibilityManager(
BrowserAccessibilityDelegate* delegate, bool for_root_frame) override; BrowserAccessibilityDelegate* delegate, bool for_root_frame) override;
......
...@@ -429,6 +429,12 @@ bool InputRouterImpl::OfferToClient(const WebInputEvent& input_event, ...@@ -429,6 +429,12 @@ bool InputRouterImpl::OfferToClient(const WebInputEvent& input_event,
bool InputRouterImpl::OfferToRenderer(const WebInputEvent& input_event, bool InputRouterImpl::OfferToRenderer(const WebInputEvent& input_event,
const ui::LatencyInfo& latency_info, const ui::LatencyInfo& latency_info,
InputEventDispatchType dispatch_type) { InputEventDispatchType dispatch_type) {
DCHECK(input_event.type != blink::WebInputEvent::GestureFlingStart ||
static_cast<const blink::WebGestureEvent&>(input_event)
.data.flingStart.velocityX != 0.0 ||
static_cast<const blink::WebGestureEvent&>(input_event)
.data.flingStart.velocityY != 0.0);
// This conversion is temporary. WebInputEvent should be generated // This conversion is temporary. WebInputEvent should be generated
// directly from ui::Event with the viewport coordinates. See // directly from ui::Event with the viewport coordinates. See
// crbug.com/563730. // crbug.com/563730.
......
...@@ -173,10 +173,6 @@ void RenderWidgetHostInputEventRouter::RouteGestureEvent( ...@@ -173,10 +173,6 @@ void RenderWidgetHostInputEventRouter::RouteGestureEvent(
RenderWidgetHostViewBase* root_view, RenderWidgetHostViewBase* root_view,
blink::WebGestureEvent* event, blink::WebGestureEvent* event,
const ui::LatencyInfo& latency) { const ui::LatencyInfo& latency) {
DCHECK(event->type != blink::WebInputEvent::GestureFlingStart ||
event->data.flingStart.velocityX != 0.0 ||
event->data.flingStart.velocityY != 0.0);
switch (event->sourceDevice) { switch (event->sourceDevice) {
case blink::WebGestureDeviceUninitialized: case blink::WebGestureDeviceUninitialized:
NOTREACHED() << "Uninitialized device type is not allowed"; NOTREACHED() << "Uninitialized device type is not allowed";
......
...@@ -1254,12 +1254,16 @@ InputEventAckState RenderWidgetHostViewAura::FilterInputEvent( ...@@ -1254,12 +1254,16 @@ InputEventAckState RenderWidgetHostViewAura::FilterInputEvent(
if (WebTouchEvent::isTouchEventType(input_event.type)) if (WebTouchEvent::isTouchEventType(input_event.type))
return INPUT_EVENT_ACK_STATE_NOT_CONSUMED; return INPUT_EVENT_ACK_STATE_NOT_CONSUMED;
// Reporting consumed for a fling suggests that there's now an *active* fling if (consumed && input_event.type == blink::WebInputEvent::GestureFlingStart) {
// that requires both animation and a fling-end notification. However, the // Here we indicate that there was no consumer for this event, as
// OverscrollController consumes a fling to stop its propagation; it doesn't // otherwise the fling animation system will try to run an animation
// actually tick a fling animation. Report no consumer to convey this. // and will also expect a notification when the fling ends. Since
if (consumed && input_event.type == blink::WebInputEvent::GestureFlingStart) // CrOS just uses the GestureFlingStart with zero-velocity as a means
// of indicating that touchpad scroll has ended, we don't actually want
// a fling animation. Note: Similar code exists in
// RenderWidgetHostViewChildFrame::FilterInputEvent()
return INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS; return INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS;
}
return consumed ? INPUT_EVENT_ACK_STATE_CONSUMED return consumed ? INPUT_EVENT_ACK_STATE_CONSUMED
: INPUT_EVENT_ACK_STATE_NOT_CONSUMED; : INPUT_EVENT_ACK_STATE_NOT_CONSUMED;
......
...@@ -333,14 +333,9 @@ void GestureInterpreterLibevdevCros::OnGestureFling(const Gesture* gesture, ...@@ -333,14 +333,9 @@ void GestureInterpreterLibevdevCros::OnGestureFling(const Gesture* gesture,
if (!cursor_) if (!cursor_)
return; // No cursor! return; // No cursor!
// We may receive a request for a GESTURE_FLING_START with zero velocity; in
// this case send a ET_SCROLL_FLING_CANCEL in case it's needed to stop a
// fling in progress.
// TODO(wjmaclean): is it possible to get consecutive GESTURE_FLING_STARTs?
bool should_fling_start = fling->fling_state == GESTURES_FLING_START &&
(fling->vx != 0 || fling->vy != 0);
EventType type = EventType type =
(should_fling_start ? ET_SCROLL_FLING_START : ET_SCROLL_FLING_CANCEL); (fling->fling_state == GESTURES_FLING_START ? ET_SCROLL_FLING_START
: ET_SCROLL_FLING_CANCEL);
// Fling is like 2-finger scrolling but with velocity instead of displacement. // Fling is like 2-finger scrolling but with velocity instead of displacement.
dispatcher_->DispatchScrollEvent(ScrollEventParams( dispatcher_->DispatchScrollEvent(ScrollEventParams(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment