New issue
Advanced search Search tips

Issue 826746 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Touch action regions incorrectly scroll and become invalid

Project Member Reported by pdr@chromium.org, Mar 28 2018

Issue description

Chrome Version: 67.0.3381.2
OS: Android

What steps will reproduce the problem?
(1) Visit http://jsbin.com/telivuw/2/quiet
(2) Notice that touching on the pink area does not scroll.
(3) Scroll the area up a bit.
(4) Notice that touching on the pink area does scroll.

I think our touch action regions are being stored on the scrolling contents layer but should be stored on an unscrolled layer.

 
Cc: pnangunoori@chromium.org
Labels: -Type-Bug hasbisect-per-revision Target-67 RegressedIn-64 FoundIn-67 M-65 M-66 M-67 FoundIn-66 Target-66 Target-65 FoundIn-65 Type-Bug-Regression
Owner: dtapu...@chromium.org
Status: Assigned (was: Untriaged)
Tested the issue in Android and able to reproduce the issue. 

Steps Followed:
1. Launch Chrome.
2. Navigate to any URL which is long. Eg.: http://jsbin.com/telivuw/2/quiet
3. Try scrolling on the page by tapping on pink box, observed that scrolling can't be done.
4. Scroll the area a bit (Up to middle of 'b' character above pink box).
5. Now try scrolling by tapping on pink box.
6. Observed that user is able to scroll now.

Chrome versions tested:
65.0.3325.109(Stable), 67.0.3382.0(Canary)

OS:
Android 8.1.0

Android Devices:
Pixel

Using the per-revision bisect providing the bisect results,
Good Build - 64.0.3279.0 (519518)
Bad Build - 64.0.3281.0 (520391)

You are looking for a change made after 519995(GOOD), but before 519996(BAD).

CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
https://chromium.googlesource.com/chromium/src/+/ff7f1749d10b1fce53591cae122639cf17848e53

From the CL above, assigning the issue to the owner concerned.

@dtapuska:  Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to owner concerned.

Please navigate to below link for log's and video--
go/chrome-androidlogs/826746



Labels: Needs-triage-Mobile Triaged-Mobile
Cc: flackr@chromium.org xidac...@chromium.org
If you apply a will-change: transform to the touch-action area then it works correctly.

The bisect is correct that it started reproducing around when mojo enabled. But the real problem is that the slow hit test regions aren't being calculated correctly. It broke because mojo is faster and by the time the browser got the ack the touch-start ack was already processed.

If you bisect with mojo disabled you'll find it works for chrome ipc up until: 
https://chromium.googlesource.com/chromium/src/+log/7bfb139d180543d9e4402dfcc7bedb79cf9c4872..1957d0b533a7d46f88d4c2809c561873cbf78ddc

which drops touch-actions if a touch-start is not in the pending ack queue. That means that it is a race.

Debugging the HandleTouchStart in the InputHandlerProxy shows that the regions aren't hit test correctly.

Cc: dtapu...@chromium.org
Owner: xidac...@chromium.org
The hit test on the compositor side doesn't find them inside the scrolling rects. Is the scroll offset being applied correctly in these transforms?
Has a discussion with pdr@, in this particular example, the problem is that we store touch-action regions on the container layer instead of the scrolling content layer, so we won't be able to account for the scroll offset.

Unfortunately, our current code architecture takes a map of PaintLayer and Rect in SetTouchEventTargetRects, and that is not enough to decide whether the touch action region should be stored on the container layer or the scrolling content layer.

pdr@ did mention that there are some proposal to re-write the logic in that area, and that should be able to solve this problem.
Owner: ----
Status: Available (was: Assigned)
Can some one from dev team, please take a look at this issue?

Thanks!!
Owner: xidac...@chromium.org
Status: Assigned (was: Available)
Status: Fixed (was: Assigned)
PaintTouchActionRects has shipped which fixes this. ScrollingCoordinatorTest.touchAction is the unit test for this case.ScrollingCoordinatorTest.touchAction
Labels: -Pri-3 Pri-2
Status: Assigned (was: Fixed)
Re-open because we revert the ship of PaintTouchActionRects.
Status: Fixed (was: Assigned)

Sign in to add a comment