Bug 1523229 - Don't reference animations from AnimationTimeline just because they're relevant; r=hiro

I don't know why this check was ever added. A comment here suggests we expected
irrelevant animations to be removed from their timeline:

  https://hg.mozilla.org/mozilla-central/rev/8406c5300ab7051ae6fe9bf41a1d30261cf70a4a#l2.16

Furthermore, a comment in the changeset description for that same changeset
suggests that to be the case:

  "For example, if a CSS animation is finished (IsRelevant() == false so that
  animation will have been removed from the timeline)"

Another comment removed in that patch has:

  "Note that we only store relevant animations on the timeline since they are
  the only ones that need ticks and are the only ones returned from
  AnimationTimeline::GetAnimations"

which suggests it was added a point when we had a GetAnimations() method on
AnimationTimeline and hence it was needed for that.

The other possibility is that we were preempting a point when timelines would
switch between being active and inactive:

  "FIXME: Once we expect animations to go back and forth betweeen being inactive
  and active, we will need to store more than just relevant animations on the
  timeline. This is because an animation might be deemed irrelevant because its
  timeline is inactive. If it is removed from the timeline at that point the
  timeline will have no way of getting the animation to add itself again once it
  becomes active."

Indeed, we might need this for ScrollTimelines. For now, however, it seems
unnecessary (a try run with simply this check removed passes all test).

(Furthermore, in bug 1253476 or one of its dependencies, this check will prevent
us from combining filling animations since the original (filling) animations
will be kept alive by the timeline. Should this indeed prove necessary for bug
1253476, that bug will add an automated test that will fail if we re-introduce
this condition.)

Differential Revision: https://phabricator.services.mozilla.com/D17798

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Brian Birtles 2019-01-28 08:04:24 +00:00
parent 920f6b20e3
commit 068c417c60

View File

@ -166,7 +166,7 @@ void DocumentTimeline::MostRecentRefreshTimeUpdated() {
// order to dispatch events.
animation->Tick();
if (!animation->IsRelevant() && !animation->NeedsTicks()) {
if (!animation->NeedsTicks()) {
animationsToRemove.AppendElement(animation);
}
}