Smooth in Dev 58.0.3029.11

Also smooth in Canary incognito window.

still happening in 59.0.3041.0 it's really easy to repro.

candrada@, can we try to see if we can repro it?  I don't see any jank on a regular Pixel using 59.0.3040.0 FWIW but I trust that wfh@ is filing good bugs so maybe a Pixel XL is required.

I can only repro when logged into reddit... and when logged in, it does repro in incognito...

I couldn't repro with Nexus 5X on 59.0.3042.3.

still getting this in 59.0.3042.3.

you have to be logged into reddit.

I can also repro on beta M57 57.0.2987.97 but not on stable M56 56.0.2924.87.

I can repro on Pixel XL / NOF27B and am logged into reddit.

It's easy to see if you hold your finger down and drag up and down to scroll. 
 Repros on M59, M58, and M57.  M56 has some jank too, but it isn't as bad as the other versions.

I'll attach video and logs soon.

Logs and video at go/chrome-androidlogs1/7/701355.

I could repro janky scrolling on Pixel-NOF26W and slow/ obstructed scroll on fling scroll on Samsung galaxy S7 / MMB29K.
But the issue reproed only after sign-in to reddit.com. It is fine when not signed-in.

I also tried nytimes desktop site with and without signing in to the site. Did not see any jank issues.

I can repro on 57.0.2987.97.  Scrolling at something like 5 FPS.

No repro on 56.0.2924.87.  Can test-team please exactly bisect?

Good build: 57.0.2973.0
Bad build: 57.0.2974.0
Regression range: https://chromium.googlesource.com/chromium/src/+log/57.0.2973.0..57.0.2974.0?pretty=fuller&n=10000

Good commit: 441953
Bad commit: 441954
Culprit CL: https://chromium.googlesource.com/chromium/src/+/c302a98ceb085bf25d0c004e666d64fd4c3ee9f6

According to this trace, we're indeed scrolling on the main thread, where it's competing with RAF and IntersectionObserver work.

https://codereview.chromium.org/2613743002 "Clip related property and border radius are now recorded for UMA" is intended to be no-op and I can't see any reason from inspecting the control flow why it might not be no-op.  But we do have an exact bisect pointing to it.  The bug is probably fairly subtle and related to how code elsewhere reacts to the new enum values.  yigu@, please investigate and fix it for M58.

Deleted: trace_reddit_loggedin_mainthreadscrolling.json 7.0 MB

trace_reddit_loggedin_mainthreadscrolling.json 7.0 MB View Download

This bug is indeed caused by my previous patch. Reddit would create something upon login that causes main thread scrolling but not without login. The current logic of recording main thread scrolling reasons may have performance issues on android devices. Start to work on a fix.

Looks like this patch caused us to use the reasons for an overflow div to scroll on main as main frame scroll reasons for the frame as well, see a demo I put together at http://output.jsbin.com/nacevox/quiet

The slow overflow scroll div should scroll on main (and probably the iframe within since the scroll can chain to the div) but not the main frame.

 Issue 693527  has been merged into this issue.

Per chatting with devs, this will affect all platforms.

Tentatively tagging M57 for tracking.

 Issue 685182  has been merged into this issue.

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

commit d528436cf0ab46778ccd56a439a250e41113fc46
Author: yigu <yigu@chromium.org>
Date: Tue Mar 21 23:09:25 2017

Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame scroll on main if there were any
scrollers which needed to scroll on main which it shouldn't. It also caused 34.5% regression in
smoothness.key_mobile_sites_smooth benchmark ( crbug.com/693527 ) .As the issue is blocking
release of M57 stable, it's better to remove the recording logic for now and work
on a correct one in a following patch.

BUG= 701355 

