Closed Bug 1341195 Opened 6 years ago Closed 6 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: boris, Assigned: boris)

References

Details

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
Blocks: stylo-style-mochitest
No longer blocks: 1337599
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.
(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
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?
Oops, I did miss Cameron's comment.
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
It might be related to bug 1335545.  Does getComputedStyle() work against script animations?
(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.
Depends on: 1335545
Now bug 1335545 has been closed. Boris, would you mind checking this again?
Flags: needinfo?(boris.chiou)
(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.
Checking this now.
Assignee: nobody → boris.chiou
Flags: needinfo?(boris.chiou)
I can see the result from getComputedStyle(), so close this bug.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.