Bug 1402547 - Handle addition/interpolate of nsSMILCSSValueType objects when both objects are empty; r?hiro
In some rare cases we can end up with both arguments passed to
nsSMILCSSValueType::SandwichAdd being "empty", that is, having the type
nsSMILCSSValueType but a null pointer.
This can happen, for example, when we have a two by-animations and linear
interpolation causing us to pass the "empty" from-value up as the underlying
value from the first by-animation to the second by-animation (which it will try
to add with its empty from-value).
In this case the result of adding "empty" to "empty" should just be "empty"
which we achieve by just doing an early return (since the fallback behavior for
failed addition is to just use the second argument as the result; note that if
we return true though the result would also be the same).
We don't currently have a test case that produces this for interpolate but it
makes sense that trying to interpolate "empty" to "empty" should likewise fail
and just produce "empty". In this case failing will make us fall back to
discrete animation and just use the "empty" values as-is. Indicating failure,
however, has the additional effect of making us use the special keyTimes
behavior defined for discrete animation.
MozReview-Commit-ID: IZ5qg0Mk5Uy
--- a/dom/smil/nsSMILCSSValueType.cpp
+++ b/dom/smil/nsSMILCSSValueType.cpp
@@ -109,34 +109,36 @@ GetZeroValueForUnit(StyleAnimationValue:
return &sZeroFloat;
case StyleAnimationValue::eUnit_Color:
return &sZeroColor;
default:
return nullptr;
}
}
-// This method requires at least one of its arguments to be non-null.
-//
// If one argument is null, this method updates it to point to "zero"
// for the other argument's Unit (if applicable; otherwise, we return false).
//
-// If neither argument is null, this method does nothing.
+// If neither argument is null, this method simply returns true.
+//
+// If both arguments are null, this method returns false.
//
// |aZeroValueStorage| should be a reference to a RefPtr<RawServoAnimationValue>.
// This is used where we may need to allocate a new ServoAnimationValue to
// represent the appropriate zero value.
//
// Returns true on success, or otherwise.
static bool
FinalizeServoAnimationValues(const RefPtr<RawServoAnimationValue>*& aValue1,
const RefPtr<RawServoAnimationValue>*& aValue2,
RefPtr<RawServoAnimationValue>& aZeroValueStorage)
{
- MOZ_ASSERT(aValue1 || aValue2, "expecting at least one non-null value");
+ if (!aValue1 && !aValue2) {
+ return false;
+ }
// Are we missing either val? (If so, it's an implied 0 in other val's units)
if (!aValue1) {
aZeroValueStorage = Servo_AnimationValues_GetZeroValue(*aValue2).Consume();
aValue1 = &aZeroValueStorage;
} else if (!aValue2) {
aZeroValueStorage = Servo_AnimationValues_GetZeroValue(*aValue1).Consume();
@@ -144,18 +146,19 @@ FinalizeServoAnimationValues(const RefPt
}
return *aValue1 && *aValue2;
}
static bool
FinalizeStyleAnimationValues(const StyleAnimationValue*& aValue1,
const StyleAnimationValue*& aValue2)
{
- MOZ_ASSERT(aValue1 || aValue2,
- "expecting at least one non-null value");
+ if (!aValue1 && !aValue2) {
+ return false;
+ }
if (!aValue1) {
aValue1 = GetZeroValueForUnit(aValue2->GetUnit());
return !!aValue1; // Fail if we have no zero value for this unit.
}
if (!aValue2) {
aValue2 = GetZeroValueForUnit(aValue1->GetUnit());
return !!aValue2; // Fail if we have no zero value for this unit.
@@ -293,17 +296,17 @@ AddOrAccumulateForServo(nsSMILValue& aDe
: aDestWrapper->mPropID;
size_t len = aValueToAddWrapper
? aValueToAddWrapper->mServoValues.Length()
: aDestWrapper->mServoValues.Length();
MOZ_ASSERT(!aValueToAddWrapper || !aDestWrapper ||
aValueToAddWrapper->mServoValues.Length() ==
aDestWrapper->mServoValues.Length(),
- "Both of values'length in the wrappers should be the same if "
+ "Both of values' length in the wrappers should be the same if "
"both of them exist");
for (size_t i = 0; i < len; i++) {
const RefPtr<RawServoAnimationValue>* valueToAdd =
aValueToAddWrapper
? &aValueToAddWrapper->mServoValues[i]
: nullptr;
const RefPtr<RawServoAnimationValue>* destValue =
@@ -354,18 +357,25 @@ AddOrAccumulate(nsSMILValue& aDest, cons
MOZ_ASSERT(aCompositeOp == CompositeOperation::Add ||
aCompositeOp == CompositeOperation::Accumulate,
"Composite operation should be add or accumulate");
MOZ_ASSERT(aCompositeOp != CompositeOperation::Add || aCount == 1,
"Count should be 1 if composite operation is add");
ValueWrapper* destWrapper = ExtractValueWrapper(aDest);
const ValueWrapper* valueToAddWrapper = ExtractValueWrapper(aValueToAdd);
- MOZ_ASSERT(destWrapper || valueToAddWrapper,
- "need at least one fully-initialized value");
+
+ // If both of the values are empty just fail. This can happen in rare cases
+ // such as when the underlying animation produced an empty value.
+ //
+ // Technically, it doesn't matter what we return here since in either case it
+ // will produce the same result: an empty value.
+ if (!destWrapper && !valueToAddWrapper) {
+ return false;
+ }
nsCSSPropertyID property = valueToAddWrapper
? valueToAddWrapper->mPropID
: destWrapper->mPropID;
// Special case: font-size-adjust and stroke-dasharray are explicitly
// non-additive (even though StyleAnimationValue *could* support adding them)
if (property == eCSSProperty_font_size_adjust ||
property == eCSSProperty_stroke_dasharray) {