From 225f25b4ddec1e2f8f9039476284e23e031315fa Mon Sep 17 00:00:00 2001 From: Jared Duke Date: Wed, 20 May 2015 12:00:52 -0700 Subject: [PATCH] [Android] Prevent touch interception for browser-consumed touches Browser-based widgets that overlay content have a shot at consuming touch events, e.g., selection handles. Such touches are reported as consumed to the View chain, however, embedding apps still have the opportunity to intercept those touch sequences. This can lead to double-handling, e.g., when dragging a selection handle left or right and triggering a View side swipe. Prevent this by calling requestDisallowInterceptTouchEvent() for touches that are consumed by such browser-based widgets. Note that we cannot call this method for all touch events as touch dispatch is often asynchronous and the disposition of the touch handling result cannot be immediately known when the touch is received from the Android platform. BUG=489337 Review URL: https://codereview.chromium.org/1140693004 Cr-Commit-Position: refs/heads/master@{#330426} (cherry picked from commit 39083255b50c17e8a75705d4bd6f0a209f9ccdbd) Review URL: https://codereview.chromium.org/1145263002 Cr-Commit-Position: refs/branch-heads/2403@{#32} Cr-Branched-From: f54b8097a9c45ed4ad308133d49f05325d6c5070-refs/heads/master@{#330231} --- .../browser/android/content_view_core_impl.cc | 7 ++++ .../browser/android/content_view_core_impl.h | 1 + .../render_widget_host_view_android.cc | 42 +++++++++++++------ .../render_widget_host_view_android.h | 1 + .../content/browser/ContentViewCore.java | 5 +++ 5 files changed, 44 insertions(+), 12 deletions(-) diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc index 92d7f5afd4dd7..123d59ba53afe 100644 --- a/content/browser/android/content_view_core_impl.cc +++ b/content/browser/android/content_view_core_impl.cc @@ -584,6 +584,13 @@ bool ContentViewCoreImpl::HasFocus() { return Java_ContentViewCore_hasFocus(env, obj.obj()); } +void ContentViewCoreImpl::RequestDisallowInterceptTouchEvent() { + JNIEnv* env = AttachCurrentThread(); + ScopedJavaLocalRef obj = java_ref_.get(env); + if (!obj.is_null()) + Java_ContentViewCore_requestDisallowInterceptTouchEvent(env, obj.obj()); +} + void ContentViewCoreImpl::OnSelectionChanged(const std::string& text) { JNIEnv* env = AttachCurrentThread(); ScopedJavaLocalRef obj = java_ref_.get(env); diff --git a/content/browser/android/content_view_core_impl.h b/content/browser/android/content_view_core_impl.h index d280c0a1fb7d3..0ca7c6472478a 100644 --- a/content/browser/android/content_view_core_impl.h +++ b/content/browser/android/content_view_core_impl.h @@ -239,6 +239,7 @@ class ContentViewCoreImpl : public ContentViewCore, void OnBackgroundColorChanged(SkColor color); bool HasFocus(); + void RequestDisallowInterceptTouchEvent(); void OnGestureEventAck(const blink::WebGestureEvent& event, InputEventAckState ack_result); bool FilterInputEvent(const blink::WebInputEvent& event); diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc index 2d41d0879d14e..0576b33ca827c 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.cc +++ b/content/browser/renderer_host/render_widget_host_view_android.cc @@ -748,12 +748,19 @@ bool RenderWidgetHostViewAndroid::OnTouchEvent( if (!host_) return false; + // If a browser-based widget consumes the touch event, it's critical that + // touch event interception be disabled. This avoids issues with + // double-handling for embedder-detected gestures like side swipe. if (selection_controller_ && - selection_controller_->WillHandleTouchEvent(event)) + selection_controller_->WillHandleTouchEvent(event)) { + RequestDisallowInterceptTouchEvent(); return true; + } - if (stylus_text_selector_.OnTouchEvent(event)) + if (stylus_text_selector_.OnTouchEvent(event)) { + RequestDisallowInterceptTouchEvent(); return true; + } ui::FilteredGestureProvider::TouchHandlingResult result = gesture_provider_.OnTouchEvent(event); @@ -782,13 +789,20 @@ bool RenderWidgetHostViewAndroid::OnTouchHandleEvent( void RenderWidgetHostViewAndroid::ResetGestureDetection() { const ui::MotionEvent* current_down_event = gesture_provider_.GetCurrentDownEvent(); - if (current_down_event) { - scoped_ptr cancel_event = current_down_event->Cancel(); - OnTouchEvent(*cancel_event); + if (!current_down_event) { + // A hard reset ensures prevention of any timer-based events that might fire + // after a touch sequence has ended. + gesture_provider_.ResetDetection(); + return; } - // A hard reset ensures prevention of any timer-based events. - gesture_provider_.ResetDetection(); + scoped_ptr cancel_event = current_down_event->Cancel(); + if (gesture_provider_.OnTouchEvent(*cancel_event).succeeded) { + bool causes_scrolling = false; + host_->ForwardTouchEventWithLatencyInfo( + ui::CreateWebTouchEventFromMotionEvent(*cancel_event, causes_scrolling), + ui::LatencyInfo()); + } } void RenderWidgetHostViewAndroid::OnDidNavigateMainFrameToNewPage() { @@ -1263,11 +1277,10 @@ void RenderWidgetHostViewAndroid::OnSelectionEvent( ui::SelectionEventType event) { DCHECK(content_view_core_); DCHECK(selection_controller_); - // Showing the selection action bar can alter the current View coordinates in - // such a way that the current MotionEvent stream is suddenly shifted in - // space. Avoid the associated scroll jump by pre-emptively cancelling gesture - // detection; scrolling after the selection is activated is unnecessary. - if (event == ui::SelectionEventType::SELECTION_SHOWN) + // If a selection drag has started, it has taken over the active touch + // sequence. Immediately cancel gesture detection and any downstream touch + // listeners (e.g., web content) to communicate this transfer. + if (event == ui::SELECTION_SHOWN) ResetGestureDetection(); content_view_core_->OnSelectionEvent( event, selection_controller_->GetStartPosition(), @@ -1531,6 +1544,11 @@ bool RenderWidgetHostViewAndroid::Animate(base::TimeTicks frame_time) { return needs_animate; } +void RenderWidgetHostViewAndroid::RequestDisallowInterceptTouchEvent() { + if (content_view_core_) + content_view_core_->RequestDisallowInterceptTouchEvent(); +} + void RenderWidgetHostViewAndroid::EvictDelegatedFrame() { if (layer_.get()) DestroyDelegatedContent(); diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h index fe1ea3eb21998..fc8decb070d1d 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.h +++ b/content/browser/renderer_host/render_widget_host_view_android.h @@ -323,6 +323,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid void StopObservingRootWindow(); void SendBeginFrame(base::TimeTicks frame_time, base::TimeDelta vsync_period); bool Animate(base::TimeTicks frame_time); + void RequestDisallowInterceptTouchEvent(); // The model object. RenderWidgetHostImpl* host_; diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java index 2ebfb8e79e70a..5ca1e8c548463 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java +++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java @@ -1188,6 +1188,11 @@ private boolean onTouchEventImpl(MotionEvent event, boolean isTouchHandleEvent) } } + @CalledByNative + private void requestDisallowInterceptTouchEvent() { + mContainerView.requestDisallowInterceptTouchEvent(true); + } + private static boolean isValidTouchEventActionForNative(int eventAction) { // Only these actions have any effect on gesture detection. Other // actions have no corresponding WebTouchEvent type and may confuse the