Commit ffea7a5b authored by pedrosimonetti's avatar pedrosimonetti Committed by Commit bot

[Contextual Search] Pinch zoom might cause a crash.

ContextualSearchEventFilter has a complex logic to detect
where to propagate events to, whether the Search Panel or
the Search Content View. Besides that, a single stream of
events (a single gesture) might be forwarded to different
targets depending on a number of factors (whether the
Panel was maximized, whether the gesture was horizontal
or vertical, whether the Search Content View is scrolled
at the top, and so on).

Things can get much more complicated when multiple
pointers are involved, like in gestures with multiple
fingers. If we mess with the order of the events, or
don't properly forward events from all pointers to the
Search Content View it will crash.

This CL fixes the problem by:

1) Forwarding all pointer events when multiple finger
gestures are used.

2) Ignoring multitouch events when forwarding horizontal
gestures to the expanded Panel. In this particular case,
we must ignore multitouch events because the presence of
a second pointer will cause the Search Page to scroll,
which is not supposed to happen in this case. So, now
we are properly ignoring ACTION_POINTER_DOWN and
ACTION_POINTER_UP events, while at the same time,
creating MotionEvents with a single pointer in order
to implement the intended "lock horizontally" behavior.

BUG=485008
BUG=486901
BUG=486902

Review URL: https://codereview.chromium.org/1137003003

