Boris Chiou [:boris] Assignee
	Description • 8 years ago

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

	Cameron McCormack (:heycam)
	Comment 1 • 8 years ago

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.

	Boris Chiou [:boris] Assignee
	Comment 2 • 8 years ago

(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

	Hiroyuki Ikezoe (:hiro)
	Comment 3 • 8 years ago

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?

	Hiroyuki Ikezoe (:hiro)
	Comment 4 • 8 years ago

Oops, I did miss Cameron's comment.

	Boris Chiou [:boris] Assignee
	Comment 5 • 8 years ago

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

	Hiroyuki Ikezoe (:hiro)
	Comment 6 • 8 years ago

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

	Boris Chiou [:boris] Assignee
	Comment 7 • 8 years ago

(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.

	Hiroyuki Ikezoe (:hiro)
	Comment 8 • 8 years ago

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

	Boris Chiou [:boris] Assignee
	Comment 9 • 8 years ago

(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.

	Boris Chiou [:boris] Assignee
	Comment 10 • 7 years ago

Checking this now.

	Boris Chiou [:boris] Assignee
	Comment 11 • 7 years ago

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