Bug 1315874 - Use base style context to resolve base values in nsSMILCSSProperty; r=heycam draft
authorBrian Birtles <birtles@gmail.com>
Mon, 03 Apr 2017 16:49:27 +0900
changeset 555976 9d8c415a78504f5a06c149d6b933f727a272d7e5
parent 555975 e47894de78368ecdf2b07cd4f21e5348909f30da
child 622741 b07aa3c697bf64c010ab0ed95233744cdc45cd41
push id52387
push userbbirtles@mozilla.com
push dateWed, 05 Apr 2017 06:09:34 +0000
reviewersheycam
bugs1315874
milestone55.0a1
Bug 1315874 - Use base style context to resolve base values in nsSMILCSSProperty; r=heycam MozReview-Commit-ID: 12Y6svWgXG
dom/smil/nsSMILCSSProperty.cpp
dom/smil/nsSMILCSSValueType.cpp
dom/smil/test/test_smilWithTransition.html
--- a/dom/smil/nsSMILCSSProperty.cpp
+++ b/dom/smil/nsSMILCSSProperty.cpp
@@ -5,56 +5,24 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* representation of a SMIL-animatable CSS property on an element */
 
 #include "nsSMILCSSProperty.h"
 
 #include "mozilla/dom/Element.h"
 #include "mozilla/Move.h"
+#include "mozilla/StyleAnimationValue.h"
+#include "nsICSSDeclaration.h"
 #include "nsSMILCSSValueType.h"
 #include "nsSMILValue.h"
-#include "nsComputedDOMStyle.h"
 #include "nsCSSProps.h"
-#include "nsIDOMElement.h"
-#include "nsIDocument.h"
-
-using namespace mozilla::dom;
-
-// Helper function
-static bool
-GetCSSComputedValue(Element* aElem,
-                    nsCSSPropertyID aPropID,
-                    nsAString& aResult)
-{
-  MOZ_ASSERT(!nsCSSProps::IsShorthand(aPropID),
-             "Can't look up computed value of shorthand property");
-  MOZ_ASSERT(nsSMILCSSProperty::IsPropertyAnimatable(aPropID),
-             "Shouldn't get here for non-animatable properties");
 
-  nsIDocument* doc = aElem->GetUncomposedDoc();
-  if (!doc) {
-    // This can happen if we process certain types of restyles mid-sample
-    // and remove anonymous animated content from the document as a result.
-    // See bug 534975.
-    return false;
-  }
-
-  nsIPresShell* shell = doc->GetShell();
-  if (!shell) {
-    NS_WARNING("Unable to look up computed style -- no pres shell");
-    return false;
-  }
-
-  RefPtr<nsComputedDOMStyle> computedStyle =
-    NS_NewComputedDOMStyle(aElem, EmptyString(), shell);
-
-  computedStyle->GetPropertyValue(aPropID, aResult);
-  return true;
-}
+using namespace mozilla;
+using namespace mozilla::dom;
 
 // Class Methods
 nsSMILCSSProperty::nsSMILCSSProperty(nsCSSPropertyID aPropID,
                                      Element* aElement,
                                      nsStyleContext* aBaseStyleContext)
   : mPropID(aPropID)
   , mElement(aElement)
   , mBaseStyleContext(aBaseStyleContext)
