Bug 1268858 - Update CSS animation keyframe handling to support CSS variables; r=heycam, a=sylvestre
authorBrian Birtles <birtles@gmail.com>
Mon, 09 May 2016 08:27:29 +0900
changeset 333003 9fe9c798d44ff40eb793feb857602cce7dff16e8
parent 333002 4428e7c6fafd8da465ff647d1fdd11360c84144a
child 333004 bcac6066d9267ca2ea7b281cfcf2223450435a5e
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam, sylvestre
bugs1268858
milestone48.0a2
Bug 1268858 - Update CSS animation keyframe handling to support CSS variables; r=heycam, a=sylvestre
dom/animation/KeyframeUtils.cpp
dom/animation/test/chrome/test_animation_properties.html
dom/animation/test/css-animations/file_keyframeeffect-getframes.html
dom/animation/test/css-transitions/file_keyframeeffect-getframes.html
layout/style/nsAnimationManager.cpp
testing/web-platform/meta/web-animations/keyframe-effect/constructor.html.ini
testing/web-platform/meta/web-animations/keyframe-effect/setFrames.html.ini
testing/web-platform/tests/web-animations/resources/keyframe-utils.js
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -298,16 +298,40 @@ public:
   }
 
   static bool LessThan(const Keyframe& aLhs, const Keyframe& aRhs)
   {
     return aLhs.mComputedOffset < aRhs.mComputedOffset;
   }
 };
 
+// ------------------------------------------------------------------
+//
+// Inlined helper methods
+//
+// ------------------------------------------------------------------
+
+inline bool
+IsInvalidValuePair(const PropertyValuePair& aPair)
+{
+  // There are three types of values we store as token streams:
+  //
+  // * Shorthand values (where we manually extract the token stream's string
+  //   value) and pass that along to various parsing methods
+  // * Longhand values with variable references
+  // * Invalid values
+  //
+  // We can distinguish between the last two cases because for invalid values
+  // we leave the token stream's mPropertyID as eCSSProperty_UNKNOWN.
+  return !nsCSSProps::IsShorthand(aPair.mProperty) &&
+         aPair.mValue.GetUnit() == eCSSUnit_TokenStream &&
+         aPair.mValue.GetTokenStreamValue()->mPropertyID
+           == eCSSProperty_UNKNOWN;
+}
+
 
 // ------------------------------------------------------------------
 //
 // Internal helper method declarations
 //
 // ------------------------------------------------------------------
 
 static void
@@ -471,20 +495,17 @@ KeyframeUtils::GetAnimationPropertiesFro
   MOZ_ASSERT(aElement);
 
   nsTArray<KeyframeValueEntry> entries;
 
   for (const Keyframe& frame : aFrames) {
     nsCSSPropertySet propertiesOnThisKeyframe;
     for (const PropertyValuePair& pair :
            PropertyPriorityIterator(frame.mPropertyValues)) {
-      // We currently store invalid longhand values on keyframes as a token
-      // stream so if we see one of them, just keep moving.
-      if (!nsCSSProps::IsShorthand(pair.mProperty) &&
-          pair.mValue.GetUnit() == eCSSUnit_TokenStream) {
+      if (IsInvalidValuePair(pair)) {
         continue;
       }
 
       // Expand each value into the set of longhands and produce
       // a KeyframeValueEntry for each value.
       nsTArray<PropertyStyleAnimationValuePair> values;
 
       // For shorthands, we store the string as a token stream so we need to
@@ -814,16 +835,25 @@ MakePropertyValuePair(nsCSSProperty aPro
                                   value);
   }
 
   if (value.GetUnit() == eCSSUnit_Null) {
     // Either we have a shorthand, or we failed to parse a longhand.
     // In either case, store the string value as a token stream.
     nsCSSValueTokenStream* tokenStream = new nsCSSValueTokenStream;
     tokenStream->mTokenStream = aStringValue;
+
+    // We are about to convert a null value to a token stream value but
+    // by leaving the mPropertyID as unknown, we will be able to
+    // distinguish between invalid values and valid token stream values
+    // (e.g. values with variable references).
+    MOZ_ASSERT(tokenStream->mPropertyID == eCSSProperty_UNKNOWN,
+               "The property of a token stream should be initialized"
+               " to unknown");
+
     // By leaving mShorthandPropertyID as unknown, we ensure that when
     // we call nsCSSValue::AppendToString we get back the string stored
     // in mTokenStream.
     MOZ_ASSERT(tokenStream->mShorthandPropertyID == eCSSProperty_UNKNOWN,
                "The shorthand property of a token stream should be initialized"
                " to unknown");
     value.SetTokenStreamValue(tokenStream);
   }
@@ -1097,31 +1127,32 @@ RequiresAdditiveAnimation(const nsTArray
     double computedOffset = i == len - 1
                             ? 1.0
                             : i == 0 ? 0.0 : 0.5;
     double offsetToUse = frame.mOffset
                          ? frame.mOffset.value()
                          : computedOffset;
 
     for (const PropertyValuePair& pair : frame.mPropertyValues) {
+      if (IsInvalidValuePair(pair)) {
+        continue;
+      }
+
       if (nsCSSProps::IsShorthand(pair.mProperty)) {
         nsCSSValueTokenStream* tokenStream = pair.mValue.GetTokenStreamValue();
         nsCSSParser parser(aDocument->CSSLoader());
         if (!parser.IsValueValidForProperty(pair.mProperty,
                                             tokenStream->mTokenStream)) {
           continue;
         }
         CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(
             prop, pair.mProperty, nsCSSProps::eEnabledForAllContent) {
           addToPropertySets(*prop, offsetToUse);
         }
       } else {
-        if (pair.mValue.GetUnit() == eCSSUnit_TokenStream) {
-          continue;
-        }
         addToPropertySets(pair.mProperty, offsetToUse);
       }
     }
   }
 
   return !propertiesWithFromValue.Equals(properties) ||
          !propertiesWithToValue.Equals(properties);
 }
--- a/dom/animation/test/chrome/test_animation_properties.html
+++ b/dom/animation/test/chrome/test_animation_properties.html
@@ -7,16 +7,21 @@
 <script type="application/javascript" src="../testharnessreport.js"></script>
 <script type="application/javascript" src="../testcommon.js"></script>
 </head>
 <body>
 <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1254419"
   target="_blank">Mozilla Bug 1254419</a>
 <div id="log"></div>
 <style>
+
+:root {
+  --var-100px: 100px;
+  --var-100px-200px: 100px 200px;
+}
 div {
   font-size: 10px; /* For calculating em-based units */
 }
 </style>
 <script>
 'use strict';
 
 function assert_properties_equal(actual, expected) {
@@ -440,17 +445,17 @@ var gTests = [
                             value(1, '3px', 'replace') ] },
                 { property: 'border-right-width',
                   values: [ value(0, '2px', 'replace', 'linear'),
                             value(1, '3px', 'replace') ] },
                 { property: 'border-top-width',
                   values: [ value(0, '2px', 'replace', 'linear'),
                             value(1, '3px', 'replace') ] } ]
   },
