From c710c2ce2de97207dba9c275097d85e951e02d73 Mon Sep 17 00:00:00 2001 From: "jdduke@chromium.org" Date: Tue, 17 Dec 2013 23:08:28 +0000 Subject: [Android] Suppress spurious ShowPress after a preventDefault'ed touch Properly reset the GestureDetector if a touch sequence is preventDefault'ed before it has turned into a movement gesture. This fixes the following problem sequence: 1) A touchstart is unconsumed by Javascript, and fed to the GestureDetector 2) The next touchmove is preventDefault'ed, and not fed to the GestureDetector 3) The ShowPress gesture timeout fires, as it never received a touchmove. BUG=328886 Review URL: https://codereview.chromium.org/105003009 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@241402 0039d316-1c4b-4281-b951-d872f2087c98 --- .../content/browser/ContentViewGestureHandler.java | 17 +++++-- .../browser/ContentViewGestureHandlerTest.java | 57 ++++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) (limited to 'content') diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewGestureHandler.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewGestureHandler.java index f4c4d89..32c8ad5 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ContentViewGestureHandler.java +++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewGestureHandler.java @@ -899,6 +899,8 @@ class ContentViewGestureHandler implements LongPressDelegate { // should always be offered to Javascript (when there is any touch handler). if (event.getActionMasked() == MotionEvent.ACTION_DOWN) { mTouchHandlingState = HAS_TOUCH_HANDLER; + if (mCurrentDownEvent != null) recycleEvent(mCurrentDownEvent); + mCurrentDownEvent = null; } mLongPressDetector.onOfferTouchEventToJavaScript(event); @@ -916,15 +918,15 @@ class ContentViewGestureHandler implements LongPressDelegate { } } + if (mTouchScrolling || mPinchInProgress) return EVENT_NOT_FORWARDED; + TouchPoint[] pts = new TouchPoint[event.getPointerCount()]; int type = TouchPoint.createTouchPoints(event, pts); if (type == TouchPoint.CONVERSION_ERROR) return EVENT_NOT_FORWARDED; - if (!mTouchScrolling && !mPinchInProgress) { - if (mMotionEventDelegate.sendTouchEvent(event.getEventTime(), type, pts)) { - return EVENT_FORWARDED_TO_NATIVE; - } + if (mMotionEventDelegate.sendTouchEvent(event.getEventTime(), type, pts)) { + return EVENT_FORWARDED_TO_NATIVE; } return EVENT_NOT_FORWARDED; } @@ -990,8 +992,13 @@ class ContentViewGestureHandler implements LongPressDelegate { break; case INPUT_EVENT_ACK_STATE_CONSUMED: case INPUT_EVENT_ACK_STATE_IGNORED: + if (mTouchHandlingState != JAVASCRIPT_CONSUMING_GESTURE + && ackedEvent.getActionMasked() != MotionEvent.ACTION_DOWN) { + resetGestureHandlers(); + } else { + mZoomManager.passTouchEventThrough(ackedEvent); + } mTouchHandlingState = JAVASCRIPT_CONSUMING_GESTURE; - mZoomManager.passTouchEventThrough(ackedEvent); trySendPendingEventsToNative(); break; case INPUT_EVENT_ACK_STATE_NOT_CONSUMED: diff --git a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java b/content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java index 3035e97..e1a4406 100644 --- a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java +++ b/content/public/android/javatests/src/org/chromium/content/browser/ContentViewGestureHandlerTest.java @@ -16,11 +16,13 @@ import android.view.MotionEvent.PointerProperties; import android.view.ViewConfiguration; import org.chromium.base.test.util.Feature; +import org.chromium.base.test.util.ScalableTimeout; import org.chromium.content.browser.ContentViewGestureHandler.MotionEventDelegate; import org.chromium.content.browser.third_party.GestureDetector; import java.util.ArrayList; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; /** * Test suite for ContentViewGestureHandler. @@ -39,12 +41,15 @@ public class ContentViewGestureHandlerTest extends InstrumentationTestCase { static class MockListener extends GestureDetector.SimpleOnGestureListener { MotionEvent mLastLongPress; + MotionEvent mLastShowPress; MotionEvent mLastSingleTap; MotionEvent mLastFling1; CountDownLatch mLongPressCalled; + CountDownLatch mShowPressCalled; public MockListener() { mLongPressCalled = new CountDownLatch(1); + mShowPressCalled = new CountDownLatch(1); } @Override @@ -54,6 +59,13 @@ public class ContentViewGestureHandlerTest extends InstrumentationTestCase { } @Override + public void onShowPress(MotionEvent e) { + mLastShowPress = MotionEvent.obtain(e); + mShowPressCalled.countDown(); + Log.e("Overscroll", "OnShowPress"); + } + + @Override public boolean onSingleTapConfirmed(MotionEvent e) { mLastSingleTap = e; return true; @@ -2192,4 +2204,49 @@ public class ContentViewGestureHandlerTest extends InstrumentationTestCase { assertEquals("A down event should be offered to Javascript with a registered touch handler", 1, mGestureHandler.getNumberOfPendingMotionEventsForTesting()); } + + /** + * Verify that no timeout-based gestures are triggered after a touch event + * is consumed. In particular, LONG_PRESS and SHOW_PRESS should not fire + * if TouchStart went unconsumed, but subsequent TouchMoves are consumed. + * + * @throws Exception + */ + @SmallTest + @Feature({"Gestures"}) + public void testNoTimeoutGestureAfterTouchConsumed() throws Exception { + getInstrumentation().runOnMainSync(new Runnable() { + @Override + public void run() { + setUp(); + + final long downTime = SystemClock.uptimeMillis(); + final long eventTime = SystemClock.uptimeMillis(); + + mGestureHandler.hasTouchEventHandlers(true); + + MotionEvent event = motionEvent(MotionEvent.ACTION_DOWN, downTime, eventTime); + assertTrue(mGestureHandler.onTouchEvent(event)); + mGestureHandler.confirmTouchEvent( + ContentViewGestureHandler.INPUT_EVENT_ACK_STATE_NOT_CONSUMED); + assertTrue("Should have a pending LONG_PRESS", + mLongPressDetector.hasPendingMessage()); + + event = MotionEvent.obtain( + downTime, eventTime + 10, MotionEvent.ACTION_MOVE, + FAKE_COORD_X, FAKE_COORD_Y + 200, 0); + assertTrue(mGestureHandler.onTouchEvent(event)); + mGestureHandler.confirmTouchEvent( + ContentViewGestureHandler.INPUT_EVENT_ACK_STATE_CONSUMED); + assertFalse("Should not have a pending LONG_PRESS", + mLongPressDetector.hasPendingMessage()); + } + }); + assertFalse(mMockListener.mShowPressCalled.await( + ScalableTimeout.ScaleTimeout(ViewConfiguration.getTapTimeout() + 10), + TimeUnit.MILLISECONDS)); + assertFalse(mMockListener.mLongPressCalled.await( + ScalableTimeout.ScaleTimeout(ViewConfiguration.getLongPressTimeout() + 10), + TimeUnit.MILLISECONDS)); + } } -- cgit v1.1