From abad21884e5be40e670eab118997d665be5a7618 Mon Sep 17 00:00:00 2001 From: Sahel Sharify Date: Mon, 15 Oct 2018 23:38:58 +0000 Subject: [PATCH] Reland "Fling bubbles from OOPIF properly" without the tests. This cl relands the following cl after excluding the added tests. https://chromium-review.googlesource.com/c/chromium/src/+/1278963 This gives the solution more time to bake on ToT without getting reverted due to flaky tests. Meanwhile I will work on the tests to deflake them. TBR=mcnee@chromium.org,creis@chromium.org Bug: 884728, 249063 Change-Id: Icc1e32ffe493f85d0ee74b0aff37ccdf078d0e8f Reviewed-on: https://chromium-review.googlesource.com/c/1281687 Commit-Queue: Charlie Reis Reviewed-by: Kevin McNee Reviewed-by: Charlie Reis Cr-Commit-Position: refs/heads/master@{#599781} --- .../cross_process_frame_connector.cc | 7 +++-- .../render_widget_host_input_event_router.cc | 31 ++++++++++++++++++- .../render_widget_host_input_event_router.h | 6 ++++ .../render_widget_host_view_child_frame.cc | 21 ++++++++++++- 4 files changed, 61 insertions(+), 4 deletions(-) diff --git a/content/browser/frame_host/cross_process_frame_connector.cc b/content/browser/frame_host/cross_process_frame_connector.cc index 1490ddc653c1c..ae85a1592c389 100644 --- a/content/browser/frame_host/cross_process_frame_connector.cc +++ b/content/browser/frame_host/cross_process_frame_connector.cc @@ -279,7 +279,8 @@ void CrossProcessFrameConnector::BubbleScrollEvent( DCHECK(event.GetType() == blink::WebInputEvent::kGestureScrollBegin || event.GetType() == blink::WebInputEvent::kGestureScrollUpdate || event.GetType() == blink::WebInputEvent::kGestureScrollEnd || - event.GetType() == blink::WebInputEvent::kGestureFlingStart); + event.GetType() == blink::WebInputEvent::kGestureFlingStart || + event.GetType() == blink::WebInputEvent::kGestureFlingCancel); auto* parent_view = GetParentRenderWidgetHostView(); if (!parent_view) @@ -302,7 +303,9 @@ void CrossProcessFrameConnector::BubbleScrollEvent( if (event.GetType() == blink::WebInputEvent::kGestureScrollBegin) { event_router->BubbleScrollEvent(parent_view, resent_gesture_event, view_); is_scroll_bubbling_ = true; - } else if (is_scroll_bubbling_) { + } else if (is_scroll_bubbling_ || + event.GetType() == blink::WebInputEvent::kGestureFlingCancel) { + // For GFC events the router decides whether to bubble them or not. event_router->BubbleScrollEvent(parent_view, resent_gesture_event, view_); } if (event.GetType() == blink::WebInputEvent::kGestureScrollEnd || diff --git a/content/browser/renderer_host/render_widget_host_input_event_router.cc b/content/browser/renderer_host/render_widget_host_input_event_router.cc index b5e4b7b5394a4..ea9122d4d7baa 100644 --- a/content/browser/renderer_host/render_widget_host_input_event_router.cc +++ b/content/browser/renderer_host/render_widget_host_input_event_router.cc @@ -294,6 +294,9 @@ void RenderWidgetHostInputEventRouter::OnRenderWidgetHostViewBaseDestroyed( if (view == last_fling_start_target_) last_fling_start_target_ = nullptr; + if (view == last_fling_start_bubbled_target_) + last_fling_start_bubbled_target_ = nullptr; + event_targeter_->ViewWillBeDestroyed(view); } @@ -1002,7 +1005,8 @@ void RenderWidgetHostInputEventRouter::BubbleScrollEvent( DCHECK(event.GetType() == blink::WebInputEvent::kGestureScrollBegin || event.GetType() == blink::WebInputEvent::kGestureScrollUpdate || event.GetType() == blink::WebInputEvent::kGestureScrollEnd || - event.GetType() == blink::WebInputEvent::kGestureFlingStart); + event.GetType() == blink::WebInputEvent::kGestureFlingStart || + event.GetType() == blink::WebInputEvent::kGestureFlingCancel); ui::LatencyInfo latency_info = ui::WebInputEventTraits::CreateLatencyInfoForWebGestureEvent(event); @@ -1031,7 +1035,26 @@ void RenderWidgetHostInputEventRouter::BubbleScrollEvent( } bubbling_gesture_scroll_target_.target = target_view; + } else if (event.GetType() == blink::WebInputEvent::kGestureFlingCancel) { + // TODO(828422): Remove once this issue no longer occurs. + if (resending_view == last_fling_start_bubbled_target_) { + ReportBubblingScrollToSameView(event, resending_view); + last_fling_start_bubbled_target_ = nullptr; + return; + } + // GFC event must get bubbled to the same target view that the last GFS has + // been bubbled. + if (last_fling_start_bubbled_target_) { + last_fling_start_bubbled_target_->ProcessGestureEvent( + GestureEventInTarget(event, last_fling_start_bubbled_target_), + latency_info); + last_fling_start_bubbled_target_ = nullptr; + } + return; } else { // !(event.GetType() == blink::WebInputEvent::kGestureScrollBegin) + // && !(event.GetType() == + // blink::WebInputEvent::kGestureFlingCancel) + if (!bubbling_gesture_scroll_target_.target) { // The GestureScrollBegin event is not bubbled, don't bubble the rest of // the scroll events. @@ -1063,6 +1086,12 @@ void RenderWidgetHostInputEventRouter::BubbleScrollEvent( bubbling_gesture_scroll_target_.target->ProcessGestureEvent( GestureEventInTarget(event, bubbling_gesture_scroll_target_.target), latency_info); + + // The GFC should be sent to the view that handles the GFS. + if (event.GetType() == blink::WebInputEvent::kGestureFlingStart) { + last_fling_start_bubbled_target_ = bubbling_gesture_scroll_target_.target; + } + if (event.GetType() == blink::WebInputEvent::kGestureScrollEnd || event.GetType() == blink::WebInputEvent::kGestureFlingStart) { first_bubbling_scroll_target_.target = nullptr; diff --git a/content/browser/renderer_host/render_widget_host_input_event_router.h b/content/browser/renderer_host/render_widget_host_input_event_router.h index 2506c3cf889ba..575caa90c4aaf 100644 --- a/content/browser/renderer_host/render_widget_host_input_event_router.h +++ b/content/browser/renderer_host/render_widget_host_input_event_router.h @@ -317,6 +317,12 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter // Tracked for the purpose of targeting subsequent fling cancel events. RenderWidgetHostViewBase* last_fling_start_target_ = nullptr; + // During scroll bubbling we bubble the GFS to the target view so that its + // fling controller takes care of flinging. In this case we should also send + // the GFC to the bubbling target so that the fling controller currently in + // charge of the fling progress could handle the fling cancellelation as well. + RenderWidgetHostViewBase* last_fling_start_bubbled_target_ = nullptr; + // Tracked for the purpose of providing a root_view when dispatching emulated // touch/gesture events. RenderWidgetHostViewBase* last_emulated_event_root_view_; diff --git a/content/browser/renderer_host/render_widget_host_view_child_frame.cc b/content/browser/renderer_host/render_widget_host_view_child_frame.cc index 40164dd7cc9d0..31b467efc932f 100644 --- a/content/browser/renderer_host/render_widget_host_view_child_frame.cc +++ b/content/browser/renderer_host/render_widget_host_view_child_frame.cc @@ -528,6 +528,24 @@ void RenderWidgetHostViewChildFrame::GestureEventAck( ack_result == INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS || ack_result == INPUT_EVENT_ACK_STATE_CONSUMED_SHOULD_BUBBLE; +// The inertial events on Mac should still get bubbled since there is no GFS to +// bubble and the inertial events are received from the OS. +#if !defined(OS_MACOSX) + // When a GFS is bubbled, we still send it to the fling controller of the + // child view to finish the scroll sequence. However the GSU and GSE events + // that are generated by the child view's fling controller do not need to get + // bubbled since the GFS event itself is bubbled and the target's fling + // controller will take care of flinging. + if ((event.GetType() == blink::WebInputEvent::kGestureScrollEnd && + event.data.scroll_end.inertial_phase == + blink::WebGestureEvent::kMomentumPhase) || + (event.GetType() == blink::WebInputEvent::kGestureScrollUpdate && + event.data.scroll_update.inertial_phase == + blink::WebGestureEvent::kMomentumPhase)) { + return; + } +#endif // defined(OS_MACOSX) + if ((event.GetType() == blink::WebInputEvent::kGestureScrollBegin) && should_bubble) { DCHECK(!is_scroll_sequence_bubbling_); @@ -546,7 +564,8 @@ void RenderWidgetHostViewChildFrame::GestureEventAck( should_bubble) || event.GetType() == blink::WebInputEvent::kGestureScrollUpdate || event.GetType() == blink::WebInputEvent::kGestureScrollEnd || - event.GetType() == blink::WebInputEvent::kGestureFlingStart) { + event.GetType() == blink::WebInputEvent::kGestureFlingStart || + event.GetType() == blink::WebInputEvent::kGestureFlingCancel) { frame_connector_->BubbleScrollEvent(event); } }