-  { desc:     'a keyframe sequence where greater shorthand precedes' +
+  { desc:     'a keyframe sequence where greater shorthand precedes'
               + ' lesser shorthand',
     frames:   [ { offset: 0, border: '2px dotted rgb(4, 5, 6)',
                              borderLeft: '1px solid rgb(1, 2, 3)' },
                 { offset: 1, border: '3px dashed rgb(7, 8, 9)' } ],
     expected: [ { property: 'border-bottom-color',
                   values: [ value(0, 'rgb(4, 5, 6)', 'replace', 'linear'),
                             value(1, 'rgb(7, 8, 9)', 'replace') ] },
                 { property: 'border-left-color',
@@ -482,17 +487,57 @@ var gTests = [
   //
   // ---------------------------------------------------------------------
 
   { desc:     'em units are resolved to px values',
     frames:   { left: ['10em', '20em'] },
     expected: [ { property: 'left',
                   values: [ value(0, '100px', 'replace', 'linear'),
                             value(1, '200px', 'replace') ] } ]
-  }
+  },
+  { desc:     'calc() expressions are resolved to the equivalent units',
+    frames:   { left: ['calc(10em + 10px)', 'calc(10em + 10%)'] },
+    expected: [ { property: 'left',
+                  values: [ value(0, 'calc(110px)', 'replace', 'linear'),
+                            value(1, 'calc(100px + 10%)', 'replace') ] } ]
+  },
+
+  // ---------------------------------------------------------------------
+  //
+  // Tests for CSS variable handling conversion
+  //
+  // ---------------------------------------------------------------------
+  { desc:     'CSS variables are resolved to their corresponding values',
+    frames:   { left: ['10px', 'var(--var-100px)'] },
+    expected: [ { property: 'left',
+                  values: [ value(0, '10px', 'replace', 'linear'),
+                            value(1, '100px', 'replace') ] } ]
+  },
+  { desc:     'CSS variables in calc() expressions are resolved',
+    frames:   { left: ['10px', 'calc(var(--var-100px) / 2 - 10%)'] },
+    expected: [ { property: 'left',
+                  values: [ value(0, '10px', 'replace', 'linear'),
+                            value(1, 'calc(50px + -10%)', 'replace') ] } ]
+  },
+  { desc:     'CSS variables in shorthands are resolved to their corresponding'
+              + ' values',
+    frames:   { margin: ['10px', 'var(--var-100px-200px)'] },
+    expected: [ { property: 'margin-top',
+                  values: [ value(0, '10px', 'replace', 'linear'),
+                            value(1, '100px', 'replace') ] },
+                { property: 'margin-right',
+                  values: [ value(0, '10px', 'replace', 'linear'),
+                            value(1, '200px', 'replace') ] },
+                { property: 'margin-bottom',
+                  values: [ value(0, '10px', 'replace', 'linear'),
+                            value(1, '100px', 'replace') ] },
+                { property: 'margin-left',
+                  values: [ value(0, '10px', 'replace', 'linear'),
+                            value(1, '200px', 'replace') ] } ]
+  },
 ];
 
 gTests.forEach(function(subtest) {
   test(function(t) {
     var div = addDiv(t);
     var animation = div.animate(subtest.frames, 100 * MS_PER_SEC);
     assert_properties_equal(animation.effect.getProperties(),
                             subtest.expected);
--- a/dom/animation/test/css-animations/file_keyframeeffect-getframes.html
+++ b/dom/animation/test/css-animations/file_keyframeeffect-getframes.html
@@ -123,16 +123,26 @@
 
 @keyframes anim-text-shadow {
   to { text-shadow: none; }
 }
 
 @keyframes anim-background-size {
   to { background-size: 50%, 6px, contain }
 }
+
+:root {
+  --var-100px: 100px;
+}
+@keyframes anim-variables {
+  to { transform: translate(var(--var-100px), 0) }
+}
+@keyframes anim-variables-shorthand {
+  to { margin: var(--var-100px) }
+}
 </style>
 <body>
 <script>
 "use strict";
 
 function getFrames(e) {
   return e.getAnimations()[0].effect.getFrames();
 }
@@ -200,18 +210,20 @@ test(function(t) {
   var div = addDiv(t);
 
   div.style.animation = 'anim-simple 100s';
   var frames = getFrames(div);
 
   assert_equals(frames.length, 2, "number of frames");
 
   var expected = [
-    { offset: 0, computedOffset: 0, easing: "ease", color: "black" },
-    { offset: 1, computedOffset: 1, easing: "ease", color: "white" },
+    { offset: 0, computedOffset: 0, easing: "ease",
+      color: "rgb(0, 0, 0)" },
+    { offset: 1, computedOffset: 1, easing: "ease",
+      color: "rgb(255, 255, 255)" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected frames for a simple ' +
    'animation');
 
@@ -293,17 +305,18 @@ test(function(t) {
 
   div.style.animation = 'anim-omit-to 100s';
   div.style.color = 'white';
   var frames = getFrames(div);
 
   assert_equals(frames.length, 2, "number of frames");
 
   var expected = [
-    { offset: 0, computedOffset: 0, easing: "ease", color: "blue" },
+    { offset: 0, computedOffset: 0, easing: "ease",
+      color: "rgb(0, 0, 255)" },
     { offset: 1, computedOffset: 1, easing: "ease",
       color: "rgb(255, 255, 255)" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected frames for an ' +
@@ -316,17 +329,18 @@ test(function(t) {
   div.style.color = 'white';
   var frames = getFrames(div);
 
   assert_equals(frames.length, 2, "number of frames");
 
   var expected = [
     { offset: 0, computedOffset: 0, easing: "ease",
       color: "rgb(255, 255, 255)" },
-    { offset: 1, computedOffset: 1, easing: "ease", color: "blue" },
+    { offset: 1, computedOffset: 1, easing: "ease",
+      color: "rgb(0, 0, 255)" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected frames for an ' +
    'animation with a 100% keyframe and no 0% keyframe');
 
@@ -337,17 +351,18 @@ test(function(t) {
   div.style.color = 'white';
   var frames = getFrames(div);
 
   assert_equals(frames.length, 3, "number of frames");
 
   var expected = [
     { offset: 0,   computedOffset: 0,   easing: "ease",
       color: "rgb(255, 255, 255)" },
-    { offset: 0.5, computedOffset: 0.5, easing: "ease", color: "blue" },
+    { offset: 0.5, computedOffset: 0.5, easing: "ease",
+      color: "rgb(0, 0, 255)" },
     { offset: 1,   computedOffset: 1,   easing: "ease",
       color: "rgb(255, 255, 255)" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected frames for an ' +
@@ -381,23 +396,23 @@ test(function(t) {
 
   div.style.animation = 'anim-different-props 100s';
   var frames = getFrames(div);
 
   assert_equals(frames.length, 4, "number of frames");
 
   var expected = [
     { offset: 0, computedOffset: 0, easing: "ease",
-      color: "black", marginTop: "8px" },
+      color: "rgb(0, 0, 0)", marginTop: "8px" },
     { offset: 0.25, computedOffset: 0.25, easing: "ease",
-      color: "blue" },
+      color: "rgb(0, 0, 255)" },
     { offset: 0.75, computedOffset: 0.75, easing: "ease",
       marginTop: "12px" },
     { offset: 1, computedOffset: 1, easing: "ease",
-      color: "white", marginTop: "16px" },
+      color: "rgb(255, 255, 255)", marginTop: "16px" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected frames for an ' +
    'animation with different properties on different keyframes, all ' +
    'with the same easing function');
@@ -407,23 +422,23 @@ test(function(t) {
 
   div.style.animation = 'anim-different-props-and-easing 100s';
   var frames = getFrames(div);
 
   assert_equals(frames.length, 4, "number of frames");
 
   var expected = [
     { offset: 0, computedOffset: 0, easing: "linear",
-      color: "black", marginTop: "8px" },
+      color: "rgb(0, 0, 0)", marginTop: "8px" },
     { offset: 0.25, computedOffset: 0.25, easing: "step-end",
-      color: "blue" },
+      color: "rgb(0, 0, 255)" },
     { offset: 0.75, computedOffset: 0.75, easing: "ease-in",
       marginTop: "12px" },
     { offset: 1, computedOffset: 1, easing: "ease",
-      color: "white", marginTop: "16px" },
+      color: "rgb(255, 255, 255)", marginTop: "16px" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected frames for an ' +
    'animation with different properties on different keyframes, with ' +
    'a different easing function on each');
@@ -433,19 +448,19 @@ test(function(t) {
 
   div.style.animation = 'anim-merge-offset 100s';
   var frames = getFrames(div);
 
   assert_equals(frames.length, 2, "number of frames");
 
   var expected = [
     { offset: 0, computedOffset: 0, easing: "ease",
-      color: "black", marginTop: "8px" },
+      color: "rgb(0, 0, 0)", marginTop: "8px" },
     { offset: 1, computedOffset: 1, easing: "ease",
-      color: "white", marginTop: "16px" },
+      color: "rgb(255, 255, 255)", marginTop: "16px" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected frames for an ' +
    'animation with multiple keyframes for the same time, and all with ' +
    'the same easing function');
@@ -455,21 +470,21 @@ test(function(t) {
 
   div.style.animation = 'anim-merge-offset-and-easing 100s';
   var frames = getFrames(div);
 
   assert_equals(frames.length, 3, "number of frames");
 
   var expected = [
     { offset: 0, computedOffset: 0, easing: "step-end",
-      color: "black", fontSize: "16px" },
+      color: "rgb(0, 0, 0)", fontSize: "16px" },
     { offset: 0, computedOffset: 0, easing: "linear",
       marginTop: "8px", paddingLeft: "2px" },
     { offset: 1, computedOffset: 1, easing: "ease",
-      color: "white", fontSize: "32px", marginTop: "16px",
+      color: "rgb(255, 255, 255)", fontSize: "32px", marginTop: "16px",
       paddingLeft: "4px" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected frames for an ' +
    'animation with multiple keyframes for the same time and with ' +
@@ -614,11 +629,53 @@ test(function(t) {
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i
                         + " after updating current style");
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected values for ' +
    'animations with background-size properties and missing keyframes');
 
+test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = 'anim-variables 100s';
+
+  var frames = getFrames(div);
+
+  var expected = [
+    { offset: 0, computedOffset: 0, easing: "ease",
+      transform: "none" },
+    { offset: 1, computedOffset: 1, easing: "ease",
+      transform: "translate(100px, 0px)" },
+  ];
+  for (var i = 0; i < frames.length; i++) {
+    assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
+  }
+}, 'KeyframeEffectReadOnly.getFrames() returns expected values for ' +
+   'animations with CSS variables as keyframe values');
+
+test(function(t) {
+  var div = addDiv(t);
+  div.style.animation = 'anim-variables-shorthand 100s';
+
+  var frames = getFrames(div);
+
+  var expected = [
+    { offset: 0, computedOffset: 0, easing: "ease",
+      marginBottom: "0px",
+      marginLeft: "0px",
+      marginRight: "0px",
+      marginTop: "0px" },
+    { offset: 1, computedOffset: 1, easing: "ease",
+      marginBottom: "100px",
+      marginLeft: "100px",
+      marginRight: "100px",
+      marginTop: "100px" },
+  ];
+  for (var i = 0; i < frames.length; i++) {
+    assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
+  }
+}, 'KeyframeEffectReadOnly.getFrames() returns expected values for ' +
+   'animations with CSS variables as keyframe values in a shorthand property');
+
 done();
 </script>
 </body>
--- a/dom/animation/test/css-transitions/file_keyframeeffect-getframes.html
+++ b/dom/animation/test/css-transitions/file_keyframeeffect-getframes.html
@@ -1,11 +1,16 @@
 <!doctype html>
 <meta charset=utf-8>
 <script src="../testcommon.js"></script>
+<style>
+:root {
+  --var-100px: 100px;
+}
+</style>
 <body>
 <script>
 'use strict';
 
 function getFrames(e) {
   return e.getAnimations()[0].effect.getFrames();
 }
 
@@ -22,17 +27,17 @@ test(function(t) {
   var div = addDiv(t);
 
   div.style.left = '0px';
   window.getComputedStyle(div).transitionProperty;
   div.style.transition = 'left 100s';
   div.style.left = '100px';
 
   var frames = getFrames(div);
-  
+
   assert_equals(frames.length, 2, "number of frames");
 
   var expected = [
     { offset: 0, computedOffset: 0, easing: "ease", left: "0px" },
     { offset: 1, computedOffset: 1, easing: "linear", left: "100px" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
@@ -45,25 +50,46 @@ test(function(t) {
   var div = addDiv(t);
 
   div.style.left = '0px';
   window.getComputedStyle(div).transitionProperty;
   div.style.transition = 'left 100s steps(2,end)';
   div.style.left = '100px';
 
   var frames = getFrames(div);
-  
+
   assert_equals(frames.length, 2, "number of frames");
 
   var expected = [
     { offset: 0, computedOffset: 0, easing: "steps(2, end)", left: "0px" },
     { offset: 1, computedOffset: 1, easing: "linear", left: "100px" },
   ];
 
   for (var i = 0; i < frames.length; i++) {
     assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
   }
 }, 'KeyframeEffectReadOnly.getFrames() returns expected frames for a simple ' +
    'transition with a non-default easing function');
 
+test(function(t) {
+  var div = addDiv(t);
+  div.style.left = '0px';
+  window.getComputedStyle(div).transitionProperty;
+  div.style.transition = 'left 100s';
+  div.style.left = 'var(--var-100px)';
+
+  var frames = getFrames(div);
+
+  // CSS transition endpoints are based on the computed value so we
+  // shouldn't see the variable reference
+  var expected = [
+    { offset: 0, computedOffset: 0, easing: 'ease', left: '0px' },
+    { offset: 1, computedOffset: 1, easing: 'linear', left: '100px' },
+  ];
+  for (var i = 0; i < frames.length; i++) {
+    assert_frames_equal(frames[i], expected[i], "ComputedKeyframe #" + i);
+  }
+}, 'KeyframeEffectReadOnly.getFrames() returns expected frames for a ' +
+   'transition with a CSS variable endpoint');
+
 done();
 </script>
 </body>
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -553,17 +553,16 @@ private:
                                           const nsCSSKeyframesRule* aRule);
   Maybe<ComputedTimingFunction> GetKeyframeTimingFunction(
     nsPresContext* aPresContext,
     nsCSSKeyframeRule* aKeyframeRule,
     const Maybe<ComputedTimingFunction>& aInheritedTimingFunction);
   nsTArray<PropertyValuePair> GetKeyframePropertyValues(
     nsPresContext* aPresContext,
     nsCSSKeyframeRule* aKeyframeRule,
-    nsCSSCompressedDataBlock* aDataBlock,
     nsCSSPropertySet& aAnimatedProperties);
   void FillInMissingKeyframeValues(
     nsPresContext* aPresContext,
     nsCSSPropertySet aAnimatedProperties,
     nsCSSPropertySet aPropertiesSetAtStart,
     nsCSSPropertySet aPropertiesSetAtEnd,
     const Maybe<ComputedTimingFunction>& aInheritedTimingFunction,
     nsTArray<Keyframe>& aKeyframes);
@@ -675,53 +674,58 @@ CSSAnimationBuilder::Build(nsPresContext
 }
 
 nsTArray<Keyframe>
 CSSAnimationBuilder::BuildAnimationFrames(nsPresContext* aPresContext,
                                           const StyleAnimation& aSrc,
                                           const nsCSSKeyframesRule* aRule)
 {
   // Ideally we'd like to build up a set of Keyframe objects that more-or-less
-  // reflects the keyframes as-specified in the @keyframes rule(s). However,
-  // that proves to be difficult because the way CSS declarations are processed
-  // differs from how we are able to represent keyframes as JavaScript objects.
+  // reflect the keyframes as-specified in the @keyframes rule(s) so that
+  // authors get something intuitive when they call anim.effect.getFrames().
   //
-  // For example, in CSS the following rules differ in meaning:
+  // That, however, proves to be difficult because the way CSS declarations are
+  // processed differs from how we are able to represent keyframes as
+  // JavaScript objects in the Web Animations API.
+  //
+  // For example,
   //
   //   { margin: 10px; margin-left: 20px }
+  //
+  // could be represented as:
+  //
+  //   { margin: '10px', marginLeft: '20px' }
+  //
+  // BUT:
+  //
   //   { margin-left: 20px; margin: 10px }
   //
-  // However, in JavaScript, since the order in which object properties are
-  // enumerated is not defined, Web Animations defines that shorthands are
-  // applied first and longhands are layered on top regardless of the order
-  // in which they are specified. As a result, we would need to represent the
-  // above as:
+  // would be represented as:
   //
-  //   { margin: '10px', marginLeft: '20px' }
   //   { margin: '10px' }
   //
-  // Similarly, redundant declarations are permitted by CSS but not in
-  // JavaScript. As such,
+  // Likewise,
   //
   //   { margin-left: 20px; margin-left: 30px }
   //
   // would be represented as:
   //
   //   { marginLeft: '30px' }
   //
-  // In effect, we would need to manually apply the rules for CSS declaration
-  // processing in order to maintain the closest possibly mapping
-  // to the source and even then, the mapping would be unclear in some
-  // cases. Furthermore, @keyframes are defined to cascade so any
-  // correspondance to the source would be further obscured once we represent
-  // the result as a single array.
+  // As such, the mapping between source @keyframes and the Keyframe objects
+  // becomes obscured. The deviation is even more significant when we consider
+  // cascading between @keyframes rules and variable references in shorthand
+  // properties.
   //
-  // Until there is specified behavior for preserving shorthands we simply
-  // expand all shorthands, apply regular declaration processing, then go and
-  // pick up the last value specified for each property at each offset.
+  // We could, perhaps, produce a mapping that makes sense most of the time
+  // but it would be complex and need to be specified and implemented
+  // interoperably. Instead, for now, for CSS Animations (and CSS Transitions,
+  // for that matter) we resolve values on @keyframes down to computed values
+  // (thereby expanding shorthands and variable references) and then pick up the
+  // last value for each longhand property at each offset.
 
   // FIXME: There is a pending spec change to make multiple @keyframes
   // rules with the same name cascade but we don't support that yet.
 
   Maybe<ComputedTimingFunction> inheritedTimingFunction =
     ConvertTimingFunction(aSrc.GetTimingFunction());
 
   // First, make up Keyframe objects for each rule
@@ -730,32 +734,30 @@ CSSAnimationBuilder::BuildAnimationFrame
 
   for (auto ruleIdx = 0, ruleEnd = aRule->StyleRuleCount();
        ruleIdx != ruleEnd; ++ruleIdx) {
     css::Rule* cssRule = aRule->GetStyleRuleAt(ruleIdx);
     MOZ_ASSERT(cssRule, "must have rule");
     MOZ_ASSERT(cssRule->GetType() == css::Rule::KEYFRAME_RULE,
                "must be keyframe rule");
     nsCSSKeyframeRule* keyframeRule = static_cast<nsCSSKeyframeRule*>(cssRule);
-    nsCSSCompressedDataBlock* dataBlock =
-      keyframeRule->Declaration()->GetNormalBlock();
 
     const nsTArray<float>& keys = keyframeRule->GetKeys();
     for (float key : keys) {
       if (key < 0.0f || key > 1.0f) {
         continue;
       }
 
       Keyframe keyframe;
       keyframe.mOffset.emplace(key);
       keyframe.mTimingFunction =
         GetKeyframeTimingFunction(aPresContext, keyframeRule,
                                   inheritedTimingFunction);
       keyframe.mPropertyValues =
-        GetKeyframePropertyValues(aPresContext, keyframeRule, dataBlock,
+        GetKeyframePropertyValues(aPresContext, keyframeRule,
                                   animatedProperties);
 
       keyframes.AppendElement(Move(keyframe));
     }
   }
 
   // Next, stable sort by offset
   std::stable_sort(keyframes.begin(), keyframes.end(),
@@ -880,33 +882,44 @@ ConvertTimingFunction(const nsTimingFunc
 
   return result;
 }
 
 nsTArray<PropertyValuePair>
 CSSAnimationBuilder::GetKeyframePropertyValues(
     nsPresContext* aPresContext,
     nsCSSKeyframeRule* aKeyframeRule,
-    nsCSSCompressedDataBlock* aDataBlock,
     nsCSSPropertySet& aAnimatedProperties)
 {
   nsTArray<PropertyValuePair> result;
+  RefPtr<nsStyleContext> styleContext =
+    mResolvedStyles.Get(aPresContext, mStyleContext,
+                        aKeyframeRule->Declaration());
 
   for (nsCSSProperty prop = nsCSSProperty(0);
        prop < eCSSProperty_COUNT_no_shorthands;
        prop = nsCSSProperty(prop + 1)) {
     if (nsCSSProps::kAnimTypeTable[prop] == eStyleAnimType_None ||
         !aKeyframeRule->Declaration()->HasNonImportantValueFor(prop)) {
       continue;
     }
 
     PropertyValuePair pair;
     pair.mProperty = prop;
-    pair.mValue = *aDataBlock->ValueFor(prop);
 
+    StyleAnimationValue computedValue;
+    if (!StyleAnimationValue::ExtractComputedValue(prop, styleContext,
+                                                   computedValue)) {
+      continue;
+    }
+    DebugOnly<bool> uncomputeResult =
+      StyleAnimationValue::UncomputeValue(prop, Move(computedValue),
+                                          pair.mValue);
+    MOZ_ASSERT(uncomputeResult,
+               "Unable to get specified value from computed value");
     MOZ_ASSERT(pair.mValue.GetUnit() != eCSSUnit_Null,
                "Not expecting to read invalid properties");
 
     result.AppendElement(Move(pair));
     aAnimatedProperties.AddProperty(prop);
   }
 
   return result;
--- a/testing/web-platform/meta/web-animations/keyframe-effect/constructor.html.ini
+++ b/testing/web-platform/meta/web-animations/keyframe-effect/constructor.html.ini
@@ -43,24 +43,23 @@
   [a KeyframeEffectReadOnly can be constructed with a one property two value property-indexed keyframes specification where the second value is invalid]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
 
   [a KeyframeEffectReadOnly constructed with a one property two value property-indexed keyframes specification where the second value is invalid roundtrips]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
 
-  [a KeyframeEffectReadOnly can be constructed with a two property property-indexed keyframes specification where one property is missing from the first keyframe]
+  [a KeyframeEffectReadOnly can be constructed with a two property keyframe sequence where one property is missing from the first keyframe]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
 
-  [a KeyframeEffectReadOnly constructed with a two property property-indexed keyframes specification where one property is missing from the first keyframe roundtrips]
+  [a KeyframeEffectReadOnly constructed with a two property keyframe sequence where one property is missing from the first keyframe roundtrips]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
 
-  [a KeyframeEffectReadOnly can be constructed with a two property property-indexed keyframes specification where one property is missing from the last keyframe]
+  [a KeyframeEffectReadOnly can be constructed with a two property keyframe sequence where one property is missing from the last keyframe]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
 
-  [a KeyframeEffectReadOnly constructed with a two property property-indexed keyframes specification where one property is missing from the last keyframe roundtrips]
+  [a KeyframeEffectReadOnly constructed with a two property keyframe sequence where one property is missing from the last keyframe roundtrips]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
-
--- a/testing/web-platform/meta/web-animations/keyframe-effect/setFrames.html.ini
+++ b/testing/web-platform/meta/web-animations/keyframe-effect/setFrames.html.ini
@@ -11,19 +11,19 @@
   [Keyframes can be replaced with a one property two value property-indexed keyframes specification where the first value is invalid]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
 
   [Keyframes can be replaced with a one property two value property-indexed keyframes specification where the second value is invalid]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
 
-  [Keyframes can be replaced with a two property property-indexed keyframes specification where one property is missing from the first keyframe]
+  [Keyframes can be replaced with a keyframe sequence with different composite values, but the same composite value for a given offset]
+    expected: FAIL
+    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216843
+
+  [Keyframes can be replaced with a two property keyframe sequence where one property is missing from the first keyframe]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
 
-  [Keyframes can be replaced with a two property property-indexed keyframes specification where one property is missing from the last keyframe]
+  [Keyframes can be replaced with a two property keyframe sequence where one property is missing from the last keyframe]
     expected: FAIL
     bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216844
-
-  [Keyframes can be replaced with a keyframe sequence with different composite values, but the same composite value for a given offset]
-    expected: FAIL
-    bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1216843
--- a/testing/web-platform/tests/web-animations/resources/keyframe-utils.js
+++ b/testing/web-platform/tests/web-animations/resources/keyframe-utils.js
@@ -135,16 +135,30 @@ var gPropertyIndexedKeyframesTests = [
                left: "50px", top: "55px" }] },
   { desc:   "a one property two value property-indexed keyframes specification"
             + " that needs to stringify its values",
     input:  { opacity: [0, 1] },
     output: [{ offset: null, computedOffset: 0, easing: "linear",
                opacity: "0" },
              { offset: null, computedOffset: 1, easing: "linear",
                opacity: "1" }] },
+  { desc:   "a property-indexed keyframes specification with a CSS variable"
+            + " reference",
+    input:  { left: [ "var(--dist)", "calc(var(--dist) + 100px)" ] },
+    output: [{ offset: null, computedOffset: 0.0, easing: "linear",
+               left: "var(--dist)" },
+             { offset: null, computedOffset: 1.0, easing: "linear",
+               left: "calc(var(--dist) + 100px)" }] },
+  { desc:   "a property-indexed keyframes specification with a CSS variable"
+            + " reference in a shorthand property",
+    input:  { margin: [ "var(--dist)", "calc(var(--dist) + 100px)" ] },
+    output: [{ offset: null, computedOffset: 0.0, easing: "linear",
+               margin: "var(--dist)" },
+             { offset: null, computedOffset: 1.0, easing: "linear",
+               margin: "calc(var(--dist) + 100px)" }] },
   { desc:   "a one property one value property-indexed keyframes specification",
     input:  { left: ["10px"] },
     output: [{ offset: null, computedOffset: 1, easing: "linear",
                left: "10px" }] },
   { desc:   "a one property one non-array value property-indexed keyframes"
             + " specification",
     input:  { left: "10px" },
     output: [{ offset: null, computedOffset: 1, easing: "linear",
@@ -158,48 +172,16 @@ var gPropertyIndexedKeyframesTests = [
                left: "10px" }] },
   { desc:   "a one property two value property-indexed keyframes specification"
             + " where the second value is invalid",
     input:  { left: ["10px", "invalid"] },
     output: [{ offset: null, computedOffset: 0, easing: "linear",
                left: "10px" },
              { offset: null, computedOffset: 1, easing: "linear",
                left: "invalid" }] },
-  { desc:   "a two property property-indexed keyframes specification where one"
-            + " property is missing from the first keyframe",
-    input:  [{ offset: 0, left: "10px" },
-             { offset: 1, left: "20px", top: "30px" }],
-    output: [{ offset: 0, computedOffset: 0, easing: "linear", left: "10px" },
-             { offset: 1, computedOffset: 1, easing: "linear",
-               left: "20px", top: "30px" }] },
-  { desc:   "a two property property-indexed keyframes specification where one"
-            + " property is missing from the last keyframe",
-    input:  [{ offset: 0, left: "10px", top: "20px" },
-             { offset: 1, left: "30px" }],
-    output: [{ offset: 0, computedOffset: 0, easing: "linear",
-               left: "10px" , top: "20px" },
-             { offset: 1, computedOffset: 1, easing: "linear",
-               left: "30px" }] },
-  { desc:   "a property-indexed keyframes specification with repeated values"
-            + " at offset 0 with different easings",
-    input:  [{ offset: 0.0, left: "100px", easing: "ease" },
-             { offset: 0.0, left: "200px", easing: "ease" },
-             { offset: 0.5, left: "300px", easing: "linear" },
-             { offset: 1.0, left: "400px", easing: "ease-out" },
-             { offset: 1.0, left: "500px", easing: "step-end" }],
-    output: [{ offset: 0.0, computedOffset: 0.0, easing: "ease",
-               left: "100px" },
-             { offset: 0.0, computedOffset: 0.0, easing: "ease",
-               left: "200px" },
-             { offset: 0.5, computedOffset: 0.5, easing: "linear",
-               left: "300px" },
-             { offset: 1.0, computedOffset: 1.0, easing: "ease-out",
-               left: "400px" },
-             { offset: 1.0, computedOffset: 1.0, easing: "step-end",
-               left: "500px" }] },
 ];
 
 var gKeyframeSequenceTests = [
   { desc:   "a one property two keyframe sequence",
     input:  [{ offset: 0, left: "10px" },
              { offset: 1, left: "20px" }],
     output: [{ offset: 0, computedOffset: 0, easing: "linear", left: "10px" },
              { offset: 1, computedOffset: 1, easing: "linear", left: "20px" }]
@@ -357,16 +339,31 @@ var gKeyframeSequenceTests = [
                composite: "replace", top: "60px" }] },
   { desc:   "a one property two keyframe sequence that needs to stringify"
             + " its values",
     input:  [{ offset: 0, opacity: 0 },
              { offset: 1, opacity: 1 }],
     output: [{ offset: 0, computedOffset: 0, easing: "linear", opacity: "0" },
              { offset: 1, computedOffset: 1, easing: "linear", opacity: "1" }]
   },
+  { desc:   "a keyframe sequence with a CSS variable reference",
+    input:  [{ left: "var(--dist)" },
+             { left: "calc(var(--dist) + 100px)" }],
+    output: [{ offset: null, computedOffset: 0.0, easing: "linear",
+               left: "var(--dist)" },
+             { offset: null, computedOffset: 1.0, easing: "linear",
+               left: "calc(var(--dist) + 100px)" }] },
+  { desc:   "a keyframe sequence with a CSS variable reference in a shorthand"
+            + " property",
+    input:  [{ margin: "var(--dist)" },
+             { margin: "calc(var(--dist) + 100px)" }],
+    output: [{ offset: null, computedOffset: 0.0, easing: "linear",
+               margin: "var(--dist)" },
+             { offset: null, computedOffset: 1.0, easing: "linear",
+               margin: "calc(var(--dist) + 100px)" }] },
   { desc:   "a keyframe sequence where shorthand precedes longhand",
     input:  [{ offset: 0, margin: "10px", marginRight: "20px" },
              { offset: 1, margin: "30px" }],
     output: [{ offset: 0, computedOffset: 0, easing: "linear",
                margin: "10px", marginRight: "20px" },
              { offset: 1, computedOffset: 1, easing: "linear",
                margin: "30px" }] },
   { desc:   "a keyframe sequence where longhand precedes shorthand",
@@ -392,16 +389,48 @@ var gKeyframeSequenceTests = [
     input:  [{ offset: 0, border: "2px dotted rgb(4, 5, 6)",
                borderLeft: "1px solid rgb(1, 2, 3)" },
              { offset: 1, border: "3px dashed rgb(7, 8, 9)" }],
     output: [{ offset: 0, computedOffset: 0, easing: "linear",
                border: "2px dotted rgb(4, 5, 6)",
                borderLeft: "1px solid rgb(1, 2, 3)" },
              { offset: 1, computedOffset: 1, easing: "linear",
                border: "3px dashed rgb(7, 8, 9)" }] },
+  { desc:   "a two property keyframe sequence where one property is missing"
+            + " from the first keyframe",
+    input:  [{ offset: 0, left: "10px" },
+             { offset: 1, left: "20px", top: "30px" }],
+    output: [{ offset: 0, computedOffset: 0, easing: "linear", left: "10px" },
+             { offset: 1, computedOffset: 1, easing: "linear",
+               left: "20px", top: "30px" }] },
+  { desc:   "a two property keyframe sequence where one property is missing"
+            + " from the last keyframe",
+    input:  [{ offset: 0, left: "10px", top: "20px" },
+             { offset: 1, left: "30px" }],
+    output: [{ offset: 0, computedOffset: 0, easing: "linear",
+               left: "10px" , top: "20px" },
+             { offset: 1, computedOffset: 1, easing: "linear",
+               left: "30px" }] },
+  { desc:   "a keyframe sequence with repeated values at offset 1 with"
+            + " different easings",
+    input:  [{ offset: 0.0, left: "100px", easing: "ease" },
+             { offset: 0.0, left: "200px", easing: "ease" },
+             { offset: 0.5, left: "300px", easing: "linear" },
+             { offset: 1.0, left: "400px", easing: "ease-out" },
+             { offset: 1.0, left: "500px", easing: "step-end" }],
+    output: [{ offset: 0.0, computedOffset: 0.0, easing: "ease",
+               left: "100px" },
+             { offset: 0.0, computedOffset: 0.0, easing: "ease",
+               left: "200px" },
+             { offset: 0.5, computedOffset: 0.5, easing: "linear",
+               left: "300px" },
+             { offset: 1.0, computedOffset: 1.0, easing: "ease-out",
+               left: "400px" },
+             { offset: 1.0, computedOffset: 1.0, easing: "step-end",
+               left: "500px" }] },
 ];
 
 var gInvalidKeyframesTests = [
   { desc:     "keyframes with an out-of-bounded positive offset",
     input:    [ { opacity: 0 },
                 { opacity: 0.5, offset: 2 },
                 { opacity: 1 } ],
     expected: { name: "TypeError" } },