1108690 - fix TODO in nsDisplayTransform::CanUseAsyncAnimations

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix this TODO:

nsDisplayTransform::CanUseAsyncAnimations(nsDisplayListBuilder* aBuilder)
{
  if (mMaybePrerender) {
    // TODO We need to make sure that if we use async animation we actually
    // pre-render even if we're out of will change budget.
    return true;
  }

Comment 1

[Benoit Girard (:BenWa)]

I believe this is true as it stands. Adding an assert should be sufficient to make sure we never break this. I wrote this because with the MaybePrerender stuff it seems to easy for forget about this.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think I might have understood the implications of this TODO back when I filed the bug, but I don't right now.

It's not clear to me that there's a problem:  nsDisplayTransform::ShouldPrerender checks ShouldPrerenderTransformedContent (which we assert is false in nsDisplayTransform::CanUseAsyncAnimations if we're going to return no) separately from its check of the will-change budget.

I'm not inclined to think it blocks shipping async animations right now.

Comment 3

[Benoit Girard (:BenWa)]

I think my intention with this TODO was just to make sure that this function is returning the correct thing:

The correctness of this function relies on that fact that we need to set mMaybePrerender to true if we might use async animation for this element otherwise it will give the wrong result.

Like you said 'ShouldPrerenderTransformedContent' should handle that.