Bug 1259285 - Part1. Move CSS/Web Animations-specific visibility handling. r?birtles draft
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Wed, 20 Apr 2016 09:05:29 +0900
changeset 353814 85015c91ebc2f9f561825cfc5a4b7af72b27f79c
parent 352861 67ac40fb8f680ea5e03805552187ba1b5e8392a1
child 353815 d4ced7df2673a5c4a11e6f761b9ace6f652cb73f
push id15958
push usermantaroh@gmail.com
push dateWed, 20 Apr 2016 00:08:28 +0000
reviewersbirtles
bugs1259285
milestone48.0a1
Bug 1259285 - Part1. Move CSS/Web Animations-specific visibility handling. r?birtles MozReview-Commit-ID: 5ZYUhvI1cqV
dom/animation/KeyframeUtils.cpp
dom/base/nsDOMWindowUtils.cpp
dom/smil/nsSMILAnimationFunction.cpp
dom/smil/nsSMILCSSValueType.cpp
dom/smil/nsSMILCSSValueType.h
layout/style/AnimationCommon.h
layout/style/StyleAnimationValue.cpp
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -500,29 +500,16 @@ KeyframeUtils::GetAnimationPropertiesFro
         if (!StyleAnimationValue::ComputeValues(pair.mProperty,
               nsCSSProps::eEnabledForAllContent, aElement, aStyleContext,
               pair.mValue, /* aUseSVGMode */ false, values)) {
           continue;
         }
         MOZ_ASSERT(values.Length() == 1,
                    "Longhand properties should produce a single"
                    " StyleAnimationValue");
-
-        // 'visibility' requires special handling that is unique to CSS
-        // Transitions/CSS Animations/Web Animations (i.e. not SMIL) so we
-        // apply that here.
-        //
-        // Bug 1259285 - Move this code to StyleAnimationValue
-        if (pair.mProperty == eCSSProperty_visibility) {
-          MOZ_ASSERT(values[0].mValue.GetUnit() ==
-                      StyleAnimationValue::eUnit_Enumerated,
-                    "unexpected unit");
-          values[0].mValue.SetIntValue(values[0].mValue.GetIntValue(),
-                                       StyleAnimationValue::eUnit_Visibility);
-        }
       }
 
       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;
         }
--- a/dom/base/nsDOMWindowUtils.cpp
+++ b/dom/base/nsDOMWindowUtils.cpp
@@ -2253,25 +2253,16 @@ ComputeAnimationValue(nsCSSProperty aPro
 
   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(),
-                        StyleAnimationValue::eUnit_Visibility);
-  }
-
   return true;
 }
 
 NS_IMETHODIMP
 nsDOMWindowUtils::AdvanceTimeAndRefresh(int64_t aMilliseconds)
 {
   // Before we advance the time, we should trigger any animations that are
   // waiting to start. This is because there are many tests that call this
--- a/dom/smil/nsSMILAnimationFunction.cpp
+++ b/dom/smil/nsSMILAnimationFunction.cpp
@@ -4,16 +4,17 @@
  * 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/. */
 
 #include "nsSMILAnimationFunction.h"
 
 #include "mozilla/dom/SVGAnimationElement.h"
 #include "mozilla/Move.h"
 #include "nsISMILAttr.h"
+#include "nsSMILCSSValueType.h"
 #include "nsSMILParserUtils.h"
 #include "nsSMILNullType.h"
 #include "nsSMILTimedElement.h"
 #include "nsAttrValueInlines.h"
 #include "nsGkAtoms.h"
 #include "nsCOMPtr.h"
 #include "nsCOMArray.h"
 #include "nsIContent.h"
@@ -379,16 +380,25 @@ nsSMILAnimationFunction::InterpolateResu
 
     if (dur > 0) {
       simpleProgress = (double)mSampleTime / dur;
     } // else leave simpleProgress at 0.0 (e.g. if mSampleTime == dur == 0)
   }
 
   nsresult rv = NS_OK;
   nsSMILCalcMode calcMode = GetCalcMode();