@@ -69,66 +37,48 @@ nsSMILCSSProperty::GetBaseValue() const
 {
   // To benefit from Return Value Optimization and avoid copy constructor calls
   // due to our use of return-by-value, we must return the exact same object
   // from ALL return points. This function must only return THIS variable:
   nsSMILValue baseValue;
 
   // SPECIAL CASE: (a) Shorthands
   //               (b) 'display'
-  if (nsCSSProps::IsShorthand(mPropID) || mPropID == eCSSProperty_display) {
+  //               (c) No base style context
+  if (nsCSSProps::IsShorthand(mPropID) ||
+      mPropID == eCSSProperty_display ||
+      !mBaseStyleContext) {
     // We can't look up the base (computed-style) value of shorthand
     // properties because they aren't guaranteed to have a consistent computed
     // value.
     //
     // Also, although we can look up the base value of the display property,
     // doing so involves clearing and resetting the property which can cause
     // frames to be recreated which we'd like to avoid.
     //
-    // In either case, just return a dummy value (initialized with the right
+    // Furthermore, if we don't (yet) have a base style context we obviously
+    // can't resolve a base value.
+    //
+    // In any case, just return a dummy value (initialized with the right
     // type, so as not to indicate failure).
     nsSMILValue tmpVal(&nsSMILCSSValueType::sSingleton);
     Swap(baseValue, tmpVal);
     return baseValue;
   }
 
-  // GENERAL CASE: Non-Shorthands
-  // (1) Put empty string in override style for property mPropID
-  // (saving old override style value, so we can set it again when we're done)
-  nsICSSDeclaration* overrideDecl = mElement->GetSMILOverrideStyle();
-  nsAutoString cachedOverrideStyleVal;
-  if (overrideDecl) {
-    overrideDecl->GetPropertyValue(mPropID, cachedOverrideStyleVal);
-    // (Don't bother clearing override style if it's already empty)
-    if (!cachedOverrideStyleVal.IsEmpty()) {
-      overrideDecl->SetPropertyValue(mPropID, EmptyString());
-    }
+  StyleAnimationValue computedValue;
+  if (!StyleAnimationValue::ExtractComputedValue(mPropID,
+                                                 mBaseStyleContext,
+                                                 computedValue)) {
+    return baseValue;
   }
 
-  // (2) Get Computed Style
-  nsAutoString computedStyleVal;
-  bool didGetComputedVal = GetCSSComputedValue(mElement, mPropID,
-                                                 computedStyleVal);
-
-  // (3) Put cached override style back (if it's non-empty)
-  if (overrideDecl && !cachedOverrideStyleVal.IsEmpty()) {
-    overrideDecl->SetPropertyValue(mPropID, cachedOverrideStyleVal);
-  }
-
-  // (4) Populate our nsSMILValue from the computed style
-  if (didGetComputedVal) {
-    // When we parse animation values we check if they are context-sensitive or
-    // not so that we don't cache animation values whose meaning may change.
-    // For base values however this is unnecessary since on each sample the
-    // compositor will fetch the (computed) base value and compare it against
-    // the cached (computed) value and detect changes for us.
-    nsSMILCSSValueType::ValueFromString(mPropID, mElement,
-                                        computedStyleVal, baseValue,
-                                        nullptr);
-  }
+  baseValue =
+    nsSMILCSSValueType::ValueFromAnimationValue(mPropID, mElement,
+                                                computedValue);
   return baseValue;
 }
 
 nsresult
 nsSMILCSSProperty::ValueFromString(const nsAString& aStr,
                                    const SVGAnimationElement* aSrcElement,
                                    nsSMILValue& aValue,
                                    bool& aPreventCachingOfSandwich) const
--- a/dom/smil/nsSMILCSSValueType.cpp
+++ b/dom/smil/nsSMILCSSValueType.cpp
@@ -2,16 +2,18 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* representation of a value for a SMIL-animated CSS property */
 
 #include "nsSMILCSSValueType.h"
+
+#include "nsComputedDOMStyle.h"
 #include "nsString.h"
 #include "nsSMILParserUtils.h"
 #include "nsSMILValue.h"
 #include "nsCSSValue.h"
 #include "nsColor.h"
 #include "nsPresContext.h"
 #include "mozilla/StyleAnimationValue.h"
 #include "mozilla/dom/Element.h"
--- a/dom/smil/test/test_smilWithTransition.html
+++ b/dom/smil/test/test_smilWithTransition.html
@@ -33,17 +33,17 @@ https://bugzilla.mozilla.org/show_bug.cg
     waitForFrame().then(function() {
       svg.setCurrentTime(1);
       ok(getComputedStyle(rect).fill, 'rgb(0, 255, 0)',
          'The end color should be lime.');
 
       return waitForAnimationFrames(2);
     }).then(function() {
       var anim = document.getAnimations()[0];
-      todo(!anim, 'Transition should not be created by restyling for SMIL');
+      ok(!anim, 'Transition should not be created by restyling for SMIL');
       SimpleTest.finish();
     });
   }
 
   // Utility methods from testcommon.js
   // For detail, see dom/animation/test/testcommon.js.
 
   function waitForFrame() {