Cr-Commit-Position: refs/heads/master@{#329275}
parent c1134634
...@@ -179,14 +179,7 @@ public class ContextualSearchEventFilter extends GestureEventFilter { ...@@ -179,14 +179,7 @@ public class ContextualSearchEventFilter extends GestureEventFilter {
public boolean onTouchEventInternal(MotionEvent e) { public boolean onTouchEventInternal(MotionEvent e) {
final int action = e.getActionMasked(); final int action = e.getActionMasked();
if (action == MotionEvent.ACTION_POINTER_DOWN if (!mIsDeterminingEventTarget && action == MotionEvent.ACTION_DOWN) {
&& e.getPointerCount() == 2
&& !mSearchPanel.isMaximized()) {
// We don't want the Search Content View's zoom level to change when the Search Panel
// is expanded (that is, not maximized) so we'll forward the events to Panel to
// prevent it from happening.
setEventTarget(EventTarget.SEARCH_PANEL);
} else if (!mIsDeterminingEventTarget && action == MotionEvent.ACTION_DOWN) {
mInitialEventY = e.getY(); mInitialEventY = e.getY();
if (mSearchPanel.isYCoordinateInsideSearchContentView(mInitialEventY * mPxToDp)) { if (mSearchPanel.isYCoordinateInsideSearchContentView(mInitialEventY * mPxToDp)) {
// If the DOWN event happened inside the Search Content View, we'll need // If the DOWN event happened inside the Search Content View, we'll need
...@@ -262,17 +255,14 @@ public class ContextualSearchEventFilter extends GestureEventFilter { ...@@ -262,17 +255,14 @@ public class ContextualSearchEventFilter extends GestureEventFilter {
} }
if (mHasChangedEventTarget) { if (mHasChangedEventTarget) {
// TODO(pedrosimonetti): handle cases with multiple pointers.
float y = e.getY();
// If the event target has changed since the beginning of the gesture, then we need // If the event target has changed since the beginning of the gesture, then we need
// to send a ACTION_CANCEL to the previous event target to make sure it no longer // to send a ACTION_CANCEL to the previous event target to make sure it no longer
// expects events. // expects events.
propagateAndRecycleEvent(createEvent(e, MotionEvent.ACTION_CANCEL, y), propagateAndRecycleEvent(copyEvent(e, MotionEvent.ACTION_CANCEL), mPreviousEventTarget);
mPreviousEventTarget);
// Similarly we need to send an ACTION_DOWN to the new event target so subsequent // Similarly we need to send an ACTION_DOWN to the new event target so subsequent
// events can be analyzed properly by the Gesture Detector. // events can be analyzed properly by the Gesture Detector.
MotionEvent syntheticActionDownEvent = createEvent(e, MotionEvent.ACTION_DOWN, y); MotionEvent syntheticActionDownEvent = copyEvent(e, MotionEvent.ACTION_DOWN);
// Store the synthetic ACTION_DOWN coordinates to prevent unwanted taps from // Store the synthetic ACTION_DOWN coordinates to prevent unwanted taps from
// happening. See {@link ContextualSearchEventFilter#propagateEventToSearchContentView}. // happening. See {@link ContextualSearchEventFilter#propagateEventToSearchContentView}.
...@@ -304,7 +294,10 @@ public class ContextualSearchEventFilter extends GestureEventFilter { ...@@ -304,7 +294,10 @@ public class ContextualSearchEventFilter extends GestureEventFilter {
/** /**
* Propagates the given {@link MotionEvent} to the given {@link EventTarget}, recycling it * Propagates the given {@link MotionEvent} to the given {@link EventTarget}, recycling it
* afterwards. This is intended for synthetic events only, those create by * afterwards. This is intended for synthetic events only, those create by
* {@link MotionEvent#obtain} or the helper {@link ContextualSearchEventFilter#createEvent}. * {@link MotionEvent#obtain} or the helper methods
* {@link ContextualSearchEventFilter#lockEventHorizontallty} and
* {@link ContextualSearchEventFilter#copyEvent}.
*
* @param e The {@link MotionEvent} to be propagated. * @param e The {@link MotionEvent} to be propagated.
* @param target The {@link EventTarget} to propagate events to. * @param target The {@link EventTarget} to propagate events to.
*/ */
...@@ -333,17 +326,37 @@ public class ContextualSearchEventFilter extends GestureEventFilter { ...@@ -333,17 +326,37 @@ public class ContextualSearchEventFilter extends GestureEventFilter {
@VisibleForTesting @VisibleForTesting
protected void propagateEventToSearchContentView(MotionEvent e) { protected void propagateEventToSearchContentView(MotionEvent e) {
MotionEvent event = e; MotionEvent event = e;
int action = event.getActionMasked();
boolean isSyntheticEvent = false; boolean isSyntheticEvent = false;
if (mGestureOrientation == GestureOrientation.HORIZONTAL if (mGestureOrientation == GestureOrientation.HORIZONTAL
&& !mSearchPanel.isMaximized()) { && !mSearchPanel.isMaximized()) {
// Lock horizontal motion, ignoring all vertical changes, when the Panel is not // Ignores multitouch events to prevent the Search Result Page from from scrolling.
// maximized. This is to prevent the Search Result Page from scrolling when if (action == MotionEvent.ACTION_POINTER_UP
// side swiping on the expanded Panel. || action == MotionEvent.ACTION_POINTER_DOWN) {
event = createEvent(e, e.getAction(), mInitialEventY); return;
}
// NOTE(pedrosimonetti): Lock horizontal motion, ignoring all vertical changes,
// when the Panel is not maximized. This is to prevent the Search Result Page
// from scrolling when side swiping on the expanded Panel. Also, note that the
// method {@link ContextualSearchEventFilter#lockEventHorizontallty} will always
// return an event with a single pointer, which is necessary to prevent
// the app from crashing when the motion involves multiple pointers.
// See: crbug.com/486901
event = MotionEvent.obtain(
e.getDownTime(),
e.getEventTime(),
// NOTE(pedrosimonetti): Use getActionMasked() to make sure we're not
// send any pointer information to the event, given that getAction()
// may have the pointer Id associated to it.
e.getActionMasked(),
e.getX(),
mInitialEventY,
e.getMetaState());
isSyntheticEvent = true; isSyntheticEvent = true;
} }
int action = event.getActionMasked();
float searchContentViewOffsetYPx = mSearchPanel.getSearchContentViewOffsetY() / mPxToDp; float searchContentViewOffsetYPx = mSearchPanel.getSearchContentViewOffsetY() / mPxToDp;
// Adjust the offset to be relative to the Search Contents View. // Adjust the offset to be relative to the Search Contents View.
...@@ -377,17 +390,12 @@ public class ContextualSearchEventFilter extends GestureEventFilter { ...@@ -377,17 +390,12 @@ public class ContextualSearchEventFilter extends GestureEventFilter {
* Creates a {@link MotionEvent} inheriting from a given |e| event. * Creates a {@link MotionEvent} inheriting from a given |e| event.
* @param e The {@link MotionEvent} to inherit properties from. * @param e The {@link MotionEvent} to inherit properties from.
* @param action The MotionEvent's Action to be used. * @param action The MotionEvent's Action to be used.
* @param y The y coordinate to be used.
* @return A new {@link MotionEvent}. * @return A new {@link MotionEvent}.
*/ */
private MotionEvent createEvent(MotionEvent e, int action, float y) { private MotionEvent copyEvent(MotionEvent e, int action) {
return MotionEvent.obtain( MotionEvent event = MotionEvent.obtain(e);
e.getDownTime(), event.setAction(action);
e.getEventTime(), return event;
action,
e.getX(),
y,
e.getMetaState());
} }
/** /**
...@@ -422,8 +430,9 @@ public class ContextualSearchEventFilter extends GestureEventFilter { ...@@ -422,8 +430,9 @@ public class ContextualSearchEventFilter extends GestureEventFilter {
// if it hasn't been determined yet or if changing the event target during the // if it hasn't been determined yet or if changing the event target during the
// middle of the gesture is supported. This will allow a smooth transition from // middle of the gesture is supported. This will allow a smooth transition from
// swiping the Panel and scrolling the Search Content View. // swiping the Panel and scrolling the Search Content View.
final boolean mayChangeEventTarget = mMayChangeEventTarget && e2.getPointerCount() == 1;
if (mHasDeterminedGestureOrientation if (mHasDeterminedGestureOrientation
&& (!mHasDeterminedEventTarget || mMayChangeEventTarget)) { && (!mHasDeterminedEventTarget || mayChangeEventTarget)) {
determineEventTarget(distanceY); determineEventTarget(distanceY);
} }
...@@ -465,14 +474,21 @@ public class ContextualSearchEventFilter extends GestureEventFilter { ...@@ -465,14 +474,21 @@ public class ContextualSearchEventFilter extends GestureEventFilter {
// Only allow horizontal movements to be propagated to the Search Content View // Only allow horizontal movements to be propagated to the Search Content View
// when the Panel is expanded (that is, not maximized). // when the Panel is expanded (that is, not maximized).
shouldPropagateEventsToSearchPanel = isVertical; shouldPropagateEventsToSearchPanel = isVertical;
// If the gesture is horizontal, then we know that the event target won't change.
if (!isVertical) mMayChangeEventTarget = false;
} }
mPreviousEventTarget = mEventTarget; EventTarget target = shouldPropagateEventsToSearchPanel
setEventTarget(shouldPropagateEventsToSearchPanel ? EventTarget.SEARCH_PANEL : EventTarget.SEARCH_CONTENT_VIEW;
? EventTarget.SEARCH_PANEL : EventTarget.SEARCH_CONTENT_VIEW);
mHasChangedEventTarget = mEventTarget != mPreviousEventTarget if (target != mEventTarget) {
&& mPreviousEventTarget != EventTarget.UNDETERMINED; mPreviousEventTarget = mEventTarget;
setEventTarget(target);
mHasChangedEventTarget = mEventTarget != mPreviousEventTarget
&& mPreviousEventTarget != EventTarget.UNDETERMINED;
}
} }
/** /**
......
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