+
+  // Force discrete calcMode for visibility since StyleAnimationValue will
+  // try to interpolate it using the special clamping behavior defined for
+  // CSS.
+  if (nsSMILCSSValueType::PropertyFromValue(aValues[0])
+        == eCSSProperty_visibility) {
+    calcMode = CALC_DISCRETE;
+  }
+
   if (calcMode != CALC_DISCRETE) {
     // Get the normalised progress between adjacent values
     const nsSMILValue* from = nullptr;
     const nsSMILValue* to = nullptr;
     // Init to -1 to make sure that if we ever forget to set this, the
     // MOZ_ASSERT that tests that intervalProgress is in range will fail.
     double intervalProgress = -1.f;
     if (IsToAnimation()) {
--- a/dom/smil/nsSMILCSSValueType.cpp
+++ b/dom/smil/nsSMILCSSValueType.cpp
@@ -423,8 +423,24 @@ nsSMILCSSValueType::ValueToString(const 
 {
   MOZ_ASSERT(aValue.mType == &nsSMILCSSValueType::sSingleton,
              "Unexpected SMIL value type");
   const ValueWrapper* wrapper = ExtractValueWrapper(aValue);
   return !wrapper ||
     StyleAnimationValue::UncomputeValue(wrapper->mPropID,
                                         wrapper->mCSSValue, aString);
 }
+
+// static
+nsCSSProperty
+nsSMILCSSValueType::PropertyFromValue(const nsSMILValue& aValue)
+{
+  if (aValue.mType != &nsSMILCSSValueType::sSingleton) {
+    return eCSSProperty_UNKNOWN;
+  }
+
+  const ValueWrapper* wrapper = ExtractValueWrapper(aValue);
+  if (!wrapper) {
+    return eCSSProperty_UNKNOWN;
+  }
+
+  return wrapper->mPropID;
+}
--- a/dom/smil/nsSMILCSSValueType.h
+++ b/dom/smil/nsSMILCSSValueType.h
@@ -93,14 +93,24 @@ public:
    * string will be empty.
    *
    * @param       aValue   The nsSMILValue to be converted into a string.
    * @param [out] aString  The string to be populated with the given value.
    * @return               true on success, false on failure.
    */
   static bool ValueToString(const nsSMILValue& aValue, nsAString& aString);
 
+  /**
+   * Return the CSS property animated by the specified value.
+   *
+   * @param   aValue   The nsSMILValue to examine.
+   * @return           The nsCSSProperty enum value of the property animated
+   *                   by |aValue|, or eCSSProperty_UNKNOWN if the type of
+   *                   |aValue| is not nsSMILCSSValueType.
+   */
+  static nsCSSProperty PropertyFromValue(const nsSMILValue& aValue);
+
 private:
   // Private constructor: prevent instances beyond my singleton.
   MOZ_CONSTEXPR nsSMILCSSValueType() {}
 };
 
 #endif // NS_SMILCSSVALUETYPE_H_
--- a/layout/style/AnimationCommon.h
+++ b/layout/style/AnimationCommon.h
@@ -84,23 +84,16 @@ template <class AnimationType>
 CommonAnimationManager<AnimationType>::ExtractComputedValueForTransition(
   nsCSSProperty aProperty,
   nsStyleContext* aStyleContext,
   StyleAnimationValue& aComputedValue)
 {
   bool result = StyleAnimationValue::ExtractComputedValue(aProperty,
                                                           aStyleContext,
                                                           aComputedValue);
-  if (aProperty == eCSSProperty_visibility) {
-    MOZ_ASSERT(aComputedValue.GetUnit() ==
-                 StyleAnimationValue::eUnit_Enumerated,
-               "unexpected unit");
-    aComputedValue.SetIntValue(aComputedValue.GetIntValue(),
-                               StyleAnimationValue::eUnit_Visibility);
-  }
   return result;
 }
 
 /**
  * Utility class for referencing the element that created a CSS animation or
  * transition. It is non-owning (i.e. it uses a raw pointer) since it is only
  * expected to be set by the owned animation while it actually being managed
  * by the owning element.
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -3690,16 +3690,20 @@ StyleAnimationValue::ExtractComputedValu
     }
     case eStyleAnimType_nscoord:
       aComputedValue.SetCoordValue(*static_cast<const nscoord*>(
         StyleDataAtOffset(styleStruct, ssOffset)));
       return true;
     case eStyleAnimType_EnumU8:
       aComputedValue.SetIntValue(*static_cast<const uint8_t*>(
         StyleDataAtOffset(styleStruct, ssOffset)), eUnit_Enumerated);
+      if (aProperty == eCSSProperty_visibility) {
+        aComputedValue.SetIntValue(aComputedValue.GetIntValue(),
+                                   eUnit_Visibility);
+      }
       return true;
     case eStyleAnimType_float:
       aComputedValue.SetFloatValue(*static_cast<const float*>(
         StyleDataAtOffset(styleStruct, ssOffset)));
       if (aProperty == eCSSProperty_font_size_adjust &&
           aComputedValue.GetFloatValue() == -1.0f) {
         // In nsStyleFont, we set mFont.sizeAdjust to -1.0 to represent
         // font-size-adjust: none.  Here, we have to treat this as a keyword