Commit 63fed693 authored by Vladimir Levin's avatar Vladimir Levin Committed by Commit Bot

content-visibility: Ensure to keep the DL unlocked if targeted by scroll

This patch generalizes the find-in-page fix we had previously to also
apply to other methods that can use to scroll the viewport to the
element in a locked subtree. Specifically, this keep the lock unlocked
for two frames to ensure that the visibility notification kicks in to
keep it unlocked until the user scrolls away.

R=chrishtr@chromium.org

Bug: 1132869
Change-Id: I598f82f73429e41839a6675321eb6fb1eb84f6b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2435808Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811709}
parent e4330f81
......@@ -395,14 +395,17 @@ void DisplayLockContext::CommitForActivationWithSignal(
DCHECK(IsLocked());
DCHECK(ShouldCommitForActivation(DisplayLockActivationReason::kAny));
// Find in page scrolls content into view. However, if the position of the
// target is outside of the bounds that would cause the auto-context to
// unlock, then we can scroll into wrong content while the context remains
// lock. To avoid this, unlock it until the next lifecycle. If the scroll is
// successful, then we will gain visibility anyway so the context will be
// unlocked for other reasons.
// TODO(vmpstr): See if scrollIntoView() needs this as well.
if (reason == DisplayLockActivationReason::kFindInPage) {
// The following actions (can) scroll content into view. However, if the
// position of the target is outside of the bounds that would cause the
// auto-context to unlock, then we can scroll into wrong content while the
// context remains lock. To avoid this, unlock it until the next lifecycle.
// If the scroll is successful, then we will gain visibility anyway so the
// context will be unlocked for other reasons.
if (reason == DisplayLockActivationReason::kAccessibility ||
reason == DisplayLockActivationReason::kFindInPage ||
reason == DisplayLockActivationReason::kFragmentNavigation ||
reason == DisplayLockActivationReason::kScrollIntoView ||
reason == DisplayLockActivationReason::kSimulatedClick) {
// Note that because the visibility is only determined at the _end_ of the
// next frame, we need to ensure that we stay unlocked for two frames.
SetKeepUnlockedUntilLifecycleCount(2);
......
......@@ -69,7 +69,7 @@ async_test((t) => {
t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "frame 2 offscreen"));
requestAnimationFrame(() => {
offscreen_container.scrollIntoView();
window.scrollBy(0, 10000);
// Frame 3 checks:
t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "frame 3"));
......
......@@ -35,7 +35,7 @@
</style>
<div class=spacer></div>
<div id=container class="container size_contained">
<div id=container class="container">
<div class=child></div>
<div id=target></div>
</div>
......@@ -45,7 +45,6 @@
function runReference() {
document.getElementById("target").scrollIntoView(true /* alignToTop */);
document.getElementById("container").classList.remove("size_contained");
requestAnimationFrame(takeScreenshot);
}
......
<!doctype HTML>
<html class="reftest-wait">
<meta charset="utf8">
<title>CSS Content Visibility: auto + scrollIntoView/fragment nav when size estimate is off (reference)</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
<script src="/common/reftest-wait.js"></script>
<style>
.small_child {
height: 500px;
}
.large_child {
height: 5000px;
position: relative;
}
#target {
position: absolute;
bottom: 0;
}
</style>
<div class=auto><div class=small_child></div></div>
<div class=auto><div class=small_child></div></div>
<div class=auto><div class=large_child><div id=target>target</div></div></div>
<div class=auto><div class=large_child></div></div>
<div class=auto><div class=small_child></div></div>
<script>
function runReference() {
target.scrollIntoView();
takeScreenshot();
}
window.onload = () => requestAnimationFrame(() => requestAnimationFrame(runReference));
</script>
<!doctype HTML>
<html class="reftest-wait">
<meta charset="utf8">
<title>CSS Content Visibility: auto + scrollIntoView when size estimate is off</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
<link rel="match" href="content-visibility-075-ref.html">
<meta name="assert" content="With content-visibility: auto, scrollIntoView targets the right element">
<script src="/common/reftest-wait.js"></script>
<style>
.auto {
content-visibility: auto;
contain-intrinsic-size: 1px 500px;
}
.child {
height: 5000px;
position: relative;
}
#target {
position: absolute;
bottom: 0;
}
</style>
<div class=auto><div class=child></div></div>
<div class=auto><div class=child></div></div>
<div class=auto><div class=child><div id=target>target</div></div></div>
<div class=auto><div class=child></div></div>
<div class=auto><div class=child></div></div>
<script>
function runTest() {
target.scrollIntoView();
// Double rAF to ensure that rendering has "settled".
requestAnimationFrame(() => requestAnimationFrame(takeScreenshot));
}
window.onload = () => requestAnimationFrame(() => requestAnimationFrame(runTest));
</script>
<!doctype HTML>
<html class="reftest-wait">
<meta charset="utf8">
<title>CSS Content Visibility: auto + scorllIntoView when size estimate is off (reference)</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
<script src="/common/reftest-wait.js"></script>
<style>
.small_child {
height: 500px;
}
.large_child {
height: 5000px;
position: relative;
}
#target {
position: absolute;
bottom: 0;
}
</style>
<div class=auto><div class=small_child></div></div>
<div class=auto><div class=small_child></div></div>
<div class=auto><div class=large_child><div id=target>target</div></div></div>
<div class=auto><div class=large_child></div></div>
<div class=auto><div class=small_child></div></div>
<script>
function runReference() {
target.scrollIntoView();
takeScreenshot();
}
window.onload = () => requestAnimationFrame(() => requestAnimationFrame(runReference));
</script>
<!doctype HTML>
<html class="reftest-wait">
<meta charset="utf8">
<title>CSS Content Visibility: auto + fragment nav when size estimate is off</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
<link rel="match" href="content-visibility-075-ref.html">
<meta name="assert" content="With content-visibility: auto, scrollIntoView targets the right element">
<script src="/common/reftest-wait.js"></script>
<style>
.auto {
content-visibility: auto;
contain-intrinsic-size: 1px 500px;
}
.child {
height: 5000px;
position: relative;
}
#target {
position: absolute;
bottom: 0;
}
</style>
<div class=auto><div class=child></div></div>
<div class=auto><div class=child></div></div>
<div class=auto><div class=child><div id=target>target</div></div></div>
<div class=auto><div class=child></div></div>
<div class=auto><div class=child></div></div>
<script>
function runTest() {
window.location.href += "#target";
// Double rAF to ensure that rendering has "settled".
requestAnimationFrame(() => requestAnimationFrame(takeScreenshot));
}
window.onload = () => requestAnimationFrame(() => requestAnimationFrame(runTest));
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment