New issue
Advanced search Search tips

Issue 713368 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocked on:
issue 526463



Sign in to add a comment

OOPIF: Make overscroll navigation work with OOPIFs

Project Member Reported by mcnee@chromium.org, Apr 19 2017

Issue description

Chrome Version: 59.0.3070.0
OS: Linux, ChromeOS, Android

What steps will reproduce the problem?
(1) With a device with a touchscreen/touchpd, visit a page with cross site iframes (e.g. http://csreis.github.io/tests/cross-site-iframe.html )
(2) Invoke overscroll behaviour starting from the iframe. On desktop, swipe left/right to invoke navigation. On Android, swipe down to refresh.

We see the following incorrect behaviour:
(1) Flings do not conclude the overscroll gesture
(2) Consumed scroll updates in the child do not prevent the activation of an overscroll gesture
(3) After an overscroll gesture starts, the child can still consume scroll updates

This is similar to  crbug.com/694393  but with OOPIFs instead of plugins.

 

Comment 1 by mcnee@chromium.org, Apr 20 2017

Labels: OS-Mac OS-Windows
The description applies to Windows as well.
On Mac with a trackpad, the overscroll doesn't activate at all.

Comment 2 by mcnee@chromium.org, May 4 2017

OOPIF overscroll issues are mainly caused by a difference in scroll bubbling behaviour between OOPIF and non-OOPIF.

Note that we're now bubbling flings as of https://chromium.googlesource.com/chromium/src/+/cff98375334e29507d061fd93d4e9146c785f43f so overscroll gestures are now concluded by flings.

With non-OOPIF, if a child consumes the first scroll update, no bubbling is done. Matching this behaviour in OOPIF should fix problem (2).

What we'll need to do in addition is allow the parent to filter the child's GestureScrollUpdates to fix problem (3).
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80e0a45861242fe3f9a4b1b1e909029f0513001f

commit 80e0a45861242fe3f9a4b1b1e909029f0513001f
Author: Kevin McNee <mcnee@chromium.org>
Date: Wed Aug 09 14:42:18 2017

Prevent child frames from consuming scrolls during overscroll navigation.

Unconsumed scrolls in a child may bubble to the root and start an
overscroll gesture (e.g. aura overscroll nav and android
pull-to-refresh). During the overscroll gesture, the child is still the
target for the scroll updates, so if the user changes direction, the
child may start consuming the updates. This results in the child
scrolling during an overscroll gesture in the root.

We now allow the root to consume the child's scroll updates if it is in
an overscroll gesture.

Bug:  713368 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I4d6f32d9ebb5cad989a3025bf2142d4c65cbfaae
Reviewed-on: https://chromium-review.googlesource.com/594787
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492974}
[modify] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/browser/frame_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/test/BUILD.gn
[add] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/test/data/wide_page.html
[add] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/test/mock_overscroll_controller_delegate_aura.cc
[add] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/test/mock_overscroll_controller_delegate_aura.h
[add] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/test/mock_overscroll_observer.h
[add] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/test/mock_overscroll_refresh_handler_android.cc
[add] https://crrev.com/80e0a45861242fe3f9a4b1b1e909029f0513001f/content/test/mock_overscroll_refresh_handler_android.h

Comment 4 by mcnee@chromium.org, Sep 13 2017

Blockedon: 526463
Incorrect behaviour (2) in the description is still an issue. This will be fixed by scroll latching once it lands ( crbug.com/526463 ).

In the meantime, it's possible to fix the behaviour without latching for touchscreen scrolling, and I have a CL up for review ( https://chromium-review.googlesource.com/c/chromium/src/+/656039 ). Unfortunately, this fix does not apply to trackpad scrolling.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14edd47ea4d6d5a4063ff0ab186f7b88c58b4471

commit 14edd47ea4d6d5a4063ff0ab186f7b88c58b4471
Author: Kevin McNee <mcnee@chromium.org>
Date: Tue Oct 03 16:56:28 2017

Do not bubble scroll if child frame has already consumed scroll.

A child may consume scroll and then stop consuming once it hits the
extent. When scroll-latching is disabled, we start bubbling these
unconsumed scroll events to the parent. We should not bubble until
the user starts a new gesture scroll.

Once scroll-latching lands, it will have the correct behaviour. In the
meantime, we change the non-scroll-latching code path to use the first
GestureScrollUpdate in a gesture scroll to determine whether to bubble.
However, this only fixes the issue for touch scrolling due to the wrapping
of GSUs in GSB/GSE pairs for wheel scrolling.

Bug:  713368 
Change-Id: I89f72c1d91555ad9e008d3a6e0b3b919c810823f
Reviewed-on: https://chromium-review.googlesource.com/656039
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506073}
[modify] https://crrev.com/14edd47ea4d6d5a4063ff0ab186f7b88c58b4471/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/14edd47ea4d6d5a4063ff0ab186f7b88c58b4471/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/14edd47ea4d6d5a4063ff0ab186f7b88c58b4471/content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc

Comment 6 by mcnee@chromium.org, Mar 12 2018

Status: Fixed (was: Started)
We now have touchpad scroll latching (crbug.com/526463), so this is now done.

Sign in to add a comment