Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-12-01 00:32:11 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1822907 part 2: When handling a content-visibility change, don't insert already-completed animations into the timeline's sampling-order list. r=hiro

Browse Source 
We have an invariant that the mAnimationOrder LinkedList is a subset of the
mAnimations hashset (omitting any animations that are hidden due to
content-visibility).  This patch corrects one case where we were incorrectly
inserting an animation into the linked list when it wasn't present in the
hashset (because the animation had completed).

This patch also adds some documentation to mention this invariant, and some
assertions to enforce it in several places.

Differential Revision: https://phabricator.services.mozilla.com/D173333
This commit is contained in:
Daniel Holbert 2023-03-23 04:41:12 +00:00
parent 42d84e917d
commit 86fdc8655d
3 changed files with 51 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
dom/animation/AnimationTimeline.cpp
View File

@ -46,6 +46,8 @@ bool AnimationTimeline::Tick() {
for (Animation* animation = mAnimationOrder.getFirst(); animation; for (Animation* animation = mAnimationOrder.getFirst(); animation;
animation = animation =
static_cast<LinkedListElement<Animation>*>(animation)->getNext()) { static_cast<LinkedListElement<Animation>*>(animation)->getNext()) {
MOZ_ASSERT(mAnimations.Contains(animation),
"The sampling order list should be a subset of the hashset");
MOZ_ASSERT(!animation->IsHiddenByContentVisibility(), MOZ_ASSERT(!animation->IsHiddenByContentVisibility(),
"The sampling order list should not contain any animations " "The sampling order list should not contain any animations "
"that are hidden by content-visibility"); "that are hidden by content-visibility");
@ -91,6 +93,8 @@ void AnimationTimeline::NotifyAnimationUpdated(Animation& aAnimation) {
void AnimationTimeline::RemoveAnimation(Animation* aAnimation) { void AnimationTimeline::RemoveAnimation(Animation* aAnimation) {
MOZ_ASSERT(!aAnimation->GetTimeline() || aAnimation->GetTimeline() == this); MOZ_ASSERT(!aAnimation->GetTimeline() || aAnimation->GetTimeline() == this);
if (static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList()) { if (static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList()) {
MOZ_ASSERT(mAnimations.Contains(aAnimation),
"The sampling order list should be a subset of the hashset");
static_cast<LinkedListElement<Animation>*>(aAnimation)->remove(); static_cast<LinkedListElement<Animation>*>(aAnimation)->remove();
} }
mAnimations.Remove(aAnimation); mAnimations.Remove(aAnimation);
@ -100,7 +104,9 @@ void AnimationTimeline::NotifyAnimationContentVisibilityChanged(
Animation* aAnimation, bool aIsVisible) { Animation* aAnimation, bool aIsVisible) {
bool inList = bool inList =
static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList(); static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList();
if (aIsVisible && !inList) { MOZ_ASSERT(!inList || mAnimations.Contains(aAnimation),
"The sampling order list should be a subset of the hashset");
if (aIsVisible && !inList && mAnimations.Contains(aAnimation)) {
mAnimationOrder.insertBack(aAnimation); mAnimationOrder.insertBack(aAnimation);
} else if (!aIsVisible && inList) { } else if (!aIsVisible && inList) {
static_cast<LinkedListElement<Animation>*>(aAnimation)->remove(); static_cast<LinkedListElement<Animation>*>(aAnimation)->remove();

1
dom/animation/AnimationTimeline.h
View File

@ -125,6 +125,7 @@ class AnimationTimeline : public nsISupports, public nsWrapperCache {
// We store them in (a) a hashset for quick lookup, and (b) a LinkedList // We store them in (a) a hashset for quick lookup, and (b) a LinkedList
// to maintain a fixed sampling order. Animations that are hidden by // to maintain a fixed sampling order. Animations that are hidden by
// `content-visibility` are not sampled and will only be in the hashset. // `content-visibility` are not sampled and will only be in the hashset.
// The LinkedList should always be a subset of the hashset.
// //
// The hashset keeps a strong reference to each animation since // The hashset keeps a strong reference to each animation since
// dealing with addref/release with LinkedList is difficult. // dealing with addref/release with LinkedList is difficult.

43
testing/web-platform/tests/css/css-contain/content-visibility/crashtests/content-visibility-transition-finished-001.html Normal file
View File

@ -0,0 +1,43 @@
<!DOCTYPE html>
<html class="test-wait">
<meta charset="utf-8">
<link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1822907">
<style>
#inner {
/* Extremely short so that we can just do a double-rAF and
* expect that this transition will have completed: */
transition: opacity 0.01s;
}
</style>
<script>
function setup() {
// Flush style, in case the document hasn't been styled yet:
let origOpacity = getComputedStyle(inner).opacity;
// Trigger the (extremely short) transition, and proceed to next step
// when it finishes.
inner.addEventListener("transitionend", handleTransitionEnd);
inner.style.opacity = "0.8";
}
function handleTransitionEnd(e) {
// Hide & unhide the subtree that contained the transition:
foo.style.contentVisibility = "hidden";
document.documentElement.offsetHeight; // flush layout
foo.style.contentVisibility = "";
// Double-rAF to see if we crash when animations are resampled.
requestAnimationFrame(() => { requestAnimationFrame(finish) });
}
function finish() {
// If we get here, we've succeeded; hooray!
document.documentElement.removeAttribute("class");
}
</script>
<body onload="setup()">
<div id="foo">
Hello
<div id="inner">Inner</div>
</div>
</body>
</html>