Regression: Delay is observed for opening 'Login in' overlay on 'espn.com' after first instance.
Reported by
rk...@etouch.net,
Jan 13 2017
|
||||||||||||
Issue descriptionChrome Version: 57.0.2980.0 (Official Build)6770bab14919fc9abed1d457387824e84cc43602-refs/heads/master@{#443474} 32/64-bit. OS: Windows(7,8,10), Mac (10.11.6, 10.12.1), Linux (14.04 LTS). What steps will reproduce the problem? (1) Launch chrome, navigate to http://www.espn.in/?src=com (2) Click on Login in button placed at left side of page. (3) Observe Actual: Delay is observed for opening 'Login in' overlay after first instance. Expected: Delay should not observe for opening Login in overlay. This is a regression issue, broken in 'M-57', will soon update the other info: Good Build: 57.0.2950.0 Bad Build: 57.0.2951.0 Note: On Mac OS, Login in overlay does not get open.
,
Jan 13 2017
Adding RB Label as this is a recent Regression. Please remove if not required. Thank You.
,
Jan 13 2017
,
Jan 19 2017
Friendly ping to get an update on this issue.
,
Jan 20 2017
Looks like by switching to ElementVisibilityObserver we've accidentally started throttling display:none frames. I don't think this is safe to do, so I'll figure out a way to work around it. (I tried a hacky way and it makes ESPN work again.)
,
Jan 20 2017
,
Jan 25 2017
Just to update regarding the added 'Needs-triage-Mobile' label on the reported version: 57.0.2980.0. Issue is reproducible on Samsung Galaxy S5 Handset, Android Version: 5.0.0; SM-G900H Build/LRX21T and Galaxy Tab S2, Android Version: 6.0.1; SM-T815Y Build/MMB29K. Layout from Desktop is different but clicking on 'Login' button delay is observed.
,
Jan 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/02d1c14d47b948cf6b4eb49c3b4cef697525f03e commit 02d1c14d47b948cf6b4eb49c3b4cef697525f03e Author: skyostil <skyostil@chromium.org> Date: Thu Jan 26 20:08:04 2017 FrameView: Don't throttle display:none frames While migrating to use IntersectionObserver to control frame throttling we accidentally also started throttling display:none frames (which IObs. considers invisible). This is not safe because many sites use such frames for driving UI logic. This patch disallows throttling for display:none frames. The revised logic also matches the frame throttling implementation in Safari. BUG= 680925 TEST=Open the login popup twice on http://www.espn.in/ and observe no delay. Review-Url: https://codereview.chromium.org/2658063002 Cr-Commit-Position: refs/heads/master@{#446419} [modify] https://crrev.com/02d1c14d47b948cf6b4eb49c3b4cef697525f03e/third_party/WebKit/Source/core/frame/FrameView.cpp [modify] https://crrev.com/02d1c14d47b948cf6b4eb49c3b4cef697525f03e/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp
,
Jan 27 2017
,
Jan 27 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/613b3cabab9f93e9c01962c327239b2f6c81d9c5 commit 613b3cabab9f93e9c01962c327239b2f6c81d9c5 Author: Sami Kyostila <skyostil@chromium.org> Date: Fri Jan 27 10:42:36 2017 FrameView: Don't throttle display:none frames While migrating to use IntersectionObserver to control frame throttling we accidentally also started throttling display:none frames (which IObs. considers invisible). This is not safe because many sites use such frames for driving UI logic. This patch disallows throttling for display:none frames. The revised logic also matches the frame throttling implementation in Safari. BUG= 680925 TEST=Open the login popup twice on http://www.espn.in/ and observe no delay. Review-Url: https://codereview.chromium.org/2658063002 Cr-Commit-Position: refs/heads/master@{#446419} (cherry picked from commit 02d1c14d47b948cf6b4eb49c3b4cef697525f03e) Review-Url: https://codereview.chromium.org/2660633002 . Cr-Commit-Position: refs/branch-heads/2987@{#141} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/613b3cabab9f93e9c01962c327239b2f6c81d9c5/third_party/WebKit/Source/core/frame/FrameView.cpp [modify] https://crrev.com/613b3cabab9f93e9c01962c327239b2f6c81d9c5/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp
,
Jan 31 2017
Verified this issue on Windows-10, Ubuntu 14.04 and Mac OS 10.12 using chrome latest Dev M57-57.0.2987.19 by following steps mentioned in the comment #0 and observed no delay while opening Login in overlay. Hence adding TE-Verified label. Thanks!
,
Jan 31 2017
,
Jan 31 2017
Android: Issue verified on 57.0.2987.19, No delay observed while opening on Login page.
,
Feb 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6d58fe57ab739bbc26291e04bce804721ba5b93d commit 6d58fe57ab739bbc26291e04bce804721ba5b93d Author: skyostil <skyostil@chromium.org> Date: Thu Feb 23 10:53:57 2017 FrameView: Remove redundant tree walk when unthrottling display:none frames When unthrottling display:none frames, there's no need to synchronously unthrottle child frames because 1) we're about to walk the subtree anyway and 2) to match Safari's behavior we actually want to keep children of display:none frames throttled. No functional changes, but added a new test to check how children of display:none frames behave. BUG= 680925 TEST=https://jsfiddle.net/ohpkjn9p/2 Review-Url: https://codereview.chromium.org/2706193003 Cr-Commit-Position: refs/heads/master@{#452448} [modify] https://crrev.com/6d58fe57ab739bbc26291e04bce804721ba5b93d/third_party/WebKit/Source/core/frame/FrameView.cpp [modify] https://crrev.com/6d58fe57ab739bbc26291e04bce804721ba5b93d/third_party/WebKit/Source/core/frame/FrameView.h [modify] https://crrev.com/6d58fe57ab739bbc26291e04bce804721ba5b93d/third_party/WebKit/Source/web/tests/FrameThrottlingTest.cpp |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by hdodda@chromium.org
, Jan 13 2017Labels: hasbisect-per-revision
Owner: skyos...@chromium.org
Status: Assigned (was: Unconfirmed)