Bug 1339332 - Part 1: Introduce natural value concept for missing keyframe in CSS Animation. r?birtles draft
authorDaisuke Akatsuka <dakatsuka@mozilla.com>
Sat, 25 Feb 2017 22:10:02 +0900
changeset 489659 6cde22c1661d766116d4a69dac06512c94508497
parent 483291 195049fabb7ac5709e5f75614ba630ba3d1b5a9b
child 489660 ee6303a577e1ee2aba59591bced85944b610d4ee
child 489683 7a850b768226e276df007d53d798fde21b9623af
push id46874
push userbmo:dakatsuka@mozilla.com
push dateSat, 25 Feb 2017 13:11:03 +0000
reviewersbirtles
bugs1339332
milestone54.0a1
Bug 1339332 - Part 1: Introduce natural value concept for missing keyframe in CSS Animation. r?birtles MozReview-Commit-ID: CcauAyfhFoQ
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeUtils.cpp
dom/animation/test/chrome/test_animation_properties.html
layout/base/nsLayoutUtils.cpp
layout/style/nsAnimationManager.cpp
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -324,23 +324,20 @@ KeyframeEffectReadOnly::UpdateProperties
 KeyframeEffectReadOnly::CompositeValue(
   nsCSSPropertyID aProperty,
   const StyleAnimationValue& aValueToComposite,
   const StyleAnimationValue& aUnderlyingValue,
   CompositeOperation aCompositeOperation)
 {
   switch (aCompositeOperation) {
     case dom::CompositeOperation::Replace:
-      return aValueToComposite;
+      // Return the underlying value if |aValueToComposite| is null (i.e.
+      // missing keyframe). Otherwise, just return the |aValueToComposite|.
+      return aValueToComposite.IsNull() ? aUnderlyingValue : aValueToComposite;
     case dom::CompositeOperation::Add: {
-      // Just return the underlying value if |aValueToComposite| is null (i.e.
-      // missing keyframe).
-      if (aValueToComposite.IsNull()) {
-        return aUnderlyingValue;
-      }
       StyleAnimationValue result(aValueToComposite);
       return StyleAnimationValue::Add(aProperty,
                                       aUnderlyingValue,
                                       Move(result));
     }
     case dom::CompositeOperation::Accumulate: {
       StyleAnimationValue result(aValueToComposite);
       return StyleAnimationValue::Accumulate(aProperty,
@@ -409,31 +406,31 @@ KeyframeEffectReadOnly::CompositeValue(
   const RefPtr<AnimValuesStyleRule>& aAnimationRule,
   const StyleAnimationValue& aValueToComposite,
   CompositeOperation aCompositeOperation)
 {
   MOZ_ASSERT(mTarget, "CompositeValue should be called with target element");
 
   StyleAnimationValue result = aValueToComposite;
 
-  if (aCompositeOperation == CompositeOperation::Replace) {
-    MOZ_ASSERT(!aValueToComposite.IsNull(),
-      "Input value should be valid in case of replace composite");
-    // Just copy the input value in case of 'Replace'.
+  if (aCompositeOperation == CompositeOperation::Replace &&
+      !aValueToComposite.IsNull()) {
+    // Just copy the input value in case of 'Replace' composite
+    // and the value is not null.
     return result;
   }
 
   // FIXME: Bug 1311257: Get the base value for the servo backend.
   if (mDocument->IsStyledByServo()) {
     return result;
   }
 
   MOZ_ASSERT(!aValueToComposite.IsNull() ||
-             aCompositeOperation == CompositeOperation::Add,
-             "InputValue should be null only if additive composite");
+             aCompositeOperation == CompositeOperation::Replace,
+             "InputValue should be null only if replace composite");
 
   result = GetUnderlyingStyle(aProperty, aAnimationRule);
 
   return CompositeValue(aProperty,
                         aValueToComposite,
                         result,
                         aCompositeOperation);
 }
@@ -446,17 +443,18 @@ KeyframeEffectReadOnly::EnsureBaseStyles
   if (!mTarget) {
     return;
   }
 
   mBaseStyleValues.Clear();
 
   for (const AnimationProperty& property : aProperties) {
     for (const AnimationPropertySegment& segment : property.mSegments) {
-      if (segment.mFromComposite == dom::CompositeOperation::Replace &&
+      if (!segment.mFromValue.IsNull() && !segment.mToValue.IsNull() &&
+          segment.mFromComposite == dom::CompositeOperation::Replace &&
           segment.mToComposite == dom::CompositeOperation::Replace) {
         continue;
       }
 
       Unused << ResolveBaseStyle(property.mProperty, aStyleContext);
       break;
     }
   }
@@ -587,19 +585,19 @@ KeyframeEffectReadOnly::ComposeStyle(
       StyleAnimationValue fromValue =
         CompositeValue(prop.mProperty, aStyleRule.mGecko,
                        segment->mFromValue.mGecko,
                        segment->mFromComposite);
       StyleAnimationValue toValue =
         CompositeValue(prop.mProperty, aStyleRule.mGecko,
                        segment->mToValue.mGecko,
                        segment->mToComposite);
-      if (fromValue.IsNull() || toValue.IsNull()) {
-        continue;
-      }
+
+      MOZ_ASSERT(!fromValue.IsNull(), "from value should not be null");
+      MOZ_ASSERT(!toValue.IsNull(), "to value should not be null");
 
       // 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
@@ -1167,26 +1165,43 @@ KeyframeEffectReadOnly::GetKeyframes(JSC
 
     JS::Rooted<JSObject*> keyframeObject(aCx, &keyframeJSValue.toObject());
     for (const PropertyValuePair& propertyValue : keyframe.mPropertyValues) {
       nsAutoString stringValue;
       if (propertyValue.mServoDeclarationBlock) {
         Servo_DeclarationBlock_SerializeOneValue(
           propertyValue.mServoDeclarationBlock,
           propertyValue.mProperty, &stringValue);
-      } else {
+      } else if (nsCSSProps::IsShorthand(propertyValue.mProperty)) {
         // nsCSSValue::AppendToString does not accept shorthands properties but
         // works with token stream values if we pass eCSSProperty_UNKNOWN as
         // the property.
-        nsCSSPropertyID propertyForSerializing =
-          nsCSSProps::IsShorthand(propertyValue.mProperty)
-          ? eCSSProperty_UNKNOWN
-          : propertyValue.mProperty;
         propertyValue.mValue.AppendToString(
-          propertyForSerializing, stringValue, nsCSSValue::eNormalized);
+          eCSSProperty_UNKNOWN, stringValue, nsCSSValue::eNormalized);
+      } else {
+        nsCSSValue cssValue = propertyValue.mValue;
+        if (cssValue.GetUnit() == eCSSUnit_Null) {
+          // We use an uninitialized nsCSSValue to represent the
+          // "neutral value". We currently only do this for keyframes generated
+          // from CSS animations with missing 0%/100% keyframes. Furthermore,
+          // currently (at least until bug 1339334) keyframes generated from
+          // CSS animations only contain longhand properties so we only need to
+          // handle null nsCSSValues for longhand properties.
+          DebugOnly<bool> uncomputeResult =
+            StyleAnimationValue::UncomputeValue(
+              propertyValue.mProperty,
+              BaseStyle(propertyValue.mProperty), cssValue);
+
+          MOZ_ASSERT(uncomputeResult,
+                     "Unable to get specified value from computed value");
+          MOZ_ASSERT(cssValue.GetUnit() != eCSSUnit_Null,
+                     "Got null computed value");
+        }
+        cssValue.AppendToString(propertyValue.mProperty,
+                                stringValue, nsCSSValue::eNormalized);
       }
 
       const char* name = nsCSSProps::PropertyIDLName(propertyValue.mProperty);
       JS::Rooted<JS::Value> value(aCx);
       if (!ToJSValue(aCx, stringValue, &value) ||
           !JS_DefineProperty(aCx, keyframeObject, name, value,
                              JSPROP_ENUMERATE)) {
         aRv.Throw(NS_ERROR_FAILURE);
@@ -1564,25 +1579,28 @@ CreateStyleContextForAnimationValue(nsCS
 void
 KeyframeEffectReadOnly::CalculateCumulativeChangeHint(
   nsStyleContext *aStyleContext)
 {
   mCumulativeChangeHint = nsChangeHint(0);
 
   for (const AnimationProperty& property : mProperties) {
     for (const AnimationPropertySegment& segment : property.mSegments) {
-      // In case composite operation is not 'replace', we can't throttle
-      // animations which will not cause any layout changes on invisible
-      // elements because we can't calculate the change hint for such properties
-      // until we compose it.
-      if (segment.mFromComposite != CompositeOperation::Replace ||
+      // In case composite operation is not 'replace' or value is null,
+      // we can't throttle animations which will not cause any layout changes
+      // on invisible elements because we can't calculate the change hint for
+      // such properties until we compose it.
+      if (segment.mFromValue.IsNull() ||
+          segment.mToValue.IsNull() ||
+          segment.mFromComposite != CompositeOperation::Replace ||
           segment.mToComposite != CompositeOperation::Replace) {
         mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible;
         return;
       }
+
       RefPtr<nsStyleContext> fromContext =
         CreateStyleContextForAnimationValue(property.mProperty,
                                             segment.mFromValue.mGecko,
                                             aStyleContext);
 
       RefPtr<nsStyleContext> toContext =
         CreateStyleContextForAnimationValue(property.mProperty,
                                             segment.mToValue.mGecko,
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -648,16 +648,22 @@ KeyframeUtils::GetComputedKeyframeValues
         if (nsCSSProps::IsShorthand(pair.mProperty)) {
           nsCSSValueTokenStream* tokenStream = pair.mValue.GetTokenStreamValue();
           if (!StyleAnimationValue::ComputeValues(pair.mProperty,
                 CSSEnabledState::eForAllContent, aElement, aStyleContext,
                 tokenStream->mTokenStream, /* aUseSVGMode */ false, values) ||
               IsComputeValuesFailureKey(pair)) {
             continue;
           }
+        } else if (pair.mValue.GetUnit() == eCSSUnit_Null) {
+          // An uninitialized nsCSSValue represents the underlying value which
+          // we represent as an uninitialized AnimationValue so we just leave
+          // neutralPair->mValue as-is.
+          PropertyStyleAnimationValuePair* neutralPair = values.AppendElement();
+          neutralPair->mProperty = pair.mProperty;
         } else {
           if (!StyleAnimationValue::ComputeValues(pair.mProperty,
                 CSSEnabledState::eForAllContent, aElement, aStyleContext,
                 pair.mValue, /* aUseSVGMode */ false, values)) {
             continue;
           }
           MOZ_ASSERT(values.Length() == 1,
                     "Longhand properties should produce a single"
@@ -1159,33 +1165,33 @@ IsComputeValuesFailureKey(const Property
 
 static void
 AppendInitialSegment(AnimationProperty* aAnimationProperty,
                      const KeyframeValueEntry& aFirstEntry)
 {
   AnimationPropertySegment* segment =
     aAnimationProperty->mSegments.AppendElement();
   segment->mFromKey        = 0.0f;
-  segment->mFromComposite  = dom::CompositeOperation::Add;
+  segment->mFromComposite  = dom::CompositeOperation::Replace;
   segment->mToKey          = aFirstEntry.mOffset;
   segment->mToValue        = aFirstEntry.mValue;
   segment->mToComposite    = aFirstEntry.mComposite;
 }
 
 static void
 AppendFinalSegment(AnimationProperty* aAnimationProperty,
                    const KeyframeValueEntry& aLastEntry)
 {
   AnimationPropertySegment* segment =
     aAnimationProperty->mSegments.AppendElement();
   segment->mFromKey        = aLastEntry.mOffset;
   segment->mFromValue      = aLastEntry.mValue;
   segment->mFromComposite  = aLastEntry.mComposite;
   segment->mToKey          = 1.0f;
-  segment->mToComposite    = dom::CompositeOperation::Add;
+  segment->mToComposite    = dom::CompositeOperation::Replace;
   segment->mTimingFunction = aLastEntry.mTimingFunction;
 }
 
 // Returns a newly created AnimationProperty if one was created to fill-in the
 // missing keyframe, nullptr otherwise (if we decided not to fill the keyframe
 // becase we don't support additive animation).
 static AnimationProperty*
 HandleMissingInitialKeyframe(nsTArray<AnimationProperty>& aResult,
--- a/dom/animation/test/chrome/test_animation_properties.html
+++ b/dom/animation/test/chrome/test_animation_properties.html
@@ -692,97 +692,97 @@ var gTests = [
   // convert to computed values.
   //
   // ---------------------------------------------------------------------
 
   { desc:     'a missing property in initial keyframe',
     frames:   [ { },
                 { margin: '5px' } ],
     expected: [ { property: 'margin-top',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(1, '5px', 'replace') ] },
                 { property: 'margin-right',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(1, '5px', 'replace') ] },
                 { property: 'margin-bottom',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(1, '5px', 'replace') ] },
                 { property: 'margin-left',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(1, '5px', 'replace') ] } ]
   },
   { desc:     'a missing property in initial keyframe and there are some ' +
               'keyframes with the same offset',
     frames:   [ { },
                 { margin: '10px', offset: 0.5 },
                 { margin: '20px', offset: 0.5 },
                 { margin: '30px'} ],
     expected: [ { property: 'margin-top',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(0.5, '10px', 'replace'),
                             value(0.5, '20px', 'replace', 'linear'),
                             value(1,   '30px', 'replace') ] },
                 { property: 'margin-right',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(0.5, '10px', 'replace'),
                             value(0.5, '20px', 'replace', 'linear'),
                             value(1,   '30px', 'replace') ] },
                 { property: 'margin-bottom',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(0.5, '10px', 'replace'),
                             value(0.5, '20px', 'replace', 'linear'),
                             value(1,   '30px', 'replace') ] },
                 { property: 'margin-left',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(0.5, '10px', 'replace'),
                             value(0.5, '20px', 'replace', 'linear'),
                             value(1,   '30px', 'replace') ] } ]
   },
   { desc:     'a missing property in final keyframe',
     frames:   [ { margin: '5px' },
                 { } ],
     expected: [ { property: 'margin-top',
                   values: [ value(0, '5px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] },
+                            value(1, undefined, 'replace') ] },
                 { property: 'margin-right',
                   values: [ value(0, '5px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] },
+                            value(1, undefined, 'replace') ] },
                 { property: 'margin-bottom',
                   values: [ value(0, '5px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] },
+                            value(1, undefined, 'replace') ] },
                 { property: 'margin-left',
                   values: [ value(0, '5px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] } ]
+                            value(1, undefined, 'replace') ] } ]
   },
   { desc:     'a missing property in final keyframe and there are some ' +
               'keyframes with the same offsets',
     frames:   [ { margin: '5px' },
                 { margin: '10px', offset: 0.5 },
                 { margin: '20px', offset: 0.5 },
                 { } ],
     expected: [ { property: 'margin-top',
                   values: [ value(0,   '5px', 'replace', 'linear'),
                             value(0.5, '10px', 'replace'),
                             value(0.5, '20px', 'replace', 'linear'),
-                            value(1,   undefined, 'add') ] },
+                            value(1,   undefined, 'replace') ] },
                 { property: 'margin-right',
                   values: [ value(0, '5px', 'replace', 'linear'),
                             value(0.5, '10px', 'replace'),
                             value(0.5, '20px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] },
+                            value(1, undefined, 'replace') ] },
                 { property: 'margin-bottom',
                   values: [ value(0, '5px', 'replace', 'linear'),
                             value(0.5, '10px', 'replace'),
                             value(0.5, '20px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] },
+                            value(1, undefined, 'replace') ] },
                 { property: 'margin-left',
                   values: [ value(0, '5px', 'replace', 'linear'),
                             value(0.5, '10px', 'replace'),
                             value(0.5, '20px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] } ]
+                            value(1, undefined, 'replace') ] } ]
   },
   { desc:     'a missing property in final keyframe where it forms the last'
               + ' segment in the series',
     frames:   [ { margin: '5px' },
                 { marginLeft: '5px',
                   marginRight: '5px',
                   marginBottom: '5px' } ],
     expected: [ { property: 'margin-bottom',
@@ -791,103 +791,103 @@ var gTests = [
                 { property: 'margin-left',
                   values: [ value(0, '5px', 'replace', 'linear'),
                             value(1, '5px', 'replace') ] },
                 { property: 'margin-right',
                   values: [ value(0, '5px', 'replace', 'linear'),
                             value(1, '5px', 'replace') ] },
                 { property: 'margin-top',
                   values: [ value(0, '5px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] } ]
+                            value(1, undefined, 'replace') ] } ]
   },
   { desc:     'a missing property in initial keyframe along with other values',
     frames:   [ {                left: '10px' },
                 { margin: '5px', left: '20px' } ],
     expected: [ { property: 'left',
                   values: [ value(0, '10px', 'replace', 'linear'),
                             value(1, '20px', 'replace') ] },
                 { property: 'margin-top',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(1, '5px', 'replace') ] },
                 { property: 'margin-right',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(1, '5px', 'replace') ] },
                 { property: 'margin-bottom',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(1, '5px', 'replace') ] },
                 { property: 'margin-left',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(1, '5px', 'replace') ] } ]
   },
   { desc:     'a missing property in final keyframe along with other values',
     frames:   [ { margin: '5px', left: '10px' },
                 {                left: '20px' } ],
     expected: [ { property: 'left',
                   values: [ value(0, '10px', 'replace', 'linear'),
                             value(1, '20px', 'replace') ] },
                 { property: 'margin-top',
                   values: [ value(0, '5px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] },
+                            value(1, undefined, 'replace') ] },
                 { property: 'margin-right',
                   values: [ value(0, '5px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] },
+                            value(1, undefined, 'replace') ] },
                 { property: 'margin-bottom',
                   values: [ value(0, '5px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] },
+                            value(1, undefined, 'replace') ] },
                 { property: 'margin-left',
                   values: [ value(0, '5px', 'replace', 'linear'),
-                            value(1, undefined, 'add') ] } ]
+                            value(1, undefined, 'replace') ] } ]
   },
   { desc:     'missing properties in both of initial and final keyframe',
     frames:   [ { left: '5px', offset: 0.5 } ],
     expected: [ { property: 'left',
-                  values: [ value(0,   undefined, 'add',     'linear'),
-                            value(0.5, '5px',       'replace', 'linear'),
-                            value(1,   undefined, 'add') ] } ]
+                  values: [ value(0,   undefined, 'replace', 'linear'),
+                            value(0.5, '5px',     'replace', 'linear'),
+                            value(1,   undefined, 'replace') ] } ]
   },
   { desc:     'missing propertes in both of initial and final keyframe along '
               + 'with other values',
     frames:   [ { left:  '5px',  offset: 0 },
                 { right: '5px',  offset: 0.5 },
                 { left:  '10px', offset: 1 } ],
     expected: [ { property: 'left',
                   values: [ value(0, '5px',  'replace', 'linear'),
                             value(1, '10px', 'replace') ] },
                 { property: 'right',
-                  values: [ value(0,   undefined, 'add',     'linear'),
+                  values: [ value(0,   undefined, 'replace', 'linear'),
                             value(0.5, '5px',     'replace', 'linear'),
-                            value(1,   undefined, 'add') ] } ]
+                            value(1,   undefined, 'replace') ] } ]
   },
 
   { desc:     'a missing property in final keyframe with duplicate offset ' +
               + 'along with other values',
     frames:   [ { left: '5px',  right: '5px', offset: 0 },
                 { left: '8px',  right: '8px', offset: 0 },
                 { left: '10px',               offset: 1 } ],
     expected: [ { property: 'left',
                   values: [ value(0, '5px',  'replace'),
                             value(0, '8px',  'replace', 'linear'),
                             value(1, '10px', 'replace') ] },
                 { property: 'right',
                   values: [ value(0, '5px',     'replace'),
                             value(0, '8px',     'replace', 'linear'),
-                            value(1, undefined, 'add') ] } ]
+                            value(1, undefined, 'replace') ] } ]
   },
 
   { desc:     'a missing property in initial keyframe with duplicate offset '
               + 'along with other values',
     frames:   [ { left: '10px',              offset: 0 },
                 { left: '8px', right: '8px', offset: 1 },
                 { left: '5px', right: '5px', offset: 1 } ],
     expected: [ { property: 'left',
                   values: [ value(0, '10px', 'replace', 'linear'),
                             value(1, '8px',  'replace'),
                             value(1, '5px',  'replace') ] },
                 { property: 'right',
-                  values: [ value(0, undefined, 'add', 'linear'),
+                  values: [ value(0, undefined, 'replace', 'linear'),
                             value(1, '8px',     'replace'),
                             value(1, '5px',     'replace') ] } ]
   },
 ];
 
 SpecialPowers.pushPrefEnv(
   { set: [["dom.animations-api.core.enabled", true]] },
   function() {
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -591,21 +591,23 @@ GetMinAndMaxScaleForAnimationProperty(co
       if (!baseStyle.IsNull()) {
         // FIXME: Bug 1311257: We need to get the baseStyle for
         //        RawServoAnimationValue.
         UpdateMinMaxScale(aFrame, { baseStyle, nullptr }, aMinScale, aMaxScale);
       }
 
       for (const AnimationPropertySegment& segment : prop.mSegments) {
         // In case of add or accumulate composite, StyleAnimationValue does
-        // not have a valid value.
-        if (segment.mFromComposite == dom::CompositeOperation::Replace) {
+        // not have a valid valuea.
+        if (!segment.mFromValue.IsNull() &&
+            segment.mFromComposite == dom::CompositeOperation::Replace) {
           UpdateMinMaxScale(aFrame, segment.mFromValue, aMinScale, aMaxScale);
         }
-        if (segment.mToComposite == dom::CompositeOperation::Replace) {
+        if (!segment.mToValue.IsNull() &&
+            segment.mToComposite == dom::CompositeOperation::Replace) {
           UpdateMinMaxScale(aFrame, segment.mToValue, aMinScale, aMaxScale);
         }
       }
     }
   }
 }
 
 gfxSize
@@ -9354,9 +9356,9 @@ nsLayoutUtils::ComputeGeometryBox(nsIFra
 }
 
 /* static */ bool
 nsLayoutUtils::HasCSSBoxLayout(nsIFrame* aFrame)
 {
   // Except SVG outer element, all SVG element does not have CSS box layout.
   return !aFrame->IsFrameOfType(nsIFrame::eSVG) ||
           aFrame->GetType() == nsGkAtoms::svgOuterSVGFrame;
-}
\ No newline at end of file
+}
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -550,24 +550,22 @@ private:
     nsPresContext* aPresContext,
     nsCSSKeyframeRule* aKeyframeRule,
     const Maybe<ComputedTimingFunction>& aInheritedTimingFunction);
   nsTArray<PropertyValuePair> GetKeyframePropertyValues(
     nsPresContext* aPresContext,
     nsCSSKeyframeRule* aKeyframeRule,
     nsCSSPropertyIDSet& aAnimatedProperties);
   void FillInMissingKeyframeValues(
-    nsPresContext* aPresContext,
     nsCSSPropertyIDSet aAnimatedProperties,
     nsCSSPropertyIDSet aPropertiesSetAtStart,
     nsCSSPropertyIDSet aPropertiesSetAtEnd,
     const Maybe<ComputedTimingFunction>& aInheritedTimingFunction,
     nsTArray<Keyframe>& aKeyframes);
-  void AppendProperty(nsPresContext* aPresContext,
-                      nsCSSPropertyID aProperty,
+  void AppendProperty(nsCSSPropertyID aProperty,
                       nsTArray<PropertyValuePair>& aPropertyValues);
   nsCSSValue GetComputedValue(nsPresContext* aPresContext,
                               nsCSSPropertyID aProperty);
 
   static TimingParams TimingParamsFrom(
     const StyleAnimation& aStyleAnimation)
   {
     TimingParams timing;
@@ -842,18 +840,18 @@ CSSAnimationBuilder::BuildAnimationFrame
     if (keyframe.mPropertyValues.IsEmpty()) {
       keyframes.RemoveElementAt(keyframeIdx - 1);
       // existingKeyframe might dangle now
     }
   }
 
   // Finally, we need to look for any animated properties that have an
   // implicit 'to' or 'from' value and fill in the appropriate keyframe
-  // with the current computed style.
-  FillInMissingKeyframeValues(aPresContext, animatedProperties,
+  // with the neutral value ( eCSSUnit_Null ).
+  FillInMissingKeyframeValues(animatedProperties,
                               propertiesSetAtStart, propertiesSetAtEnd,
                               inheritedTimingFunction, keyframes);
 
   return keyframes;
 }
 
 Maybe<ComputedTimingFunction>
 CSSAnimationBuilder::GetKeyframeTimingFunction(
@@ -965,17 +963,16 @@ FindMatchingKeyframe(
     }
     ++aIndex;
   }
   return false;
 }
 
 void
 CSSAnimationBuilder::FillInMissingKeyframeValues(
-    nsPresContext* aPresContext,
     nsCSSPropertyIDSet aAnimatedProperties,
     nsCSSPropertyIDSet aPropertiesSetAtStart,
     nsCSSPropertyIDSet aPropertiesSetAtEnd,
     const Maybe<ComputedTimingFunction>& aInheritedTimingFunction,
     nsTArray<Keyframe>& aKeyframes)
 {
   static const size_t kNotSet = static_cast<size_t>(-1);
 
@@ -1022,33 +1019,31 @@ CSSAnimationBuilder::FillInMissingKeyfra
   for (nsCSSPropertyID prop = nsCSSPropertyID(0);
        prop < eCSSProperty_COUNT_no_shorthands;
        prop = nsCSSPropertyID(prop + 1)) {
     if (!aAnimatedProperties.HasProperty(prop)) {
       continue;
     }
 
     if (startKeyframe && !aPropertiesSetAtStart.HasProperty(prop)) {
-      AppendProperty(aPresContext, prop, startKeyframe->mPropertyValues);
+      AppendProperty(prop, startKeyframe->mPropertyValues);
     }
     if (endKeyframe && !aPropertiesSetAtEnd.HasProperty(prop)) {
-      AppendProperty(aPresContext, prop, endKeyframe->mPropertyValues);
+      AppendProperty(prop, endKeyframe->mPropertyValues);
     }
   }
 }
 
 void
 CSSAnimationBuilder::AppendProperty(
-    nsPresContext* aPresContext,
     nsCSSPropertyID aProperty,
     nsTArray<PropertyValuePair>& aPropertyValues)
 {
   PropertyValuePair propertyValue;
   propertyValue.mProperty = aProperty;
-  propertyValue.mValue = GetComputedValue(aPresContext, aProperty);
 
   aPropertyValues.AppendElement(Move(propertyValue));
 }
 
 nsCSSValue
 CSSAnimationBuilder::GetComputedValue(nsPresContext* aPresContext,
                                       nsCSSPropertyID aProperty)
 {