Bug 1259285 - Part1 - Move CSS/Web Animations-specific visibility handling. r=birtles
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Wed, 20 Apr 2016 09:05:29 +0900
changeset 293968 dbe9e023494e73887a2d30625f3bdbf8d8d5df6d
parent 293967 a46d02998795b1e04849fc6d4fe0fbe09235fcca
child 293969 c5612333462913f110509fcd201e7857ee8bc4e2
push id30194
push usercbook@mozilla.com
push dateWed, 20 Apr 2016 09:50:56 +0000
treeherdermozilla-central@f05a1242fb29 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1259285
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 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
@@ -2256,25 +2256,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
@@ -4007,16 +4007,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