Review-Url: https://codereview.chromium.org/2766893002
Cr-Commit-Position: refs/heads/master@{#458596}

[modify] https://crrev.com/d528436cf0ab46778ccd56a439a250e41113fc46/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/d528436cf0ab46778ccd56a439a250e41113fc46/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp

Let's get this patch back in M57 (branch 2987) and M58 (branch 3029).

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

commit 493ca6a37c65b8c751fc5e50a00b0126b10758a8
Author: yigu <yigu@chromium.org>
Date: Wed Mar 22 18:50:53 2017

Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame scroll on main if there were any
scrollers which needed to scroll on main which it shouldn't. It also caused 34.5% regression in
smoothness.key_mobile_sites_smooth benchmark ( crbug.com/693527 ) .As the issue is blocking
release of M57 stable, it's better to remove the recording logic for now and work
on a correct one in a following patch.

BUG= 701355 
TBR=pdr@chromium.org
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2766893002
Cr-Commit-Position: refs/heads/master@{#458596}
(cherry picked from commit d528436cf0ab46778ccd56a439a250e41113fc46)

Review-Url: https://codereview.chromium.org/2769763002
Cr-Commit-Position: refs/branch-heads/3029@{#366}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/493ca6a37c65b8c751fc5e50a00b0126b10758a8/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/493ca6a37c65b8c751fc5e50a00b0126b10758a8/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp

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

commit c8352fb469276b9d2d1bcef3fc93f202aa5c2b75
Author: yigu <yigu@chromium.org>
Date: Wed Mar 22 19:07:44 2017

Remove logic about recording style related main thread scroll reasons

The current recording incorrectly propagates the style related main thread scrolling reasons
of a scrollable overflow element to the main frame. It would make the main frame scroll on main if there were any
scrollers which needed to scroll on main which it shouldn't. It also caused 34.5% regression in
smoothness.key_mobile_sites_smooth benchmark ( crbug.com/693527 ) .As the issue is blocking
release of M57 stable, it's better to remove the recording logic for now and work
on a correct one in a following patch.

BUG= 701355 
TBR=pdr@chromium.org
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2766893002
Cr-Commit-Position: refs/heads/master@{#458596}
(cherry picked from commit d528436cf0ab46778ccd56a439a250e41113fc46)

Review-Url: https://codereview.chromium.org/2766353002
Cr-Commit-Position: refs/branch-heads/2987@{#864}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/c8352fb469276b9d2d1bcef3fc93f202aa5c2b75/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/c8352fb469276b9d2d1bcef3fc93f202aa5c2b75/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp

Marking this as fixed given the revert has landed on all release branches and trunk.  Please ensure we file a new bug (or reopen the original bug tracking this work) to finalize follow-up.

Tested this issue on Windows-10, Ubuntu 14.04 and Mac OS 10.12 chrome latest stable #57.0.2987.123. By opening the provided link from original comment #1 observed the scroll is smooth and no janky scrolling is seen. 

I have tested the same on previous stable #57.0.2987.110 and the reported version of chrome #59.0.3040.0 and I am unable to reproduce this issue.

yigu@ Could you please let us know is there any other test case available to verify this issue in desktop from Chrome-TE end.

Thanks!

Issue is fixed in latest M57 on andorid. No janky scrolling is seen.

brajkumar@ Please see flackr@'s comment #21 with explanation and an example. In that example, there are 3 scrollable regions and scrolling on main frame is expected to be smooth. Scrolling on the other 2 regions are supposed to be janky.

The fix was initially landed in 57.0.2987.123 and I believed that it has not yet been pushed out to stable according to https://omahaproxy.appspot.com/.

BTW, to reproduce the problem using the provided reddit page from comment #1, you have to login.

The fix is also landed in 59.0.3049.0 and 58.0.3029.34.

Verified on CrOS 9202.56.1/ Chrome 57.0.2987.123 - Peppy

Thank you for fixing the bug. 
Requesting postmortem for this please see go/chrome-postmortems for the process to follow).

Verified the bug on Chrome version 57.0.2987.123 by logging into  reddit thread provided in bug report and the test page in Comment#21,and haven't seen any janky ness  on Windows 7,10, Mac OSX 10.12.3 and Linux(ubuntu 14.04Lts)

Verified with chrome stable '57.0.2987.126' on Samsung Galaxy S5/MMB29M

yigu@, have you been able to start a postmortem as requested in c#37?  Please share it with me when you get a chance, before the end of this week if possible.

Working on that. Will share it with you by Thursday EOD.

Verified the issue on Mac 10.12.4, Win 10 and Ubuntu 14.04 using 58.0.3029.41 and its working fine.

Note: Was able to reproduce the issue on 57.0.2987.110.

Per comment 20 "Reddit would create something upon login that causes main thread scrolling but not without login"

Do we know what the "something" that was created after reddit login?

After login, it creates a form for comments that has the property "border-radius: 5px;" which caused the slow scroll on the page.