1459776 - Audit nsIFrame usage for animations

Status: NEW

(Core :: DOM: Animation, enhancement, P3)

Description

[Hiroyuki Ikezoe (:hiro)]

In bug 1320608, I did fix our mismatch nsIFrame handling for table element.  But I suspect there are still mis-usage, we should audit all of them.  Now I highly doubt that we do properly handle mMayHaveTransformAnimation and mMayHaveOpacityAnimation flags in nsIFrame.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Depending on bug 1463605, since I am going to fix some of mis-usage in the bug.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

|aFrame->RefusedAsyncAnimation()| in FindAnimationsForCompositor() [1] looks a bit error-prone.  I think it's not a problem so far, but for continuation frames, nsDisplayOpacity has the last continuation as |mFrame|, so FindAnimationsForCompositor() is called for the last one, whereas in animation code we consider |nsIFrame*| is the primary frame basically, so it might be a matter that we will mis-use there.  We should add a comment there.


[1] https://hg.mozilla.org/mozilla-central/file/da28b92efe6f/dom/animation/EffectCompositor.cpp#l155

Comment 3

[Brian Birtles (:birtles)]

There is at least one bug here. See this test case: https://codepen.io/birtles/pen/LqmqYY

If we drop the 'display: table' line the stacking order changes. In Chrome it does not.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

(In reply to Brian Birtles (:birtles, away Feb 8, 11, 18) from comment #3)

There is at least one bug here. See this test case: https://codepen.io/birtles/pen/LqmqYY

If we drop the 'display: table' line the stacking order changes. In Chrome it does not.

The relevant place is here;
https://searchfox.org/mozilla-central/rev/00c0d068ece99717bea7475f7dc07e61f7f35984/layout/generic/nsFrame.cpp#2791

IIRC, the nsIFrame owned by the display item is not the style frame for table element.

CCing Boris, he might have a chance to fix them along with bug 1526850.