1341195 - stylo: getComputedStyle() doesn't return a valid value while the animation is running on compositor

Status: RESOLVED FIXED

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

Description

[Boris Chiou [:boris]]

While running some omta test cases [1], we use window.getComputedStyle() and SpecialPowers.DOMWindowUtils.getOMTAStyle() to compare the style data in compositor with the computed value calculated on the main thread. However, window.getComputedStyle() usually return invalid value, which means we didn't do something like RuleNodeWithReplacement() to replace the animation rule (and cascade?) after calling getComputedStyle(), so we still have many test cases failed.

[1] http://searchfox.org/mozilla-central/rev/12cf11303392edac9f1da0c02e3d9ad2ecc8f4d3/layout/style/test/animation_utils.js#499-510

Comment 1

[Cameron McCormack (:heycam)]

Have you verified that it is a problem with not replacing the style rule correctly?  There were some recent changes with how we mark animation style flushes as needed in bug 1334735, so I wonder if we are not calling SetNeedThrottledAnimationFlush() in some case.

Comment 2

[Boris Chiou [:boris]]

(In reply to Cameron McCormack (:heycam) from comment #1)
> Have you verified that it is a problem with not replacing the style rule
> correctly?  There were some recent changes with how we mark animation style
> flushes as needed in bug 1334735, so I wonder if we are not calling
> SetNeedThrottledAnimationFlush() in some case.

Actually, I am still not sure the root cause, just guess. I can try your suggestion now!! Thanks

Comment 3

[Hiroyuki Ikezoe (:hiro)]

One thing I am concerned is that we don't check mElementsToRestyle at all in stylo. 

In case of gecko, the key point is here[1]. Even if we throttle animations, we can get the up-to-date composed values.

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

How do we know we need to flush style for elements that have animations running on compositor in stylo?

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Oops, I did miss Cameron's comment.

Comment 5

[Boris Chiou [:boris]]

I think SetNeedThrottledAnimationFlush() works, so this is not related to Bug 1334735. I tried to do some hacks on [1]: remove the check of |postedRestyle| in PostRestyleForThrottledAnimations(), and it works. Looks like the value in elementsToRestyle is not correct at that moment.

[1] http://searchfox.org/mozilla-central/rev/12cf11303392edac9f1da0c02e3d9ad2ecc8f4d3/dom/animation/EffectCompositor.cpp#332-334

Comment 6

[Hiroyuki Ikezoe (:hiro)]

It might be related to bug 1335545.  Does getComputedStyle() work against script animations?

Comment 7

[Boris Chiou [:boris]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)
> It might be related to bug 1335545.  Does getComputedStyle() work against
> script animations?

Oops!  yes, getComputedStyle(div)["transform"] return valid values and looks like PostRestyleForThrottledAnimations() works based on script animations, so I can pass the test case.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Now bug 1335545 has been closed. Boris, would you mind checking this again?

Comment 9

[Boris Chiou [:boris]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)
> Now bug 1335545 has been closed. Boris, would you mind checking this again?

Sure, keep the ni and I will check this later.

Comment 10

[Boris Chiou [:boris]]

Checking this now.

Comment 11

[Boris Chiou [:boris]]

I can see the result from getComputedStyle(), so close this bug.