Bug 1245748 - Add nsStyleContext parameter to StyleAnimationValue::ComputeValue(s); r=heycam
authorBrian Birtles <birtles@gmail.com>
Tue, 22 Mar 2016 16:31:09 +0900
changeset 290208 20252fc7d044ec2b8b7e402d19e8b0d52f97e773
parent 290207 bb43b0b51db3c76af8448173e6aa71b710b76398
child 290209 47f2d102202f8e4d2afd14b8ad2d51b0b4b27474
push id30114
push usercbook@mozilla.com
push dateThu, 24 Mar 2016 15:15:54 +0000
treeherdermozilla-central@24c5fbde4488 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1245748
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1245748 - Add nsStyleContext parameter to StyleAnimationValue::ComputeValue(s); r=heycam StyleAnimationValue::ComputeValue(s) will automatically look up the style context of the supplied element. This is mostly fine, but when we start using this method in scenarios where we are building the initial style context (as happens later in this patch series) it can easily land us in a situation where we iterate indefinitely. It would be better, instead, to just explicitly pass in the style context we want to use, as we already do for StyleAnimationValue::ExtractComputedValue. MozReview-Commit-ID: ZoVBlBRRBI
dom/animation/KeyframeUtils.cpp
dom/base/nsDOMWindowUtils.cpp
dom/smil/nsSMILCSSValueType.cpp
layout/style/StyleAnimationValue.cpp
layout/style/StyleAnimationValue.h
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -5,26 +5,31 @@
 
 #include "mozilla/KeyframeUtils.h"
 
 #include "mozilla/AnimationUtils.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/Move.h"
 #include "mozilla/TimingParams.h"
 #include "mozilla/dom/BaseKeyframeTypesBinding.h" // For FastBaseKeyframe etc.
+#include "mozilla/dom/Element.h"
 #include "mozilla/dom/KeyframeEffect.h"
 #include "mozilla/dom/KeyframeEffectBinding.h"
 #include "jsapi.h" // For ForOfIterator etc.
 #include "nsClassHashtable.h"
 #include "nsCSSParser.h"
 #include "nsCSSProps.h"
 #include "nsCSSPseudoElements.h" // For CSSPseudoElementType
 #include "nsTArray.h"
 #include <algorithm> // For std::stable_sort
 
+// TODO: Remove once we drop LookupStyleContext
+#include "nsComputedDOMStyle.h"
+#include "nsIDocument.h"
+#include "nsIPresShell.h"
 
 namespace mozilla {
 
 // ------------------------------------------------------------------
 //
 // Internal data types
 //
 // ------------------------------------------------------------------
@@ -300,16 +305,20 @@ GetKeyframeListFromPropertyIndexedKeyfra
                                            nsTArray<Keyframe>& aResult,
                                            ErrorResult& aRv);
 
 static bool
 RequiresAdditiveAnimation(const nsTArray<Keyframe>& aKeyframes,
                           nsIDocument* aDocument);
 
 
