New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 680925 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Delay is observed for opening 'Login in' overlay on 'espn.com' after first instance.

Reported by rk...@etouch.net, Jan 13 2017

Issue description

Chrome 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.
 
Actual_Overlay.mp4
1.4 MB View Download
Expected_Overlay.mp4
907 KB View Download

Comment 1 by hdodda@chromium.org, Jan 13 2017

Cc: hdodda@chromium.org
Labels: hasbisect-per-revision
Owner: skyos...@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good Build: 57.0.2950.0 (revision : 438011)
Bad Build: 57.0.2951.0 (revision : 438385)

You are probably looking for a change made after 438226 (known good), but no later than 438227 (first known 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/+log/1029d11c4d75fd95fa87181056c672f6dafe112d..1667f2eb188cac8da8dcb2fc84a8357fd94b5f61

Review-Url: https://codereview.chromium.org/2468863002

From the CL above, assigning the issue to the concern owner 

@skyostil - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thanks!
Labels: ReleaseBlock-Stable
Adding RB Label as this is a recent Regression. Please remove if not required.
Thank You.
Components: -Blink Blink>Scheduling

Comment 4 by ajha@chromium.org, Jan 19 2017

Cc: kinuko@chromium.org
Friendly ping to get an update on this issue.
Cc: mlamouri@chromium.org
Status: Started (was: Assigned)
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.)
Labels: Needs-triage-Mobile

Comment 7 by ajha@chromium.org, Jan 25 2017

Labels: OS-Android
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.

 
Project Member

Comment 8 by bugdroid1@chromium.org, 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

Labels: Merge-Request-57
Status: Fixed (was: Started)
Project Member

Comment 10 by sheriffbot@chromium.org, Jan 27 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
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
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 27 2017

Labels: -merge-approved-57 merge-merged-2987
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

Labels: -Needs-triage-Mobile TE-Verified-M57 TE-Verified-57.0.2987.19
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!

Issue 680925.mp4
1.6 MB View Download
Labels: Needs-triage-Mobile
Android: 
Issue verified on 57.0.2987.19, No delay observed while opening on Login page.
Project Member

Comment 15 by bugdroid1@chromium.org, 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