From 4dafc82388c21708b32f3ebb64ec482639c455f5 Mon Sep 17 00:00:00 2001
From: Brian Birtles <birtles@gmail.com>
Date: Fri, 13 May 2016 11:56:10 +0900
Subject: [PATCH] Bug 1268858 part 2 - Distinguish between valid and invalid
 token stream values in keyframe value handling; r=heycam

In order to support CSS variable references in animation properties we need to
handle token stream values. However, we already use token stream values for two
other purposes:

* To store shorthand property values
* To store invalid longhand property values

The shorthand property value case is already handled correctly, however for
longhand values we need to distinguish between valid token streams (e.g.
values that contain variable references) and invalid values stored as token
streams.

This patch makes us do that by looking at the mPropertyID member of the
nsCSSValueTokenStream object which will be eCSSProperty_UNKNOWN for an invalid
value but will be set to something sensible for a valid token stream.


MozReview-Commit-ID: AXUaO5dopBC
---
 dom/animation/KeyframeUtils.cpp | 45 ++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/dom/animation/KeyframeUtils.cpp b/dom/animation/KeyframeUtils.cpp
index d0b3d72776b3..7aae33ee864a 100644
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -303,6 +303,30 @@ public:
   }
 };
 
+// ------------------------------------------------------------------
+//
+// 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;
+}
+
 
 // ------------------------------------------------------------------
 //
@@ -476,10 +500,7 @@ KeyframeUtils::GetAnimationPropertiesFromKeyframes(
     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;
       }
 
@@ -819,6 +840,15 @@ MakePropertyValuePair(nsCSSProperty aProperty, const nsAString& aStringValue,
     // 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.
@@ -1102,6 +1132,10 @@ RequiresAdditiveAnimation(const nsTArray<Keyframe>& aKeyframes,
                          : 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());
@@ -1114,9 +1148,6 @@ RequiresAdditiveAnimation(const nsTArray<Keyframe>& aKeyframes,
           addToPropertySets(*prop, offsetToUse);
         }
       } else {
-        if (pair.mValue.GetUnit() == eCSSUnit_TokenStream) {
-          continue;
-        }
         addToPropertySets(pair.mProperty, offsetToUse);
       }
     }