+// TODO: This is only temporary until we remove the call sites for this.
+already_AddRefed<nsStyleContext>
+LookupStyleContext(dom::Element* aElement, CSSPseudoElementType aPseudoType);
+
 // ------------------------------------------------------------------
 //
 // Public API
 //
 // ------------------------------------------------------------------
 
 /* static */ void
 KeyframeUtils::BuildAnimationPropertyList(
@@ -924,16 +933,23 @@ ApplyDistributeSpacing(nsTArray<OffsetIn
  */
 static void
 GenerateValueEntries(Element* aTarget,
                      CSSPseudoElementType aPseudoType,
                      nsTArray<OffsetIndexedKeyframe>& aKeyframes,
                      nsTArray<KeyframeValueEntry>& aResult,
                      ErrorResult& aRv)
 {
+  RefPtr<nsStyleContext> styleContext =
+    LookupStyleContext(aTarget, aPseudoType);
+  if (!styleContext) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return;
+  }
+
   nsCSSPropertySet properties;              // All properties encountered.
   nsCSSPropertySet propertiesWithFromValue; // Those with a defined 0% value.
   nsCSSPropertySet propertiesWithToValue;   // Those with a defined 100% value.
 
   for (OffsetIndexedKeyframe& keyframe : aKeyframes) {
     Maybe<ComputedTimingFunction> easing =
       TimingParams::ParseEasing(keyframe.mKeyframeDict.mEasing,
                                 aTarget->OwnerDoc(), aRv);
@@ -971,17 +987,17 @@ GenerateValueEntries(Element* aTarget,
                  "ConvertKeyframeSequence should have parsed single "
                  "DOMString values from the property-values pairs");
       // Parse the property's string value and produce a KeyframeValueEntry (or
       // more than one, for shorthands) for it.
       nsTArray<PropertyStyleAnimationValuePair> values;
       if (StyleAnimationValue::ComputeValues(pair.mProperty,
                                              nsCSSProps::eEnabledForAllContent,
                                              aTarget,
-                                             aPseudoType,
+                                             styleContext,
                                              pair.mValues[0],
                                              /* aUseSVGMode */ false,
                                              values)) {
         for (auto& value : values) {
           // If we already got a value for this property on the keyframe,
           // skip this one.
           if (propertiesOnThisKeyframe.HasProperty(value.mProperty)) {
             continue;
@@ -1131,16 +1147,23 @@ BuildAnimationPropertyListFromPropertyIn
     Element* aTarget,
     CSSPseudoElementType aPseudoType,
     JS::Handle<JS::Value> aValue,
     InfallibleTArray<AnimationProperty>& aResult,
     ErrorResult& aRv)
 {
   MOZ_ASSERT(aValue.isObject());
 
+  RefPtr<nsStyleContext> styleContext =
+    LookupStyleContext(aTarget, aPseudoType);
+  if (!styleContext) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return;
+  }
+
   // Convert the object to a PropertyIndexedKeyframe dictionary to
   // get its explicit dictionary members.
   dom::binding_detail::FastBasePropertyIndexedKeyframe keyframes;
   if (!keyframes.Init(aCx, aValue, "BasePropertyIndexedKeyframe argument",
                       false)) {
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
@@ -1195,17 +1218,17 @@ BuildAnimationPropertyListFromPropertyIn
     // With future spec clarifications we might decide to preserve the invalid
     // value on the segment and make the animation code deal with the invalid
     // value instead.
     nsTArray<PropertyStyleAnimationValuePair> fromValues;
     float fromKey = 0.0f;
     if (!StyleAnimationValue::ComputeValues(pair.mProperty,
                                             nsCSSProps::eEnabledForAllContent,
                                             aTarget,
-                                            aPseudoType,
+                                            styleContext,
                                             pair.mValues[0],
                                             /* aUseSVGMode */ false,
                                             fromValues)) {
       // We need to throw for an invalid first value, since that would imply an
       // additive animation, which we don't support yet.
       aRv.Throw(NS_ERROR_DOM_ANIM_MISSING_PROPS_ERR);
       return;
     }
@@ -1248,17 +1271,17 @@ BuildAnimationPropertyListFromPropertyIn
 
     double portion = 1.0 / (count - 1);
     for (size_t i = 0; i < count - 1; ++i) {
       nsTArray<PropertyStyleAnimationValuePair> toValues;
       float toKey = (i + 1) * portion;
       if (!StyleAnimationValue::ComputeValues(pair.mProperty,
                                               nsCSSProps::eEnabledForAllContent,
                                               aTarget,
-                                              aPseudoType,
+                                              styleContext,
                                               pair.mValues[i + 1],
                                               /* aUseSVGMode */ false,
                                               toValues)) {
         if (i + 1 == count - 1) {
           // We need to throw for an invalid last value, since that would
           // imply an additive animation, which we don't support yet.
           aRv.Throw(NS_ERROR_DOM_ANIM_MISSING_PROPS_ERR);
           return;
@@ -1451,9 +1474,24 @@ RequiresAdditiveAnimation(const nsTArray
       }
     }
   }
 
   return !propertiesWithFromValue.Equals(properties) ||
          !propertiesWithToValue.Equals(properties);
 }
 
+already_AddRefed<nsStyleContext>
+LookupStyleContext(dom::Element* aElement, CSSPseudoElementType aPseudoType)
+{
+  nsIDocument* doc = aElement->GetCurrentDoc();
+  nsIPresShell* shell = doc->GetShell();
+  if (!shell) {
+    return nullptr;
+  }
+
+  nsIAtom* pseudo =
+    aPseudoType < CSSPseudoElementType::Count ?
+    nsCSSPseudoElements::GetPseudoAtom(aPseudoType) : nullptr;
+  return nsComputedDOMStyle::GetStyleContextForElement(aElement, pseudo, shell);
+}
+
 } // namespace mozilla
--- a/dom/base/nsDOMWindowUtils.cpp
+++ b/dom/base/nsDOMWindowUtils.cpp
@@ -2243,23 +2243,27 @@ nsDOMWindowUtils::BeginTabSwitch()
 }
 
 static bool
 ComputeAnimationValue(nsCSSProperty aProperty,
                       Element* aElement,
                       const nsAString& aInput,
                       StyleAnimationValue& aOutput)
 {
-
-  if (!StyleAnimationValue::ComputeValue(aProperty,
-                                         aElement,
-                                         CSSPseudoElementType::NotPseudo,
-                                         aInput,
-                                         false,
-                                         aOutput)) {
+  nsIDocument* doc = aElement->GetCurrentDoc();
+  nsIPresShell* shell = doc->GetShell();
+  if (!shell) {
+    return false;
+  }
+
+  RefPtr<nsStyleContext> styleContext =
+    nsComputedDOMStyle::GetStyleContextForElement(aElement, nullptr, shell);
+
+  if (!StyleAnimationValue::ComputeValue(aProperty, aElement, styleContext,
+                                         aInput, false, aOutput)) {
     return false;
   }
 
   // This matches TransExtractComputedValue in nsTransitionManager.cpp.
   if (aProperty == eCSSProperty_visibility) {
     MOZ_ASSERT(aOutput.GetUnit() == StyleAnimationValue::eUnit_Enumerated,
                "unexpected unit");
     aOutput.SetIntValue(aOutput.GetIntValue(),
--- a/dom/smil/nsSMILCSSValueType.cpp
+++ b/dom/smil/nsSMILCSSValueType.cpp
@@ -354,23 +354,25 @@ ValueFromStringHelper(nsCSSProperty aPro
   // they're more complicated than our simple negation logic here can handle.
   if (aPropID != eCSSProperty_stroke_dasharray) {
     int32_t absValuePos = nsSMILParserUtils::CheckForNegativeNumber(aString);
     if (absValuePos > 0) {
       isNegative = true;
       subStringBegin = (uint32_t)absValuePos; // Start parsing after '-' sign
     }
   }
+  RefPtr<nsStyleContext> styleContext =
+    nsComputedDOMStyle::GetStyleContextForElement(aTargetElement, nullptr,
+                                                  aPresContext->PresShell());
+  if (!styleContext) {
+    return false;
+  }
   nsDependentSubstring subString(aString, subStringBegin);
-  if (!StyleAnimationValue::ComputeValue(aPropID,
-                                         aTargetElement,
-                                         CSSPseudoElementType::NotPseudo,
-                                         subString,
-                                         true,
-                                         aStyleAnimValue,
+  if (!StyleAnimationValue::ComputeValue(aPropID, aTargetElement, styleContext,
+                                         subString, true, aStyleAnimValue,
                                          aIsContextSensitive)) {
     return false;
   }
   if (isNegative) {
     InvertSign(aStyleAnimValue);
   }
 
   if (aPropID == eCSSProperty_font_size) {
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -2547,46 +2547,26 @@ BuildStyleRule(nsCSSProperty aProperty,
   }
 
   RefPtr<css::StyleRule> rule = new css::StyleRule(nullptr,
                                                      declaration,
                                                      0, 0);
   return rule.forget();
 }
 
-inline
-already_AddRefed<nsStyleContext>
-LookupStyleContext(dom::Element* aElement,
-                   CSSPseudoElementType aPseudoType)
-{
-  nsIDocument* doc = aElement->GetCurrentDoc();
-  nsIPresShell* shell = doc->GetShell();
-  if (!shell) {
-    return nullptr;
-  }
-
-  nsIAtom* pseudo =
-    aPseudoType < CSSPseudoElementType::Count ?
-    nsCSSPseudoElements::GetPseudoAtom(aPseudoType) : nullptr;
-  return nsComputedDOMStyle::GetStyleContextForElement(aElement, pseudo, shell);
-}
-
 /* static */ bool
 StyleAnimationValue::ComputeValue(nsCSSProperty aProperty,
                                   dom::Element* aTargetElement,
-                                  CSSPseudoElementType aPseudoType,
+                                  nsStyleContext* aStyleContext,
                                   const nsAString& aSpecifiedValue,
                                   bool aUseSVGMode,
                                   StyleAnimationValue& aComputedValue,
                                   bool* aIsContextSensitive)
 {
   MOZ_ASSERT(aTargetElement, "null target element");
-  MOZ_ASSERT(aTargetElement->GetCurrentDoc(),
-             "we should only be able to actively animate nodes that "
-             "are in a document");
 
   // Parse specified value into a temporary css::StyleRule
   // Note: BuildStyleRule needs an element's OwnerDoc, BaseURI, and Principal.
   // If it is a pseudo element, use its parent element's OwnerDoc, BaseURI,
   // and Principal.
   RefPtr<css::StyleRule> styleRule =
     BuildStyleRule(aProperty, aTargetElement, aSpecifiedValue, aUseSVGMode);
   if (!styleRule) {
@@ -2602,95 +2582,88 @@ StyleAnimationValue::ComputeValue(nsCSSP
       // to change depending on the context
       *aIsContextSensitive = false;
     }
     return true;
   }
 
   AutoTArray<PropertyStyleAnimationValuePair,1> values;
   bool ok = ComputeValues(aProperty, nsCSSProps::eIgnoreEnabledState,
-                          aTargetElement, aPseudoType, styleRule, values,
-                          aIsContextSensitive);
+                          aTargetElement, aStyleContext, styleRule,
+                          values, aIsContextSensitive);
   if (!ok) {
     return false;
   }
 
   MOZ_ASSERT(values.Length() == 1);
   MOZ_ASSERT(values[0].mProperty == aProperty);
 
   aComputedValue = values[0].mValue;
   return true;
 }
 
 /* static */ bool
 StyleAnimationValue::ComputeValues(nsCSSProperty aProperty,
                                    nsCSSProps::EnabledState aEnabledState,
                                    dom::Element* aTargetElement,
-                                   CSSPseudoElementType aPseudoType,
+                                   nsStyleContext* aStyleContext,
                                    const nsAString& aSpecifiedValue,
                                    bool aUseSVGMode,
                                    nsTArray<PropertyStyleAnimationValuePair>& aResult)
 {
   MOZ_ASSERT(aTargetElement, "null target element");
-  MOZ_ASSERT(aTargetElement->GetCurrentDoc(),
-             "we should only be able to actively animate nodes that "
-             "are in a document");
 
   // Parse specified value into a temporary css::StyleRule
   // Note: BuildStyleRule needs an element's OwnerDoc, BaseURI, and Principal.
   // If it is a pseudo element, use its parent element's OwnerDoc, BaseURI,
   // and Principal.
   RefPtr<css::StyleRule> styleRule =
     BuildStyleRule(aProperty, aTargetElement, aSpecifiedValue, aUseSVGMode);
   if (!styleRule) {
     return false;
   }
 
   aResult.Clear();
-  return ComputeValues(aProperty, aEnabledState, aTargetElement, aPseudoType,
-                       styleRule, aResult, /* aIsContextSensitive */ nullptr);
+  return ComputeValues(aProperty, aEnabledState, aTargetElement,
+                       aStyleContext, styleRule, aResult,
+                       /* aIsContextSensitive */ nullptr);
 }
 
 /* static */ bool
 StyleAnimationValue::ComputeValues(
     nsCSSProperty aProperty,
     nsCSSProps::EnabledState aEnabledState,
     dom::Element* aTargetElement,
-    CSSPseudoElementType aPseudoType,
+    nsStyleContext* aStyleContext,
     css::StyleRule* aStyleRule,
     nsTArray<PropertyStyleAnimationValuePair>& aValues,
     bool* aIsContextSensitive)
 {
+  MOZ_ASSERT(aStyleContext);
   if (!nsCSSProps::IsEnabled(aProperty, aEnabledState)) {
     return false;
   }
 
-  // Look up style context for our target, element:psuedo pair
-  RefPtr<nsStyleContext> styleContext = LookupStyleContext(aTargetElement,
-                                                           aPseudoType);
-  if (!styleContext) {
-    return false;
-  }
-  MOZ_ASSERT(styleContext->PresContext()->StyleSet()->IsGecko(),
+  MOZ_ASSERT(aStyleContext->PresContext()->StyleSet()->IsGecko(),
              "ServoStyleSet should not use StyleAnimationValue for animations");
-  nsStyleSet* styleSet = styleContext->PresContext()->StyleSet()->AsGecko();
+  nsStyleSet* styleSet = aStyleContext->PresContext()->StyleSet()->AsGecko();
 
   RefPtr<nsStyleContext> tmpStyleContext;
   if (aIsContextSensitive) {
     MOZ_ASSERT(!nsCSSProps::IsShorthand(aProperty),
                "to correctly set aIsContextSensitive for shorthand properties, "
                "this code must be adjusted");
 
     nsCOMArray<nsIStyleRule> ruleArray;
     ruleArray.AppendObject(styleSet->InitialStyleRule());
     css::Declaration* declaration = aStyleRule->GetDeclaration();
     ruleArray.AppendObject(declaration);
     declaration->SetImmutable();
     tmpStyleContext =
-      styleSet->ResolveStyleByAddingRules(styleContext, ruleArray);
+      styleSet->ResolveStyleByAddingRules(aStyleContext, ruleArray);
     if (!tmpStyleContext) {
       return false;
     }
 
     // Force walk of rule tree
     nsStyleStructID sid = nsCSSProps::kSIDTable[aProperty];
     tmpStyleContext->StyleData(sid);
 
@@ -2708,17 +2681,17 @@ StyleAnimationValue::ComputeValues(
   // then we need to throw the temporary style context out since the property's
   // value may have been biased by the 'initial' values supplied.
   if (!aIsContextSensitive || *aIsContextSensitive) {
     nsCOMArray<nsIStyleRule> ruleArray;
     css::Declaration* declaration = aStyleRule->GetDeclaration();
     ruleArray.AppendObject(declaration);
     declaration->SetImmutable();
     tmpStyleContext =
-      styleSet->ResolveStyleByAddingRules(styleContext, ruleArray);
+      styleSet->ResolveStyleByAddingRules(aStyleContext, ruleArray);
     if (!tmpStyleContext) {
       return false;
     }
   }
 
   // Extract computed value of our property (or all longhand components, if
   // aProperty is a shorthand) from the temporary style rule
   if (nsCSSProps::IsShorthand(aProperty)) {
--- a/layout/style/StyleAnimationValue.h
+++ b/layout/style/StyleAnimationValue.h
@@ -132,18 +132,20 @@ public:
    * specified value depends on inherited style or on the values of other
    * properties.
    *
    * @param aProperty       The property whose value we're computing.
    * @param aTargetElement  The content node to which our computed value is
    *                        applicable. For pseudo-elements, this is the parent
    *                        element to which the pseudo is attached, not the
    *                        generated content node.
-   * @param aPseudoType     The type of pseudo-element to which the computed
-   *                        value is applicable.
+   * @param aStyleContext   The style context used to compute values from the
+   *                        specified value. For pseudo-elements, this should
+   *                        be the style context corresponding to the pseudo
+   *                        element.
    * @param aSpecifiedValue The specified value, from which we'll build our
    *                        computed value.
    * @param aUseSVGMode     A flag to indicate whether we should parse
    *                        |aSpecifiedValue| in SVG mode.
    * @param [out] aComputedValue The resulting computed value.
    * @param [out] aIsContextSensitive
    *                        Set to true if |aSpecifiedValue| may produce
    *                        a different |aComputedValue| depending on other CSS
@@ -151,17 +153,17 @@ public:
    *                        false otherwise.
    *                        Note that the operation of this method is
    *                        significantly faster when |aIsContextSensitive| is
    *                        nullptr.
    * @return true on success, false on failure.
    */
   static bool ComputeValue(nsCSSProperty aProperty,
                            mozilla::dom::Element* aTargetElement,
-                           CSSPseudoElementType aPseudoType,
+                           nsStyleContext* aStyleContext,
                            const nsAString& aSpecifiedValue,
                            bool aUseSVGMode,
                            StyleAnimationValue& aComputedValue,
                            bool* aIsContextSensitive = nullptr);
 
   /**
    * Like ComputeValue, but returns an array of StyleAnimationValues.
    *
@@ -170,17 +172,17 @@ public:
    * values for all of aProperty's longhand components.  aEnabledState
    * is used to filter the longhand components that will be appended
    * to aResult.  On failure, aResult might still have partial results
    * in it.
    */
   static bool ComputeValues(nsCSSProperty aProperty,
                             nsCSSProps::EnabledState aEnabledState,
                             mozilla::dom::Element* aTargetElement,
-                            CSSPseudoElementType aPseudoType,
+                            nsStyleContext* aStyleContext,
                             const nsAString& aSpecifiedValue,
                             bool aUseSVGMode,
                             nsTArray<PropertyStyleAnimationValuePair>& aResult);
 
   /**
    * Creates a specified value for the given computed value.
    *
    * The first overload fills in an nsCSSValue object; the second
@@ -398,17 +400,17 @@ public:
   bool operator==(const StyleAnimationValue& aOther) const;
   bool operator!=(const StyleAnimationValue& aOther) const
     { return !(*this == aOther); }
 
 private:
   static bool ComputeValues(nsCSSProperty aProperty,
                             nsCSSProps::EnabledState aEnabledState,
                             mozilla::dom::Element* aTargetElement,
-                            CSSPseudoElementType aPseudoType,
+                            nsStyleContext* aStyleContext,
                             mozilla::css::StyleRule* aStyleRule,
                             nsTArray<PropertyStyleAnimationValuePair>& aValues,
                             bool* aIsContextSensitive);
 
   void FreeValue();
 
   static const char16_t* GetBufferValue(nsStringBuffer* aBuffer) {
     return static_cast<char16_t*>(aBuffer->Data());