Child consumes scroll that it should ignore with scroll latching and OOPIF-based guests |
|||
Issue descriptionChrome Version: 63.0.3227.0 What steps will reproduce the problem? (1) Ensure scroll-latching (TouchpadAndWheelScrollLatching) and OOPIF-based guests (GuestViewCrossProcessFrames) are enabled (2) Visit chrome://chrome-signin/ (3) Resize the page if necessary so that the guest is scrollable (4) Scroll inside the guest when it is at a scroll extent so that the rest of the scroll gesture sequence should be ignored in the guest (5) In the same gesture, change to a scrollable direction in the guest What is the expected result? The guest does not scroll for the rest of the gesture. What happens instead? The guest scrolls. We get the following sequence of acks: GestureScrollBegin INPUT_EVENT_ACK_STATE_CONSUMED_SHOULD_BUBBLE GestureScrollUpdate INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS upon direction change, we get GestureScrollUpdate INPUT_EVENT_ACK_STATE_CONSUMED GestureScrollEnd INPUT_EVENT_ACK_STATE_IGNORED Compare to the regular OOPIF case where the GSB and GSUs are all INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS.
,
Oct 3 2017
Assigning it to mcnee@ since he has a working fix for this, thanks!
,
Nov 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08bf6de63fc34302bfb764ffe72f9f9c1bbd68c2 commit 08bf6de63fc34302bfb764ffe72f9f9c1bbd68c2 Author: Kevin McNee <mcnee@chromium.org> Date: Wed Nov 08 15:24:48 2017 Preserve scroll latching in guest views. In chromium-review.googlesource.com/c/chromium/src/+/581910/, there was an attempt to prevent the guest's viewport from being added to the scroll chain. However, this was considered a layering violation as cc should not know about guests. Instead, if the viewport is the scrolling node and it cannot scroll according to the GSB's delta hints, we get an INPUT_EVENT_ACK_STATE_CONSUMED_SHOULD_BUBBLE ack so that RWHVCF knows that it should still bubble even though the event was consumed. The RWHVCF is currently still forwarding the subsequent events in the scroll sequence to the child before bubbling them. This is a problem for scroll latching as the child can consume them. If the scroll changed direction to a scrollable direction in the child, then the child would scroll, and then the event would be bubbled to the bubbling target which could also consume scroll. We now use the ack of the GSB in RWHVCF to determine whether to send events that are intended for bubbling to the child. If the gesture sequence is being bubbled, then we immediately bubble the event without sending it to the renderer. For the guest view case, this is needed for correctness. We can also do this for the general OOPIF case where we know that the renderer will just drop the event and send an ack that it was not consumed. Not sending the event to the renderer is an optimization in this case. Bug: 770852 Change-Id: I02ef22966574b5ce84fe8ea6ac803c8c3e5a2074 Reviewed-on: https://chromium-review.googlesource.com/699699 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Sahel Sharifymoghaddam <sahel@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: James MacLean <wjmaclean@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#514832} [modify] https://crrev.com/08bf6de63fc34302bfb764ffe72f9f9c1bbd68c2/chrome/browser/apps/guest_view/web_view_browsertest.cc [modify] https://crrev.com/08bf6de63fc34302bfb764ffe72f9f9c1bbd68c2/content/browser/renderer_host/render_widget_host_view_child_frame.cc [modify] https://crrev.com/08bf6de63fc34302bfb764ffe72f9f9c1bbd68c2/content/browser/renderer_host/render_widget_host_view_child_frame.h [modify] https://crrev.com/08bf6de63fc34302bfb764ffe72f9f9c1bbd68c2/content/public/test/browser_test_utils.cc [modify] https://crrev.com/08bf6de63fc34302bfb764ffe72f9f9c1bbd68c2/content/public/test/browser_test_utils.h
,
Nov 8 2017
,
Feb 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cbb1c646b962308f397996719ec263b81a1390c7 commit cbb1c646b962308f397996719ec263b81a1390c7 Author: Kevin McNee <mcnee@chromium.org> Date: Fri Feb 02 21:35:20 2018 Don't substitute GestureFlingStarts with GestureScrollEnds when bubbling We were substituting GFS events with GSE events when bubbling scroll from an OOPIF. This was done to prevent an OOPIF based guest from consuming a fling that is intended for bubbling (see crbug.com/770852 ). It seems that this can lead to an invalid input event stream. Fortunately, the main concern in that bug was with GestureScrollUpdates. The fling was a minor issue. The filtering of GSU events is still valid, so we only remove the GFS substitution. Bug: 770852 , 806940, 802085 Change-Id: I647471ea4ff94396ef6e6af21fb89e1841a0eaf7 Reviewed-on: https://chromium-review.googlesource.com/899878 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#534160} [modify] https://crrev.com/cbb1c646b962308f397996719ec263b81a1390c7/content/browser/renderer_host/render_widget_host_view_child_frame.cc [modify] https://crrev.com/cbb1c646b962308f397996719ec263b81a1390c7/content/browser/renderer_host/render_widget_host_view_child_frame.h [modify] https://crrev.com/cbb1c646b962308f397996719ec263b81a1390c7/content/browser/site_per_process_hit_test_browsertest.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by mcnee@chromium.org
, Oct 2 2017