Bug 1304886 - Part 1: Make StyleAnimationValue::Accumulate() infallible. r?birtles
MozReview-Commit-ID: 9ve3k6a3eAg
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -363,34 +363,26 @@ KeyframeEffectReadOnly::ComposeStyle(
// Iteration composition for accumulate
if (mEffectOptions.mIterationComposite ==
IterationCompositeOperation::Accumulate &&
computedTiming.mCurrentIteration > 0) {
const AnimationPropertySegment& lastSegment =
prop.mSegments.LastElement();
// FIXME: Bug 1293492: Add a utility function to calculate both of
// below StyleAnimationValues.
- DebugOnly<bool> accumulateResult =
+ fromValue =
StyleAnimationValue::Accumulate(prop.mProperty,
- fromValue,
lastSegment.mToValue,
+ Move(fromValue),
computedTiming.mCurrentIteration);
- // We can't check the accumulation result in case of filter property.
- // That's because some filter property can't accumulate,
- // e.g. 'contrast(2) brightness(2)' onto 'brightness(1) contrast(1)'
- // because of mismatch of the order.
- MOZ_ASSERT(accumulateResult || prop.mProperty == eCSSProperty_filter,
- "could not accumulate value");
- accumulateResult =
+ toValue =
StyleAnimationValue::Accumulate(prop.mProperty,
- toValue,
lastSegment.mToValue,
+ Move(toValue),
computedTiming.mCurrentIteration);
- MOZ_ASSERT(accumulateResult || prop.mProperty == eCSSProperty_filter,
- "could not accumulate value");
}
// Special handling for zero-length segments
if (segment->mToKey == segment->mFromKey) {
if (computedTiming.mProgress.Value() < 0) {
aStyleRule->AddValue(prop.mProperty, Move(fromValue));
} else {
aStyleRule->AddValue(prop.mProperty, Move(toValue));
new file mode 100644
--- /dev/null
+++ b/dom/animation/test/crashtests/1304886-1.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<script>
+window.onload=function(){
+ var e = document.createElement("div");
+ document.documentElement.appendChild(e);
+ e.animate([{"font":"status-bar"},
+ {"font":"unset"}],
+ {duration:6,
+ iterationStart:4,
+ iterationComposite:"accumulate"});
+};
+</script>
+</html>
--- a/dom/animation/test/crashtests/crashtests.list
+++ b/dom/animation/test/crashtests/crashtests.list
@@ -6,8 +6,9 @@ pref(dom.animations-api.core.enabled,tru
pref(dom.animations-api.core.enabled,true) load 1216842-4.html
pref(dom.animations-api.core.enabled,true) load 1216842-5.html
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
--- a/gfx/layers/composite/AsyncCompositionManager.cpp
+++ b/gfx/layers/composite/AsyncCompositionManager.cpp
@@ -589,28 +589,26 @@ SampleValue(float aPortion, Animation& a
StyleAnimationValue endValue = aEnd;
// Iteration composition for accumulate
if (static_cast<dom::IterationCompositeOperation>
(aAnimation.iterationComposite()) ==
dom::IterationCompositeOperation::Accumulate &&
aCurrentIteration > 0) {
// FIXME: Bug 1293492: Add a utility function to calculate both of
// below StyleAnimationValues.
- DebugOnly<bool> accumulateResult =
+ startValue =
StyleAnimationValue::Accumulate(aAnimation.property(),
- startValue,
aLastValue,
+ Move(startValue),
aCurrentIteration);
- MOZ_ASSERT(accumulateResult, "could not accumulate value");
- accumulateResult =
+ endValue =
StyleAnimationValue::Accumulate(aAnimation.property(),
- endValue,
aLastValue,
+ Move(endValue),
aCurrentIteration);
- MOZ_ASSERT(accumulateResult, "could not accumulate value");
}
StyleAnimationValue interpolatedValue;
// This should never fail because we only pass transform and opacity values
// to the compositor and they should never fail to interpolate.
DebugOnly<bool> uncomputeResult =
StyleAnimationValue::Interpolate(aAnimation.property(),
startValue, endValue,
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -3095,62 +3095,68 @@ StyleAnimationValue::AddWeighted(nsCSSPr
return true;
}
}
MOZ_ASSERT(false, "Can't interpolate using the given common unit");
return false;
}
-bool
+StyleAnimationValue
StyleAnimationValue::Accumulate(nsCSSPropertyID aProperty,
- StyleAnimationValue& aDest,
- const StyleAnimationValue& aValueToAccumulate,
+ const StyleAnimationValue& aA,
+ StyleAnimationValue&& aB,
uint64_t aCount)
{
+ StyleAnimationValue result(Move(aB));
+
+ if (aCount == 0) {
+ return result;
+ }
+
Unit commonUnit =
- GetCommonUnit(aProperty, aDest.GetUnit(), aValueToAccumulate.GetUnit());
+ GetCommonUnit(aProperty, result.GetUnit(), aA.GetUnit());
switch (commonUnit) {
case eUnit_Filter: {
- UniquePtr<nsCSSValueList> result =
- AddWeightedFilterList(1.0, aDest.GetCSSValueListValue(),
- aCount, aValueToAccumulate.GetCSSValueListValue(),
+ UniquePtr<nsCSSValueList> resultList =
+ AddWeightedFilterList(1.0, result.GetCSSValueListValue(),
+ aCount, aA.GetCSSValueListValue(),
ColorAdditionType::Unclamped);
- if (!result) {
- return false;
+ if (resultList) {
+ result.SetAndAdoptCSSValueListValue(resultList.release(), eUnit_Filter);
}
-
- aDest.SetAndAdoptCSSValueListValue(result.release(), eUnit_Filter);
- return true;
+ break;
}
case eUnit_Shadow: {
- UniquePtr<nsCSSValueList> result =
- AddWeightedShadowList(1.0, aDest.GetCSSValueListValue(),
- aCount, aValueToAccumulate.GetCSSValueListValue(),
+ UniquePtr<nsCSSValueList> resultList =
+ AddWeightedShadowList(1.0, result.GetCSSValueListValue(),
+ aCount, aA.GetCSSValueListValue(),
ColorAdditionType::Unclamped);
- if (!result) {
- return false;
+ if (resultList) {
+ result.SetAndAdoptCSSValueListValue(resultList.release(), eUnit_Shadow);
}
- aDest.SetAndAdoptCSSValueListValue(result.release(), eUnit_Shadow);
- return true;
+ break;
}
case eUnit_Color: {
- RGBAColorData color1 = ExtractColor(aDest);
- RGBAColorData color2 = ExtractColor(aValueToAccumulate);
+ RGBAColorData color1 = ExtractColor(result);
+ RGBAColorData color2 = ExtractColor(aA);
auto resultColor = MakeUnique<nsCSSValue>();
resultColor->SetRGBAColorValue(
AddWeightedColors(1.0, color1, aCount, color2));
- aDest.SetAndAdoptCSSValueValue(resultColor.release(), eUnit_Color);
- return true;
+ result.SetAndAdoptCSSValueValue(resultColor.release(), eUnit_Color);
+ break;
}
default:
- return Add(aProperty, aDest, aValueToAccumulate, aCount);
+ Unused << AddWeighted(aProperty,
+ 1.0, result,
+ aCount, aA,
+ result);
+ break;
}
- MOZ_ASSERT_UNREACHABLE("Can't accumulate using the given common unit");
- return false;
+ return result;
}
already_AddRefed<css::StyleRule>
BuildStyleRule(nsCSSPropertyID aProperty,
dom::Element* aTargetElement,
const nsAString& aSpecifiedValue,
bool aUseSVGMode)
{
--- a/layout/style/StyleAnimationValue.h
+++ b/layout/style/StyleAnimationValue.h
@@ -142,35 +142,26 @@ public:
*/
static MOZ_MUST_USE bool
AddWeighted(nsCSSPropertyID aProperty,
double aCoeff1, const StyleAnimationValue& aValue1,
double aCoeff2, const StyleAnimationValue& aValue2,
StyleAnimationValue& aResultValue);
/**
- * Accumulates |aValueToAccumulate| onto |aDest| |aCount| times.
- * The result is stored in |aDest| on success.
- *
- * @param aDest The base value to be accumulated.
- * @param aValueToAccumulate The value to accumulate.
- * @param aCount The number of times to accumulate
- * aValueToAccumulate.
- * @return true on success, false on failure.
- *
- * NOTE: This function will work as a wrapper of StyleAnimationValue::Add()
- * if |aProperty| isn't color or shadow or filter. For these properties,
- * this function may return a color value that at least one of its components
- * has a value which is outside the range [0, 1] so that we can calculate
- * plausible values as interpolation with the return value.
+ * Accumulates |aA| onto |aA| |aCount| times then accumulates |aB| onto the
+ * result.
+ * If |aCount| is zero or no accumulation or addition procedure is defined
+ * for |aProperty| the result will be |aB|.
*/
- static MOZ_MUST_USE bool
- Accumulate(nsCSSPropertyID aProperty, StyleAnimationValue& aDest,
- const StyleAnimationValue& aValueToAccumulate,
- uint64_t aCount);
+ static StyleAnimationValue
+ Accumulate(nsCSSPropertyID aProperty,
+ const StyleAnimationValue& aA,
+ StyleAnimationValue&& aB,
+ uint64_t aCount = 1);
// Type-conversion methods
// -----------------------
/**
* Creates a computed value for the given specified value
* (property ID + string). A style context is needed in case the
* specified value depends on inherited style or on the values of other
* properties.
--- a/testing/web-platform/tests/web-animations/interfaces/KeyframeEffect/iterationComposite.html
+++ b/testing/web-platform/tests/web-animations/interfaces/KeyframeEffect/iterationComposite.html
@@ -7,16 +7,56 @@
<script src="../../testcommon.js"></script>
<div id="log"></div>
<script>
'use strict';
test(function(t) {
var div = createDiv(t);
var anim =
+ div.animate({ alignContent: ['flex-start', 'flex-end'] },
+ { duration: 100 * MS_PER_SEC,
+ easing: 'linear',
+ iterations: 10,
+ iterationComposite: 'accumulate' });
+
+ anim.currentTime = anim.effect.timing.duration / 2;
+ assert_equals(getComputedStyle(div).alignContent, 'flex-end',
+ 'Animated visibility style at 50s of the first iteration');
+ anim.currentTime = anim.effect.timing.duration * 2;
+ assert_equals(getComputedStyle(div).alignContent, 'flex-start',
+ 'Animated visibility style at 0s of the third iteration');
+ anim.currentTime += anim.effect.timing.duration / 2;
+ assert_equals(getComputedStyle(div).alignContent, 'flex-end',
+ 'Animated visibility style at 50s of the third iteration');
+}, 'iterationComposite of discrete type animation (align-content)');
+
+test(function(t) {
+ var div = createDiv(t);
+ var anim =
+ div.animate({ alignContent: ['flex-start', 'flex-start'] },
+ { duration: 100 * MS_PER_SEC,
+ easing: 'linear',
+ iterations: 10,
+ iterationComposite: 'accumulate' });
+
+ anim.currentTime = anim.effect.timing.duration / 2;
+ assert_equals(getComputedStyle(div).alignContent, 'flex-start',
+ 'Animated visibility style at 50s of the first iteration');
+ anim.currentTime = anim.effect.timing.duration * 2;
+ assert_equals(getComputedStyle(div).alignContent, 'flex-start',
+ 'Animated visibility style at 0s of the third iteration');
+ anim.currentTime += anim.effect.timing.duration / 2;
+ assert_equals(getComputedStyle(div).alignContent, 'flex-start',
+ 'Animated visibility style at 50s of the third iteration');
+}, 'iterationComposite of discrete type animation for the same value');
+
+test(function(t) {
+ var div = createDiv(t);
+ var anim =
div.animate({ marginLeft: ['0px', '10px'] },
{ duration: 100 * MS_PER_SEC,
easing: 'linear',
iterations: 10,
iterationComposite: 'accumulate' });
anim.pause();
anim.currentTime = anim.effect.timing.duration / 2;