Skip to content

GitLab

T
tangled
Commit 59ec1d86 authored by Stephen McGruer's avatar Stephen McGruer Committed by Commit Bot
Browse files
Options

Remove DCHECK for ScrollTree containment in ScrollTimeline::CurrentTime 

The original assertion that if a ScrollTimeline had a non-null scroller
id (either pending or active) then said id would be in the ScrollTree
was actually false. There are cases where the ScrollTimeline can have a
scroller id, but the scrolling element would not be in the ScrollTree -
for example a composited overflow: visible element would meet this
criteria.

As such, this CL converts the DCHECK into an early-exit for this case.

Bug: 853231
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I0ca14a8e8f516357ee9814388514ba2f72fdfd47
Reviewed-on: https://chromium-review.googlesource.com/1133317
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574284}
parent 9b959230
Hide whitespace changes
Inline Side-by-side
Showing with 127 additions and 4 deletions
cc/animation/scroll_timeline.cc
View file @ 59ec1d86
......@@ -28,15 +28,20 @@ std::unique_ptr<ScrollTimeline> ScrollTimeline::CreateImplInstance() const {
double ScrollTimeline::CurrentTime(const ScrollTree& scroll_tree,
bool is_active_tree) const {
// If the scroller isn't in the ScrollTree, the element either no longer
// exists or is not currently scrollable. By the spec, return an unresolved
// time value.
// We may be asked for the CurrentTime before the pending tree with our
// scroller has been activated, or after the scroller has been removed (e.g.
// if it is no longer composited). In these cases the best we can do is to
// return an unresolved time value.
if ((is_active_tree && !active_id_) || (!is_active_tree && !pending_id_))
return std::numeric_limits<double>::quiet_NaN();
ElementId scroller_id =
is_active_tree ? active_id_.value() : pending_id_.value();
DCHECK(scroll_tree.FindNodeFromElementId(scroller_id));
// The scroller may not be in the ScrollTree if it is not currently scrollable
// (e.g. has overflow: visible). By the spec, return an unresolved time value.
if (!scroll_tree.FindNodeFromElementId(scroller_id))
return std::numeric_limits<double>::quiet_NaN();
gfx::ScrollOffset offset = scroll_tree.current_scroll_offset(scroller_id);
DCHECK_GE(offset.x(), 0);
......
third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/animation-worklet-scroll-timeline-non-scrollable-expected.html 0 → 100644
View file @ 59ec1d86
<!DOCTYPE html>
<style>
#box {
width: 100px;
height: 100px;
background-color: #00ff00;
transform: translate(0, 100px);
/* Force compositing. */
backface-visibility: hidden;
}
#covered {
width: 100px;
height: 100px;
background-color: #ff8080;
}
#scroller {
overflow: visible;
height: 100px;
width: 100px;
/* Force compositing of the scroller. */
backface-visibility: hidden;
}
#contents {
height: 1000px;
width: 100%;
}
</style>
<div id="box"></div>
<div id="covered"></div>
<div id="scroller">
<div id="contents"></div>
</div>
third_party/WebKit/LayoutTests/virtual/threaded/fast/animationworklet/animation-worklet-scroll-timeline-non-scrollable.html 0 → 100644
View file @ 59ec1d86
<!DOCTYPE html>
<style>
#box {
width: 100px;
height: 100px;
background-color: #00ff00;
}
#covered {
width: 100px;
height: 100px;
background-color: #ff8080;
}
/* In this test the scroller is composited (forcibly) but is not actually
scrollable. This should result in a composited worklet animation being
created, but it should get a NaN currentTime. */
.scroller {
overflow: visible;
height: 100px;
width: 100px;
/* Force compositing of the scroller. */
backface-visibility: hidden;
}
#contents {
height: 1000px;
}
</style>
<div id="box"></div>
<div id="covered"></div>
<div id="scroller" class="scroller">
<div id="contents"></div>
</div>
<script id="visual_update" type="text/worklet">
registerAnimator("test_animator", class {
animate(currentTime, effect) {
// A non-scrollable scroller should result in a NaN currentTime; set the
// localTime accordingly so we can spot this.
if (isNaN(currentTime)) {
effect.localTime = 500;
} else {
effect.localTime = 1000;
}
}
});
</script>
<script src="resources/animation-worklet-tests.js"></script>
<script>
if (window.testRunner) {
testRunner.waitUntilDone();
}
runInAnimationWorklet(
document.getElementById('visual_update').textContent
).then(()=>{
const box = document.getElementById('box');
const effect = new KeyframeEffect(box,
[
{ transform: 'translateY(0)' },
{ transform: 'translateY(200px)' }
], {
duration: 1000,
}
);
const scroller = document.getElementById('scroller');
const timeline = new ScrollTimeline({ scrollSource: scroller, timeRange: 1000, orientation: 'block' });
const animation = new WorkletAnimation('test_animator', effect, timeline);
animation.play();
// Ensure that the WorkletAnimation will have been started on the compositor.
if (window.testRunner) {
waitTwoAnimationFrames(_ => {
testRunner.notifyDone();
});
}
});
</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