Bugzilla

So in my pernosco recording, it seems like we're inserting the same animation (actually a CSSTransition instance) into mAnimationOrder. First here:

https://searchfox.org/mozilla-central/rev/137075514eddc08c68ff652e9899da82e8043574/dom/animation/AnimationTimeline.cpp#97-102
```cpp
void AnimationTimeline::NotifyAnimationContentVisibilityChanged(
    Animation* aAnimation, bool visible) {
  bool inList =
      static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList();
  if (visible && !inList) {
    mAnimationOrder.insertBack(aAnimation);
```

...and then here:

https://searchfox.org/mozilla-central/rev/137075514eddc08c68ff652e9899da82e8043574/dom/animation/AnimationTimeline.cpp#78,83-84
```cpp
void AnimationTimeline::NotifyAnimationUpdated(Animation& aAnimation) {
...
    if (!aAnimation.IsHiddenByContentVisibility()) {
      mAnimationOrder.insertBack(&aAnimation);
```

So we're not quite managing that list properly.  Notably the first snippet there (in `NotifyAnimationContentVisibilityChanged`) has a `inList` check to avoid redundant insertion, but the second snippet (in `NotifyAnimationUpdated`) does not. Presumably we could just add the same check, to avoid this issue, but I'm not sure yet whether the lack-of-that-check here was just an oversight vs. an unstated invariant that we're expecting to maintain.

So in my pernosco recording, it seems like we're inserting the same animation (actually a CSSTransition instance) into the same list, via `mAnimationOrder.insertBack(...)`. First here:

https://searchfox.org/mozilla-central/rev/137075514eddc08c68ff652e9899da82e8043574/dom/animation/AnimationTimeline.cpp#97-102
```cpp
void AnimationTimeline::NotifyAnimationContentVisibilityChanged(
    Animation* aAnimation, bool visible) {
  bool inList =
      static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList();
  if (visible && !inList) {
    mAnimationOrder.insertBack(aAnimation);
```

...and then here:

https://searchfox.org/mozilla-central/rev/137075514eddc08c68ff652e9899da82e8043574/dom/animation/AnimationTimeline.cpp#78,83-84
```cpp
void AnimationTimeline::NotifyAnimationUpdated(Animation& aAnimation) {
...
    if (!aAnimation.IsHiddenByContentVisibility()) {
      mAnimationOrder.insertBack(&aAnimation);
```

So we're not quite managing that list properly.  Notably the first snippet there (in `NotifyAnimationContentVisibilityChanged`) has a `inList` check to avoid redundant insertion, but the second snippet (in `NotifyAnimationUpdated`) does not. Presumably we could just add the same check, to avoid this issue, but I'm not sure yet whether the lack-of-that-check here was just an oversight vs. an unstated invariant that we're expecting to maintain.

So in my pernosco recording, it seems like we're inserting the same animation (actually a CSSTransition instance) into the same list, via `mAnimationOrder.insertBack(...)`. First here:

https://searchfox.org/mozilla-central/rev/137075514eddc08c68ff652e9899da82e8043574/dom/animation/AnimationTimeline.cpp#97-102
```cpp
void AnimationTimeline::NotifyAnimationContentVisibilityChanged(
    Animation* aAnimation, bool visible) {
  bool inList =
      static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList();
  if (visible && !inList) {
    mAnimationOrder.insertBack(aAnimation);
```

...and then here:

https://searchfox.org/mozilla-central/rev/137075514eddc08c68ff652e9899da82e8043574/dom/animation/AnimationTimeline.cpp#78-79,83-84
```cpp
void AnimationTimeline::NotifyAnimationUpdated(Animation& aAnimation) {
  if (mAnimations.EnsureInserted(&aAnimation)) {
...
    if (!aAnimation.IsHiddenByContentVisibility()) {
      mAnimationOrder.insertBack(&aAnimation);
```

So we're not quite managing that list properly.  Notably the first snippet there (in `NotifyAnimationContentVisibilityChanged`) has a `inList` check to avoid redundant insertion, but the second snippet (in `NotifyAnimationUpdated`) does not. Presumably we could just add the same check, to avoid this issue, but I'm not sure yet whether the lack-of-that-check here was just an oversight vs. an unstated invariant that we're expecting to maintain.