Bug 1322291 - Part 2: Make sure that the base style is set even if additive or accumulates animations are in the delay phase. r?birtles
Before this patch we skipped KeyframeEffectReadOnly::ComposeStyle() for
animations that are not in effect.
After this patch we call KeyframeEffectReadOnly::ComposeStyle() even if the
animation is not in-effect state in order to prepare the base style for
properties that can be run on the compositor because the in-effect animation
will be sent to the compositor and might be composed onto the base style on the
compositor after the animation gets out of its delay phase.
MozReview-Commit-ID: FuAZv4jqVMJ
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -893,20 +893,16 @@ Animation::HasLowerCompositeOrderThan(co
void
Animation::ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
const nsCSSPropertyIDSet& aPropertiesToSkip)
{
if (!mEffect) {
return;
}
- if (!IsInEffect()) {
- return;
- }
-
// In order to prevent flicker, there are a few cases where we want to use
// a different time for rendering that would otherwise be returned by
// GetCurrentTime. These are:
//
// (a) For animations that are pausing but which are still running on the
// compositor. In this case we send a layer transaction that removes the
// animation but which also contains the animation values calculated on
// the main thread. To prevent flicker when this occurs we want to ensure
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -383,16 +383,59 @@ KeyframeEffectReadOnly::CompositeValue(
return CompositeValue(aProperty,
aValueToComposite,
result,
aCompositeOperation);
}
void
+KeyframeEffectReadOnly::EnsureBaseStylesForCompositor(
+ const nsCSSPropertyIDSet& aPropertiesToSkip)
+{
+ if (!mTarget) {
+ return;
+ }
+
+ RefPtr<nsStyleContext> styleContext;
+
+ for (const AnimationProperty& property : mProperties) {
+ if (!nsCSSProps::PropHasFlags(property.mProperty,
+ CSS_PROPERTY_CAN_ANIMATE_ON_COMPOSITOR)) {
+ continue;
+ }
+
+ if (aPropertiesToSkip.HasProperty(property.mProperty)) {
+ continue;
+ }
+
+ for (const AnimationPropertySegment& segment : property.mSegments) {
+ if (segment.mFromComposite == dom::CompositeOperation::Replace &&
+ segment.mToComposite == dom::CompositeOperation::Replace) {
+ continue;
+ }
+
+ if (!styleContext) {
+ styleContext = GetTargetStyleContext();
+ }
+ MOZ_RELEASE_ASSERT(styleContext);
+
+ Unused << EffectCompositor::GetBaseStyle(property.mProperty,
+ styleContext,
+ *mTarget->mElement,
+ mTarget->mPseudoType);
+ // Make this property as needing a base style so that we send the (now
+ // cached) base style to the compositor.
+ SetNeedsBaseStyle(property.mProperty);
+ break;
+ }
+ }
+}
+
+void
KeyframeEffectReadOnly::ComposeStyle(
RefPtr<AnimValuesStyleRule>& aStyleRule,
const nsCSSPropertyIDSet& aPropertiesToSkip)
{
if (mIsComposingStyle) {
return;
}
@@ -401,16 +444,29 @@ KeyframeEffectReadOnly::ComposeStyle(
ComputedTiming computedTiming = GetComputedTiming();
mProgressOnLastCompose = computedTiming.mProgress;
mCurrentIterationOnLastCompose = computedTiming.mCurrentIteration;
// If the progress is null, we don't have fill data for the current
// time so we shouldn't animate.
if (computedTiming.mProgress.IsNull()) {
+ // If we are not in-effect, this effect might still be sent to the
+ // compositor and later become in-effect (e.g. if it is in the delay phase).
+ // In that case, we might need the base style in order to perform
+ // additive/accumulative animation on the compositor.
+
+ // In case of properties that can be run on the compositor, we need the base
+ // styles for such properties because those animation will be sent to
+ // compositor while they are in delay phase so that we can composite this
+ // animation on the compositor once the animation is out of the delay phase
+ // on the compositor.
+ if (computedTiming.mPhase == ComputedTiming::AnimationPhase::Before) {
+ EnsureBaseStylesForCompositor(aPropertiesToSkip);
+ }
return;
}
mNeedsBaseStyleSet.Empty();
for (size_t propIdx = 0, propEnd = mProperties.Length();
propIdx != propEnd; ++propIdx)
{
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -422,16 +422,21 @@ protected:
const RefPtr<AnimValuesStyleRule>& aAnimationRule,
const StyleAnimationValue& aValueToComposite,
CompositeOperation aCompositeOperation);
// Set a bit in mNeedsBaseStyleSet if |aProperty| can be run on the
// compositor.
void SetNeedsBaseStyle(nsCSSPropertyID aProperty);
+ // Ensure the base styles is available for any properties that can be run on
+ // the compositor and which are not includes in |aPropertiesToSkip|.
+ void EnsureBaseStylesForCompositor(
+ const nsCSSPropertyIDSet& aPropertiesToSkip);
+
Maybe<OwningAnimationTarget> mTarget;
KeyframeEffectParams mEffectOptions;
// The specified keyframes.
nsTArray<Keyframe> mKeyframes;
// A set of per-property value arrays, derived from |mKeyframes|.
new file mode 100644
--- /dev/null
+++ b/dom/animation/test/crashtests/1322291-1.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<script>
+document.addEventListener("DOMContentLoaded", boom);
+function boom(){
+ let o1 = (function(){
+ let e=document.createElement("frameset");
+ document.documentElement.appendChild(e);
+ return e;
+ })();
+ let a1 = o1.animate({ "transform": "rotate3d(22,73,26,374grad)" },
+ { duration: 10, delay: 100 });
+
+ // We need to wait the end of the delay to ensure that the animation is
+ // composited on the compositor, but there is no event for script animation
+ // that fires after the delay phase finished. So we wait for finished promise
+ // instead.
+ a1.finished.then(function() {
+ document.documentElement.className = "";
+ });
+}
+</script>
+</body>
+</html>
--- a/dom/animation/test/crashtests/crashtests.list
+++ b/dom/animation/test/crashtests/crashtests.list
@@ -8,10 +8,11 @@ pref(dom.animations-api.core.enabled,tru
pref(dom.animations-api.core.enabled,true) load 1216842-6.html
pref(dom.animations-api.core.enabled,true) load 1272475-1.html
pref(dom.animations-api.core.enabled,true) load 1272475-2.html
pref(dom.animations-api.core.enabled,true) load 1278485-1.html
pref(dom.animations-api.core.enabled,true) load 1277272-1.html
pref(dom.animations-api.core.enabled,true) load 1290535-1.html
pref(dom.animations-api.core.enabled,true) load 1304886-1.html
pref(dom.animations-api.core.enabled,true) load 1322382-1.html
+pref(dom.animations-api.core.enabled,true) load 1322291-1.html
pref(dom.animations-api.core.enabled,true) load 1323114-1.html
pref(dom.animations-api.core.enabled,true) load 1323114-2.html
--- a/dom/animation/test/style/file_missing-keyframe-on-compositor.html
+++ b/dom/animation/test/style/file_missing-keyframe-on-compositor.html
@@ -466,11 +466,30 @@ promise_test(t => {
assert_matrix_equals(transform, 'matrix(1, 0, 0, 1, 250, 0)',
'Composed transform value should be composed onto the value of ' +
'lower-priority animation with fill:forwards during positive endDelay');
});
}, 'Transform value for animation with no keyframe at offset 0 at 50% when ' +
'composed onto a underlying animation with fill:forwards during positive ' +
'endDelay');
+promise_test(t => {
+ useTestRefreshMode(t);
+
+ var div = addDiv(t, { style: 'transform: translateX(100px)' });
+ div.animate({ transform: 'translateX(200px)' },
+ { duration: 100 * MS_PER_SEC, delay: 50 * MS_PER_SEC });
+
+ return waitForPaintsFlushed().then(() => {
+ SpecialPowers.DOMWindowUtils.advanceTimeAndRefresh(100 * MS_PER_SEC);
+
+ var transform =
+ SpecialPowers.DOMWindowUtils.getOMTAStyle(div, 'transform');
+ assert_matrix_equals(transform, 'matrix(1, 0, 0, 1, 150, 0)',
+ 'Transform value for animation with positive delay should be composed ' +
+ 'onto the base style');
+ });
+}, 'Transform value for animation with no keyframe at offset 0 and with ' +
+ 'positive delay');
+
done();
</script>
</body>